perf(levm): replace BTreeMap/BTreeSet with FxHashMap/FxHashSet for accessed storage slots#6185
perf(levm): replace BTreeMap/BTreeSet with FxHashMap/FxHashSet for accessed storage slots#6185ilitteri wants to merge 2 commits into
Conversation
github-actions
Bot
added
levm
🤖 Kimi Code ReviewReview SummaryThe PR replaces Issues Found
Security & Consensus Considerations
Performance ImpactThe change should provide measurable performance improvements as Suggested Changes// Line 739: Fix formatting
let mut initial_accessed_storage_slots: FxHashMap<Address, FxHashSet<H256>> = FxHashMap::default();
// Line 255: Fix typo in comment
/// Used by SELFDESTRUCT to record storage reads in BLOB per EIP-7928:
Additional NoteThe CHANGELOG.md entry has the wrong year (2026 instead of 2025). Automated review by Kimi (Moonshot AI) · custom prompt |
🤖 Claude Code ReviewPerfect! Now I have all the context I need. Let me analyze this PR thoroughly. Code Review: PR #6185 - Replace BTreeMap/BTreeSet with FxHashMap/FxHashSet for accessed storage slotsSummaryThis is a well-justified performance optimization that replaces ordered data structures with hash-based ones for EVM warm/cold slot tracking. The change is targeted, minimal, and correctly preserves determinism where needed. ✅ Strengths
🔍 Detailed AnalysisNo functional changes
Access list construction (vm.rs:196-221)The critical determinism requirement is met: let mut entries = BTreeMap::<Address, BTreeSet<H256>>::new();This local conversion ensures the output
Hash function safety
|
🤖 Codex Code ReviewFindings
Notes
If you’ve already assessed collision risk as acceptable for this hot path, I’m fine with the change. Automated review by OpenAI Codex · custom prompt |
Greptile OverviewGreptile SummaryThis PR optimizes LEVM’s EIP-2929 warm/cold slot tracking by switching Key concerns before merge:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| CHANGELOG.md | Adds a perf changelog entry, but the PR number/link is incorrect (#6184 vs #6185). |
| crates/vm/levm/src/vm.rs | Switches accessed storage slot tracking from BTreeMap/BTreeSet to FxHashMap/FxHashSet and adjusts access list building; note public API now returns an unordered set which can break deterministic downstream uses. |
Sequence Diagram
sequenceDiagram
participant VM as VM
participant Sub as Substate
participant Parent as Parent Substate (checkpoint)
Note over VM,Sub: Transaction start
VM->>Sub: initialize(env, tx)
Sub->>Sub: seed accessed_addresses (origin, coinbase, precompiles)
Sub->>Sub: seed accessed_storage_slots from tx.access_list
Note over VM,Sub: During opcode execution (SLOAD/SSTORE)
VM->>Sub: is_slot_accessed(address, key)
Sub->>Sub: accessed_storage_slots.get(address).contains(key)
alt not in current
Sub->>Parent: is_slot_accessed(address, key) (recursive)
end
VM->>Sub: add_accessed_slot(address, key)
Sub->>Sub: accessed_storage_slots.entry(address).or_default().insert(key)
Note over VM,Sub: Nested call / checkpointing
VM->>Sub: push_backup()
VM->>Sub: (mutations during inner call)
alt inner call succeeds
VM->>Sub: commit_backup()
Sub->>Sub: merge delta.accessed_storage_slots into current (extend sets)
else inner call fails
VM->>Sub: revert_backup()
end
Note over VM,Sub: Access list generation (once per tx)
VM->>Sub: make_access_list()
Sub->>Sub: iterate substates and merge into local BTreeMap/BTreeSet
Sub-->>VM: Vec<AccessListEntry> (deterministic order)
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /// Returns all accessed storage slots for a given address. | ||
| /// Used by SELFDESTRUCT to record storage reads in BAL per EIP-7928: | ||
| /// "SELFDESTRUCT: Include modified/read storage keys as storage_read" | ||
| pub fn get_accessed_storage_slots(&self, address: &Address) -> BTreeSet<H256> { | ||
| let mut slots = BTreeSet::new(); | ||
| pub fn get_accessed_storage_slots(&self, address: &Address) -> FxHashSet<H256> { | ||
| let mut slots = FxHashSet::default(); |
There was a problem hiding this comment.
Return type change breaks callers
get_accessed_storage_slots now returns FxHashSet<H256> (was BTreeSet<H256>). Any caller that relied on deterministic ordering (common for receipts/tracing/encoding) will now see non-deterministic iteration order. If the result is later serialized/hashed/compared, this can cause flaky tests or consensus-sensitive differences. Consider either keeping the public API ordered (return BTreeSet/sorted Vec) or sorting at the call site(s) where determinism is required.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/vm/levm/src/vm.rs
Line: 252:256
Comment:
**Return type change breaks callers**
`get_accessed_storage_slots` now returns `FxHashSet<H256>` (was `BTreeSet<H256>`). Any caller that relied on deterministic ordering (common for receipts/tracing/encoding) will now see non-deterministic iteration order. If the result is later serialized/hashed/compared, this can cause flaky tests or consensus-sensitive differences. Consider either keeping the public API ordered (return `BTreeSet`/sorted `Vec`) or sorting at the call site(s) where determinism is required.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Pull request overview
This PR optimizes LEVM’s EIP-2929 warm/cold storage-slot tracking by switching the in-memory data structure in Substate from ordered trees to faster hash-based collections, while preserving deterministic ordering where it matters (access list output).
Changes:
- Replace
BTreeMap<Address, BTreeSet<H256>>withFxHashMap<Address, FxHashSet<H256>>forSubstate::accessed_storage_slots. - Update initialization and helper APIs (
from_accesses,get_accessed_storage_slots) to use the new hash-based types. - Add a changelog entry under Perf documenting the optimization.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
crates/vm/levm/src/vm.rs |
Switch accessed storage-slot tracking to FxHashMap/FxHashSet; keep make_access_list() deterministic via local BTreeMap/BTreeSet. |
CHANGELOG.md |
Document the performance optimization in the Perf section. |
💡 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
|
Benchmark Block Execution Results Comparison Against Main
|
…ge slots The EVM Substate tracked accessed storage slots (EIP-2929) using BTreeMap<Address, BTreeSet<H256>>, which has O(log n) lookup and insert. Since these are called on every SLOAD/SSTORE, replacing with FxHashMap<Address, FxHashSet<H256>> gives O(1) amortized operations. The cold-path make_access_list still uses local BTreeMap/BTreeSet for deterministic ordering in its output.
ilitteri
force-pushed
the
perf/flatten-accessed-storage-slots
branch
from
9e52403 to
6a5c343
Compare
Motivation
Profile analysis (Rank 7 in the optimization plan) shows that
access_storage_slotaccounts for 2.37s cumulative (4.76%) in the gigablocks scenario. TheBTreeMap<Address, BTreeSet<H256>>data structure foraccessed_storage_slotsin the EVM Substate adds O(log n) overhead to every SLOAD/SSTORE warm/cold check.Description
Replace
BTreeMap<Address, BTreeSet<H256>>withFxHashMap<Address, FxHashSet<H256>>for theaccessed_storage_slotsfield inSubstate. This gives O(1) amortized lookup and insert for the hot path (add_accessed_slot,is_slot_accessed).The cold-path
make_access_list()still constructs a localBTreeMap<Address, BTreeSet<H256>>for deterministic ordering in its output. This function is called at most once per transaction.Files modified:
crates/vm/levm/src/vm.rsBenchmark Results
Baseline: runs #63/#64/#65 on
mainat1a47fb18Profile Highlights
Gigablocks (runs #64 vs #80):
Slow (runs #65 vs #81):
RocksDB write path and compaction are essentially unchanged — this optimization is purely EVM-side.