Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Additional arithmetic EVM opcode benchmarks #10916

Merged
merged 1 commit into from
Jul 29, 2019

Conversation

vkomenda
Copy link
Contributor

These benchmarks apply to demanding arithmetic operations, in particular to MULMOD. A helper function run_code was added in order to keep the benchmark code succinct.

@parity-cla-bot
Copy link

It looks like @vkomenda signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@ordian ordian added the A0-pleasereview 🤓 Pull request needs code review. label Jul 24, 2019
@ordian ordian added this to the 2.7 milestone Jul 24, 2019
});
}

/// Compute mulmod(U256::MAX, U256::MAX, 1) 1000 times.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of having a loop in the bytecode with different number of iterations, just to make sure it scales linearly? IMHO we should be benchmarking a single operation and criterion can take care of running it in a loop. It would make sense to have a loop if we were using a JIT compiler, but we're not.

Copy link
Contributor Author

@vkomenda vkomenda Jul 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without a loop in the smart contract you get significant noise from loading the contract. You are right, different numbers of iterations are to check that scaling is linear.

@ordian ordian added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 24, 2019
@debris debris requested a review from tomusdrw July 29, 2019 10:32
Copy link
Collaborator

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid raw bytecode duplication it might be worth to use a macro instead to generate those cases.

And long term - it would be really awesome to produce the bytecode from some mnemonic based ASM (most likely a macro again) - the current code is really unchangeable.

@debris debris merged commit ee9bfac into openethereum:master Jul 29, 2019
ordian added a commit that referenced this pull request Aug 7, 2019
* master:
  journaldb changes (#10929)
  Allow default block parameter to be blockHash (#10932)
  Enable sealing when engine is ready (#10938)
  Fix some warnings and typos. (#10941)
  Updated security@parity.io key (#10939)
  Change the return type of step_inner function. (#10940)
  get rid of hidden mutability of Spec (#10904)
  simplify BlockReward::reward implementation (#10906)
  Kaspersky AV whitelisting (#10919)
  additional arithmetic EVM opcode benchmarks (#10916)
  [Cargo.lock] cargo update -p crossbeam-epoch (#10921)
  Fixes incorrect comment. (#10913)
  Add file path to disk map write/read warnings (#10911)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants