commitment: GetAsOf on-disk shortcut missing — drives ~2 GiB unnecessary history-file I/O on SSTORE-bloated benchmark

[mh0lt]

Summary

The parallel commitment calculator's HistoryStateReader.Read (execution/commitment/commitmentdb/reader.go:71-72) calls roTx.GetAsOf(d, plainKey, limitReadAsOfTxNum) for sibling account/storage lookups. This is correct for in-memory consistency — the calculator runs concurrently with execution and must read sd.mem at a fixed txNum boundary to avoid seeing future state from concurrent writers.

What's missing: when the lookup misses sd.mem and falls through to disk, GetAsOf still walks the history segment files first (db/state/history.go:1228-1236, historySeekInFiles before historySeekInDB). For the on-disk path, GetLatest semantics would give equivalent results without the history-file walk — there are no concurrent writers in the file/MDBX tier.

This is NOT a regression — GetAsOf was correctly added to support concurrent commitment+execution. It is missing functionality — the internal shortcut to use GetLatest semantics for the on-disk path when no concurrent-writer concern applies.

Measurement (issue #20920 SSTORE-bloated benchmark)

Test: test_sstore_bloated[10GB-fork_Osaka-NO_CACHE-existing_slots_True-write_new_value_True-30M] on perf-devnet-3 block 24358306, no docker / no overlayfs, direct erigon launch with parallel exec.

Per-mmap'd-file Rss delta during the test phase (positive = pages brought in from disk):

File	Rss delta
v2.0-storage.7472-7480.ef (storage history)	+1.29 GiB
v2.0-commitment.7488-7492.kvi (commit index)	+306 MiB
v3.0-storage.7480-7481.ef (storage history)	+290 MiB
v3.0-storage.7481-7482.ef (storage history)	+285 MiB
v2.0-commitment.7492-7494.kvi	+188 MiB
v3.0-storage.7482-7483.ef (storage history)	+160 MiB
v2.0-storage.7488-7492.kv (latest values)	+152 MiB
v2.0-commitment.7494-7495.kvi	+128 MiB
v2.0-commitment.7488-7492.kv	+106 MiB
v3.0-storage.7483-7484.ef (storage history)	+54 MiB
chaindata mdbx.dat	+32 MiB

Storage history .ef files: ~2.08 GiB of the test-phase I/O. chaindata MDBX is trivial (32 MiB) — confirms the MDBX-first GetLatest path IS fast for recent writes; the I/O cost is from the GetAsOf path going files-first.

Branch-invariance confirmed by running on both exec3/remove-rwtx-threading-merge-main and bal-devnet-3 — same files dominate, same per-IOP granularity (~4 KB).

Architectural target

Long-term, callers should ALWAYS use GetAsOf as the API. The smarts go inside GetAsOf: detect when (key, txNum) has no concurrent-writer / no-later-version concern and shortcut to the GetLatest path internally. Same correctness, no API churn for callers.

The internal shortcut needs a cheap way to know "is there any history write for this key newer than the txNum boundary?" If no, GetLatest is equivalent. If yes, walk history. May need a per-key/per-domain watermark or a fast bloom over recent history writes — non-trivial.

Short-term workaround for the SSTORE-bloat measurement

In the calculator's state reader, gate GetAsOf on whether sd.mem has the key (needed for in-memory consistency vs concurrent writers); on sd.mem miss, use GetLatest (MDBX-first → files only on miss). This is the cheap patch that confirms the I/O hypothesis without rewriting GetAsOf semantics. Production fix is the smart-shortcut above.

References

• Code: execution/commitment/commitmentdb/reader.go:49-81 (HistoryStateReader)
• Code: db/state/domain.go:1401-1440 (GetAsOf, files-first)
• Code: db/state/history.go:1223-1294 (HistorySeek, files-first vs DB)
• Related: Erigon trails geth/besu/nethermind by ~1.5–2× on cold-SSTORE-bloated workloads; planned fix blocked by --experimental.concurrent-commitment wrong-trie-root bug #20920 (SSTORE-bloated benchmark gap to geth/besu/nethermind)

[AskAlexSharov]

I guess: HistorySeek reading in files first - to avoid seen garbage left after new Prune.
But in my head: maybe easy to read from db first, if possible post-filter by dt.files.EndTxNum()
(Domain.getLatest does post-filter by dt.files.EndTxNum())
cc: @sudeepdino008

[VBulikov]

yperbasis added Imp2 as Importance