-
Notifications
You must be signed in to change notification settings - Fork 131
perf(l1,l2): avoid unnecessary code hashing #5397
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
Conversation
Lines of code reportTotal lines added: Detailed view |
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.
Pull Request Overview
This PR introduces a performance optimization to avoid unnecessary code hashing by adding a new Code::from_bytecode_unchecked function that accepts a pre-computed or placeholder hash. The optimization targets two scenarios: snap sync (where hashes are pre-validated) and init code execution (where hashes are never accessed).
Key changes:
- Added
from_bytecode_uncheckedmethod to accept bytecode with a trusted or bogus hash - Refactored account creation code to avoid double-hashing by reusing computed hashes
- Updated VM execution paths for init code to use zero hash placeholders
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/common/types/account.rs | Implements from_bytecode_unchecked and refactors GenesisAccount conversion to avoid double-hashing |
| crates/vm/levm/src/opcode_handlers/system.rs | Uses from_bytecode_unchecked with zero hash for CREATE opcode init code |
| crates/vm/levm/src/hooks/default_hook.rs | Uses from_bytecode_unchecked with zero hash for create transaction init code |
| crates/vm/levm/runner/src/input.rs | Refactors to reuse computed hash instead of hashing twice |
| crates/networking/p2p/sync.rs | Attempts to use from_bytecode_unchecked for snap sync with pre-validated hashes (contains bug) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Benchmark Results ComparisonNo significant difference was registered for any benchmark run. Detailed ResultsBenchmark Results: BubbleSort
Benchmark Results: ERC20Approval
Benchmark Results: ERC20Mint
Benchmark Results: ERC20Transfer
Benchmark Results: Factorial
Benchmark Results: FactorialRecursive
Benchmark Results: Fibonacci
Benchmark Results: FibonacciRecursive
Benchmark Results: ManyHashes
Benchmark Results: MstoreBench
Benchmark Results: Push
Benchmark Results: SstoreBench_no_opt
|
Implements the `Code::from_bytecode_unchecked` function to create the code object from a trusted or bogus hash. The trusted case is used for snap sync, where the downloader task already checks the hash is correct, so it can pass it down. The bogus hash case is for init code, where the hash is never used (there is no account pointing to it, no entries in the DB addressed by it and no opcode can access the current code's hash, only the hash for an account associated to an address).
b5e9cfa to
10be50c
Compare
Benchmark Block Execution Results Comparison Against Main
|
JereSalo
left a comment
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.
Very interesting change. At first I thought that something could go wrong with this but after thinking a while it seems that it's a great optimization to make. N1
The comments added are good, if you want we can add an extra comment on the attribute bytecode of CallFrame in call_frame.rs saying that in case of initcode bytecode.hash is a "dummy" because it's never read. I believe newcomers would appreciate that.
Great idea. Adding the comment. |
Co-authored-by: Tomás Grüner <[email protected]>
|
@Oppen changelog has conflicts but I think it would be good to merge afterwards. wdyt? |
Implements the
Code::from_bytecode_uncheckedfunction to create thecode object from a trusted or bogus hash.
The trusted case is used for snap sync, where the downloader task
already checks the hash is correct, so it can pass it down.
The bogus hash case is for init code, where the hash is never used
(there is no account pointing to it, no entries in the DB addressed by
it and no opcode can access the current code's hash, only the hash for
an account associated to an address).
Gas Benchmarks
Main
http://ethrex-performance-1:4000/reports/37
PR
http://ethrex-performance-1:4000/reports/38