perf(l1): batch and stream the BAL code prefetch - #7099
Conversation
|
5005218 to
39bcc7e
Compare
🤖 Kimi Code ReviewOverall this is a high-quality optimization PR with sophisticated I/O scheduling. The sharding strategy for RocksDB reads is well-reasoned and the chunking in the prefetcher correctly overlaps account state and code fetching. No critical issues found. Below are minor observations and suggestions:
|
Greptile SummaryThis PR streams BAL account/code warming in chunks and adds batched account-code retrieval across the VM and storage layers.
Confidence Score: 4/5The PR appears safe to merge after the non-blocking changelog link placeholder is replaced with PR #7099. The production batching and warming paths preserve ordering, cache behavior, and best-effort fallback semantics; the only accepted issue is an invalid changelog reference. Files Needing Attention: CHANGELOG.md
|
| Filename | Overview |
|---|---|
| crates/storage/store.rs | Adds ordered, deduplicated batched code reads using the buffer, LRU, and parallel backend reads; no concrete correctness defect was found. |
| crates/vm/levm/src/db/mod.rs | Adds caching batch methods that preserve the current database adapters' ordered one-result-per-input contract. |
| crates/vm/backends/levm/mod.rs | Reworks BAL warming into account chunks and falls back to individual code reads when a batch fails. |
| crates/blockchain/vm.rs | Bridges Store's optional-code batch results into the VM's all-present code contract while handling empty-code hashes. |
| test/tests/storage/account_code_batch_tests.rs | Covers batch parity, ordering, duplicate hashes, absent hashes, and both batching branches. |
| CHANGELOG.md | Documents the optimization but retains a broken PRNUM pull-request placeholder. |
Sequence Diagram
sequenceDiagram
participant Pipeline as Block pipeline
participant Warmer as BAL warmer
participant Cache as CachingDatabase
participant Store
participant Executor
Pipeline->>Warmer: Start with BAL accounts
loop Each account chunk
Warmer->>Cache: get_account_states_batch(addresses)
Cache->>Store: Fetch uncached account states
Store-->>Cache: Ordered states
Warmer->>Cache: get_account_codes_batch(distinct hashes)
Cache->>Store: Fetch uncached code batch
Store-->>Cache: Ordered bytecodes
end
Executor->>Cache: Read warmed accounts and code
Prompt To Fix All With AI
### Issue 1
CHANGELOG.md:18
**Unresolved pull-request placeholder**
The new changelog entry displays `#PRNUM` and links to a nonexistent `/pull/PRNUM` page instead of identifying PR #7099.
```suggestion
- Batch and stream the BAL contract-code prefetch: warm accounts and their code in chunks instead of reading every access-list account before the first bytecode, take code hashes from the account read rather than a second lookup per account, and add a batched bytecode read that resolves the buffer and code cache first, then either fans out parallel point gets or shards the remainder across concurrent `multi_get`s, whichever reaches the greater read queue depth for the batch size [#7099](https://github.com/lambdaclass/ethrex/pull/7099)
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: 5005218 | Re-trigger Greptile
| ### 2026-08-04 | ||
|
|
||
| - Cut the cost of a cold contract-code access: store jump destinations as a 1-bit-per-byte bitmap instead of a persisted RLP list of `u32` offsets, count the bytecode in the code cache's byte budget, answer `EXTCODESIZE` from the code-length table instead of materializing the bytecode, and give the account-code column families a bloom filter and 4KB data blocks. `COLD_ACCOUNT_CODE_ACCESS` drops from 7736 to 4652 gas in the EIP-8038 repricing fit, and `COLD_ACCOUNT_CODE_WRITE` from 10415 to 6355 [#7095](https://github.com/lambdaclass/ethrex/pull/7095) | ||
| - Batch and stream the BAL contract-code prefetch: warm accounts and their code in chunks instead of reading every access-list account before the first bytecode, take code hashes from the account read rather than a second lookup per account, and add a batched bytecode read that resolves the buffer and code cache first, then either fans out parallel point gets or shards the remainder across concurrent `multi_get`s, whichever reaches the greater read queue depth for the batch size [#PRNUM](https://github.com/lambdaclass/ethrex/pull/PRNUM) |
There was a problem hiding this comment.
Unresolved pull-request placeholder
The new changelog entry displays #PRNUM and links to a nonexistent /pull/PRNUM page instead of identifying PR #7099.
| - Batch and stream the BAL contract-code prefetch: warm accounts and their code in chunks instead of reading every access-list account before the first bytecode, take code hashes from the account read rather than a second lookup per account, and add a batched bytecode read that resolves the buffer and code cache first, then either fans out parallel point gets or shards the remainder across concurrent `multi_get`s, whichever reaches the greater read queue depth for the batch size [#PRNUM](https://github.com/lambdaclass/ethrex/pull/PRNUM) | |
| - Batch and stream the BAL contract-code prefetch: warm accounts and their code in chunks instead of reading every access-list account before the first bytecode, take code hashes from the account read rather than a second lookup per account, and add a batched bytecode read that resolves the buffer and code cache first, then either fans out parallel point gets or shards the remainder across concurrent `multi_get`s, whichever reaches the greater read queue depth for the batch size [#7099](https://github.com/lambdaclass/ethrex/pull/7099) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: CHANGELOG.md
Line: 18
Comment:
**Unresolved pull-request placeholder**
The new changelog entry displays `#PRNUM` and links to a nonexistent `/pull/PRNUM` page instead of identifying PR #7099.
```suggestion
- Batch and stream the BAL contract-code prefetch: warm accounts and their code in chunks instead of reading every access-list account before the first bytecode, take code hashes from the account read rather than a second lookup per account, and add a batched bytecode read that resolves the buffer and code cache first, then either fans out parallel point gets or shards the remainder across concurrent `multi_get`s, whichever reaches the greater read queue depth for the batch size [#7099](https://github.com/lambdaclass/ethrex/pull/7099)
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Already fixed. The placeholder was filled in once the PR number existed; the entry links #7099.
🤖 Codex Code ReviewNo correctness, security, or consensus findings in this diff. The store-side batch read in store.rs preserves caller order, deduplicates backend reads, and rebuilds Residual risk: I could not run Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Lines of code reportTotal lines added: Detailed view |
🤖 Claude Code ReviewReview: PR 7099 — Batch and stream the BAL code prefetchOverall design is sound and well-documented: chunking the warmer to interleave account-state and code prefetch instead of using two whole-block phases is a real fix for the head-of-line blocking described in the PR body, and the batched RocksDB read (buffer/LRU-first, then fan-out-vs-shard Findings, ranked by relevance:
Nothing here indicates a consensus-affecting or memory-safety bug — this is a warmer/prefetch path with best-effort semantics by design, and the Automated review by Claude (Anthropic) · sonnet · custom prompt |
The warmer read account states for every BAL address before fetching a single bytecode, so the executor faulted in code itself for the whole of that phase, and it then issued one point get per code hash. Warm accounts and code in chunks instead, sized to the smallest batch that still saturates the store's sharded read path, and add a batched bytecode read that resolves the buffer and LRU first, shards the remainder across one multi_get per shard, and decodes each shard in place. Code hashes now come out of the account read rather than costing a second lookup per account, and the LRU is locked once per batch instead of twice per code.
39bcc7e to
ee9e6d6
Compare
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
|
…into perf/bal-code-prefetch-batch
Benchmark Block Execution Results Comparison Against Main
|
…into perf/bal-code-prefetch-batch
…into perf/bal-code-prefetch-batch # Conflicts: # CHANGELOG.md
The shard cap was a constant 64 while the choice between sharding and the point-get fan-out compared shard count against core count, so on a host with at least 64 threads the sharded path could never be selected. Derive the cap from the core count instead, so there is always a batch size at which sharding earns its spawns. The chunk size was derived from that shard geometry, but a chunk bounds accounts while the code fetch keys on distinct uncached hashes, which is at most that and usually far fewer. Derive it from the concurrency a fetch must sustain. Warming code now goes through prefetch_codes, which returns nothing: the previous shape assembled a result vector under the write lock that guards every executor code read, for a caller that discarded it. Absent hashes are no longer an error for the batch either, so the serial per-hash fallback is gone. Tests compared the batch against a per-hash read taken after it, which the batch's own cache insert answered; they now assert against independently built codes.
The join() expect escalated a shard panic onto the warmer's thread, which the best-effort contract around this read does not cover: the fallback handles Err, not panic. Map it to one Err per key the shard covered, so the results stay aligned with the request and one bad shard costs the prefetch rather than the caller.
…into perf/bal-code-prefetch-batch
Deriving the shard cap purely from the core count cut read depth on small hosts: a 12-core box went from 64 shards to 24 on a 22.9k-hash batch, on exactly the workload this targets. Floor the cap at 64 so those hosts keep the depth they already reached, and keep the 2x-cores term so the path stays reachable above 32 cores. Sizing the warmer chunk from the core count made it worse again: a 1024-account chunk yields ~4 shards, below the gate, so the batch fell back to the fan-out and sharding never ran. On the blocks this targets the warmer occupies the whole execution window, so it is throughput-bound and depth beats starting earlier. Size the chunk to saturate the shard cap instead.
# Conflicts: # crates/vm/db.rs # crates/vm/levm/src/db/mod.rs
The warm read every bytecode a BAL's accounts reference, whether or not the executor would look at it. An opcode reading only account state (BALANCE, EXTCODEHASH) leaves all of it unread, and on a block of unique max-size contracts that is gigabytes of reads for nothing, competing with the account reads the executor is waiting on. Cap it at the bytecode cache's capacity, since past that the warm evicts what it just inserted, and fetch a chunk's hashes in slices so the budget and the cancellation flag are observed part-way through a chunk instead of once per chunk. The chunk width itself stays wide: prefetch_accounts routes a chunk to the sharded batch read only once its cold addresses reach BLOATED_BATCH_THRESHOLD, so narrowing it would leave that path unreachable.
The budget only has to bind where a block's distinct bytecode outgrows what the node keeps resident. Cover the other direction: a block that fits warms every bytecode, once each, in a single batch, exactly as it did before the budget. Also correct the rationale. The per-block cache the executor reads through is an unbounded map, so a warm past the budget does not evict what it just inserted; what it does is spend read bandwidth on bytecode a BAL cannot tell us anyone will read, and hold it resident.
Motivation
On a block whose accounts hold code the executor has not seen, the BAL warmer does not get ahead of execution. Measured on the benchmarkoor stateful suite by parsing the client's own per-block log (
warmeragainstexec), grouped by block gas:Light blocks are fine at 0.268. Once a block carries real code-loading work the warmer consumes the whole execution window and lands at the wire or later: on the slowest blocks (~6100 ms exec) it finishes with 15-24 ms of slack, and on 256 of the 525 blocks over 1000 ms it finishes after execution. So the executor is racing the warmer for bytecode instead of being shielded by it, and warmer throughput is the constraint.
Two structural reasons, both in
LEVM::warm_block_from_bal:code_hashes.par_iter().for_each(|h| store.get_account_code(h))is agetper hash, capped at core count in flight, where the accounts alongside it already collapse into a single batched read. There was no batched bytecode read on theDatabasetrait at all.Description
get_account_codes_batchon the VMDatabasetrait, default impl loopingget_account_codeso non-rocksdb backends are unaffected, threaded throughDatabaseLogger,DynVmDatabase,CachingDatabaseandStoreVmDatabase.Store::get_account_codes_batchresolves the block-data buffer and the code LRU first, then reads what is left. Cold reads here are latency-bound, so the read path is chosen by which mechanism gets more requests in flight for the batch at hand: a rayon fan-out of point gets reaches queue depth ~= core count for free, while contiguous shards of the sorted keys on blocking threads reach queue depth ~= shard count and share RocksDB blocks within a shard, but pay a spawn each. It shards only once the batch forms more 256-key shards than there are cores, so ordinary blocks keep the fan-out and bloated ones get the depth. Each shard decodes its own values, keeping the jumpdest bitmap rebuild parallel. The LRU is locked once per batch rather than twice per code.warm_block_from_balwalks the access list in chunks instead of two whole-block phases, so code reads start after the first slice of addresses. The chunk is256 * 64, the smallest that still saturates the shard path, so chunking cannot trade the executor's head start for shallower reads.CachingDatabase::get_account_states_batchreturns the states it warms, which removes aget_account_stateper access-list account (~115k on a bloated block) that existed only to recover hashes Phase 1 had already fetched.Notes
DatabaseLoggerrecords every hash in the batch before delegating, matching the per-hash path, and the warmer does not run when witness collection is on.prefetch_accountskeeps its own implementation rather than delegating to the states batch, because its other caller passes a whole write set and would pay for a result vector it discards.shard_counthelper and adds the storage-side batch this mirrors. The constants are local here to keep this independent of that PR; worth folding into the helper whenever the two meet.MultiGetbatches blob-backed values, which only a run on the bloated fixtures will show.warmer/execon heavy blocks is the direct readout: if that median does not drop below 1.0 with real slack, the batching was not the constraint.How was it tested?
test/tests/storage/account_code_batch_tests.rschecks the batched read against the per-hash read the executor uses, since the prefetch warms a cache execution then trusts: same bytecode and same jumpdest bitmap per hash,Nonefor absent hashes, results aligned to the caller's order rather than the internal read order, and every position of a hash repeated within one batch. Sized fromavailable_parallelismso the parity check crosses into the sharded path, with a small-batch case covering the fan-out.Plus the workspace suites and
-D warningsclippy.