-
Notifications
You must be signed in to change notification settings - Fork 106
perf(levm): use bitmap for jumpdests #4608
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
base: main
Are you sure you want to change the base?
Conversation
Lines of code reportTotal lines added: Detailed view
|
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
|
Benchmark Block Execution Results Comparison Against Main
|
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 optimizes the JumpTargetFilter
implementation to address a performance bottleneck where the original is_blacklisted
function consumed ~30% of execution time during mainnet block execution.
- Replaces incremental iteration with precomputed bitmap approach using
BitVec<u8, Msb0>
- Changes
is_blacklisted
from potentially O(n) to O(1) lookup operation - Adopts Geth's
codeBitmap
strategy for identifying validJUMPDEST
positions
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
crates/vm/levm/src/utils.rs | Refactors JumpTargetFilter to use bitmap-based jumpdest tracking instead of incremental filtering |
crates/vm/levm/Cargo.toml | Adds bitvec dependency for bitmap functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
jumpdests.set(i, true); | ||
} else if (Opcode::PUSH1..=Opcode::PUSH32).contains(&opcode) { | ||
// PUSH1 (0x60) to PUSH32 (0x7f): skip 1 to 32 bytes | ||
let skip = opcode as usize - Opcode::PUSH0 as usize; |
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.
The skip calculation is incorrect. For PUSH1 through PUSH32, the number of data bytes to skip should be opcode as usize - Opcode::PUSH1 as usize + 1
. Currently, using PUSH0
as the base means PUSH1 skips 1 byte, PUSH2 skips 2 bytes, etc., but it should skip 1, 2, 3... up to 32 bytes respectively.
let skip = opcode as usize - Opcode::PUSH0 as usize; | |
let skip = opcode as usize - Opcode::PUSH1 as usize + 1; |
Copilot uses AI. Check for mistakes.
…ex into jump_dest_filter_opt
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.
Why not just check if it's a jumpdest rather than checking that it's not blacklisted now that we have a bitvec with jumpdests? It feels more straightforward and less complex but I don't know if the other approach has more benefits
Co-authored-by: Copilot <[email protected]>
Motivation
The original
is_blacklisted
function inJumpTargetFilter
was a performance bottleneck, consuming ~30% of execution time during mainnet blocks stateless execution.Description
Inspired by Geth's
codeBitmap
approach, this PR:BitVec<u8, Msb0>
where bits mark validJUMPDEST
positions, skippingPUSH1..PUSH32
data bytes.is_blacklisted
to O(1) bit lookup:address >= bytecode.len() || !jumpdests[address]
.