perf(core): drop the per-state-read heap allocations in the witness path - #198
Conversation
`WitnessDatabase` reads go through salt's `find()` with in-place decoding and stack-allocated key encodings; `LightWitness::metadata` stops cloning the `SaltValue` it only reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CHVpMMX9N69sUNbuKgpBVY
Claude review status
✅ Review clean Last reviewed: head New this round: 0 finding(s), 0 question(s) · Resolved this round: 0 · Open questions: 0 |
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50741dc320
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| EphemeralSaltState::new(self.witness) | ||
| .plain_value(plain_key) | ||
| .find(plain_key) | ||
| .map(|found| found.map(|(_, salt_value)| salt_value)) |
There was a problem hiding this comment.
When W is LightWitnessExecutor in the debug-trace server, the old plain_value path can use StateReader::plain_value_fast, which is backed by the prebuilt direct_lookup_tbl in light_witness.rs; invoking EphemeralSaltState::find directly bypasses that index and probes the SALT table for every account and slot read. On trace replays with many state reads, this discards the executor's purpose-built fast path and may outweigh the saved value allocation. Resolve the SaltKey through plain_value_fast and read the SaltValue directly, retaining the find fallback needed for non-existent keys.
Useful? React with 👍 / 👎.
1 participant
Summary
PR 5/6 of the #170 split, stacked on #197. Removes the heap allocations on the per-state-read path of block replay. Listed in vincent's review of #170 as "A hot-path optimization" worth its own line.
This lookup runs once per unique account and storage slot touched by a block, so it is the hottest thing in
WitnessDatabase.WitnessDatabasereads go through salt'sfind(), which hands back theSaltValue(an inline array) instead ofplain_value()'s freshly heap-allocated copy of the value bytes. Callers decode from.value()in place.PlainKey::account_key_bytes(a borrow of the address) andPlainKey::storage_key_bytes(aFixedBytes<52>) replacePlainKey::{Account,Storage}(..).encode()'sVec.encode()itself delegates to them, so there is one definition of each encoding.bucket_id_for_account/bucket_id_for_slotuse them too — that is one allocation per call removed on another per-read path.LightWitness::metadatastops cloning theSaltValueit only reads.Net: two heap allocations per witnessed state read, plus one per
bucket_id_for_*call.Correctness
The interesting claim is that
find()returns the same bytesplain_value()did. That is pinned by the mainnet fixture replays (validate_block_mainnet_fixtures,validate_block_deriving_updates_mainnet_fixtures,replayed_gas_used_matches_the_mainnet_header): a read that returned different bytes changes execution and fails the post-state-root check on every fixture.stack_key_encodings_match_encodepins the narrower claim — that the allocation-free encoders agree withencode()— against a future edit that stops delegating and lets the two drift into looking up different keys.Testing
cargo fmt --all --check,cargo clippy --workspace --all-targets --all-features(0 warnings),cargo sort --check, full workspace suite 482 passed / 0 failed,cargo test -p stateless-core --no-default-features --lib --no-runclean.