-
Notifications
You must be signed in to change notification settings - Fork 555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tests: migrate revme benchmarks to Criterion #1569
base: main
Are you sure you want to change the base?
Conversation
There is already a cli that allows you to run bytecode in and even has a revm/bins/revme/src/cmd/evmrunner.rs Lines 94 to 103 in b3f31fc
|
Wouldn't mind replacing it with |
I missed that, this is cool! I'll update my PR to use |
Hi @rakita , I've updates the existing |
bins/revme/src/cmd/evmrunner.rs
Outdated
let evm = Evm::builder() | ||
.with_db(BenchmarkDB::new_bytecode(Bytecode::new())) | ||
.modify_tx_env(|tx| { | ||
tx.caller = address!("1000000000000000000000000000000000000000"); | ||
tx.transact_to = TxKind::Call(address!("0000000000000000000000000000000000000000")); | ||
}) | ||
.build(); | ||
|
||
let host = DummyHost::new(*evm.context.evm.env.clone()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let evm = Evm::builder() | |
.with_db(BenchmarkDB::new_bytecode(Bytecode::new())) | |
.modify_tx_env(|tx| { | |
tx.caller = address!("1000000000000000000000000000000000000000"); | |
tx.transact_to = TxKind::Call(address!("0000000000000000000000000000000000000000")); | |
}) | |
.build(); | |
let host = DummyHost::new(*evm.context.evm.env.clone()); | |
let mut env = Env::default(); | |
env.tx.caller = address!("1000000000000000000000000000000000000000"); | |
env.tx.transact_to = TxKind::Call(address!("0000000000000000000000000000000000000000")); | |
let host = DummyHost::new(env); |
Dont need builder here.
bins/revme/src/cmd/evmrunner.rs
Outdated
criterion_group.bench_function("bytecode", |b| { | ||
b.iter(|| { | ||
Interpreter::new(contract.clone(), u64::MAX, false).run( | ||
EMPTY_SHARED_MEMORY, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EMPTY_SHARED_MEMORY, | |
SharedMemory::new(); |
Shared memory is preallocated and shared between multiple calls, so having SharedMemory::new() will have an initialization hit but it will be slightly faster on memory expansion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left few nits. lgtm otherwise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This PR migrates the existing
revme evm --bench
tool to Criterion. The benefits of doing so include:The main motivation for this change is that the tool can be used in gas-cost-estimator project, but its purpose goes beyond it. You can learn more about the project from this short presentation, or visit the wiki Stage 4 Scope.
In the future, I'll extend the tool to run on a preloaded state, which is useful for testing scenarios with opcodes like
DELEGATECALL
orSLOAD
.