test(benchmark): flat-state base point-read benchmark (RocksDB vs sorted-arena vs LMDB) - #12684
test(benchmark): flat-state base point-read benchmark (RocksDB vs sorted-arena vs LMDB)#12684LukaszRozmej wants to merge 1 commit into
Conversation
…ena vs LMDB) Phase-0 go/no-go gate for the MDBX-like flat persistence workstream: a standalone micro-benchmark comparing cold/warm uniform-random point reads (account 20B keys, storage-slot 52B split keys; hits and guaranteed misses; 1/8/32 threads) across three backends holding byte-identical synthetic data: - production-tuned RocksDB flat columns (default DbConfig Flat* options plus the 300/700 MiB Account/Storage HyperClockCaches mirroring FlatRocksDbConfigAdjuster); - a sharded SortedTable arena prototype (256 prefix shards in one mmap'd ArenaFile, in-memory shard directory) reusing the persisted-snapshot sorted-table format via verbatim copies of the internal Sorted/* files (no InternalsVisibleTo for benchmarks); - LMDB via the LightningDB package (benchmark-only dependency). The deterministic dataset builder (NETH_FLATBENCH_SCALE=smoke|full, NETH_FLATBENCH_DIR) bulk-loads all three backends in ascending key order, is reused across runs via a parameter marker, and every benchmark setup re-validates sampled keys byte-for-byte on the selected backend. State/FlatBase/README.md documents the full-scale procedure, cold-run methodology (drop_caches), metrics to record, and the go/no-go thresholds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
core-repository-dispatch-app
Bot
added
build changes
dependencies
|
Claude finished @LukaszRozmej's task in 10m 50s —— View job Review: flat-base point-read benchmarkReviewed the full diff against I verified the parts most likely to be silently wrong, and they hold up:
The findings below are about the benchmark's methodology rather than its correctness — which is the thing that matters most here, since the numbers feed a go/no-go gate. High1. The Medium2. Warm mode measures a 32 K hot key set, not random access. (inline) 3. A marker mismatch silently 4. The 7 copied Low5. The Only #1 seems worth blocking on, since it can change the answer the benchmark is meant to produce; #2–#4 are cheap and should land before the Note on verification: · branch |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| int perThread = TotalReads / Threads; | ||
| long[] sums = _threadSums; | ||
| Parallel.For(0, Threads, t => | ||
| { | ||
| using IFlatReadSession session = _backend.BeginSession(); | ||
| sums[t] = ReadRange(session, start + t * perThread, perThread, slots); | ||
| }); |
There was a problem hiding this comment.
High — the Threads = 32 row does not actually run 32 concurrent readers
Parallel.For(0, Threads, …) schedules onto the ThreadPool with a range partitioner: for 32 indices it hands each pool worker a contiguous chunk that the worker runs sequentially. So the achieved concurrency is bounded by the number of live pool threads, which starts at ProcessorCount and only grows via the starvation injector at roughly 1 thread per 0.5–1 s.
Each worker here does TotalReads / Threads = 256 reads. Even at a pessimistic 100 µs cold latency that's ~25 ms of work per worker — the injector never fires. On a 16-core host the Threads = 32 row measures ~16-way concurrency; under the README's single-shot cold procedure (--warmupCount 0 --iterationCount 1 --invocationCount 1) the pool has had no chance to ramp at all, so it is worse.
This matters because concurrent read scaling is exactly the property the go/no-go compares (arena ≥ 0.85× LMDB) — LMDB's headline characteristic is lock-free MVCC reader scaling, and silently capping every backend at ProcessorCount can flip the decision.
Two secondary effects of the same call:
[MemoryDiagnoser]will attribute theParallel.Forrange partitioner / task / closure allocations to the benchmark, so the "zero per-read allocations" property only holds for theThreads = 1row.- The workers do not start together, so there is no steady-state 32-way phase to measure even if the pool did ramp.
Suggested fix: create Threads dedicated Threads once in [GlobalSetup], park them on a Barrier, and have Run release/join them. A cheaper stopgap is ThreadPool.SetMinThreads(Threads + Environment.ProcessorCount, …) plus new ParallelOptions { MaxDegreeOfParallelism = Threads } — but that still doesn't guarantee 32 simultaneous in-flight reads, so dedicated threads are the honest option. Either way, it would be worth recording the observed peak concurrency so a throttled run is visible in the results rather than silent.
| private const int TotalReads = 8192; | ||
| private const int PoolSize = 1 << 15; | ||
| private const int PoolMask = PoolSize - 1; |
There was a problem hiding this comment.
Medium — the 32 K key pool makes "warm" mode a 32 K-hot-key benchmark, not a random-access one
_cursor advances by TotalReads (8192) per invocation and indexes the pool with & PoolMask (32767), so the pool fully wraps every 4 invocations.
- Cold mode is fine: with
--warmupCount 0 --iterationCount 1 --invocationCount 1only jitting + one measured invocation run, so the measured 8192 keys are distinct from the jitted ones and never repeat. Good design. - Warm mode (the first README command, and the source of the indicative numbers in the PR description) is not. BDN's pilot will pick ~150 invocations/iteration at ~3 ms each, so the same 32768 keys are re-read ~37× per iteration. At
fullscale that is a working set of roughly 32768 × 4 KiB ≈ 130 MB — it fits entirely in the OS page cache and inside the 300 MiB Account HyperClockCache. So the "warm" row measures a tiny hot set, not warm random access over 300 M accounts, and it compresses exactly the differences the benchmark exists to expose.
PoolSize is cheap to raise: 4 M entries costs ~290 MB for the slot pool and ~80 MB for the account pool, which is nothing next to the documented "tens of GB of RAM headroom". Alternatively derive the key from the global read index instead of a fixed pool.
At minimum the README's Warm section should state that warm numbers reflect a 32 K hot key set regardless of dataset scale, so nobody reads them as a warm-at-scale signal.
| public static FlatBaseDatasetSpec EnsureBuilt() | ||
| { | ||
| FlatBaseDatasetSpec spec = FlatBaseDatasetSpec.FromEnvironment(); | ||
| if (File.Exists(spec.MarkerPath) && File.ReadAllText(spec.MarkerPath) == spec.MarkerText) | ||
| return spec; | ||
|
|
||
| if (Directory.Exists(spec.DatasetDir)) | ||
| Directory.Delete(spec.DatasetDir, recursive: true); | ||
| Directory.CreateDirectory(spec.DatasetDir); | ||
|
|
||
| Console.WriteLine($"[FlatBase] Building dataset: {spec.MarkerText} under {spec.DatasetDir}"); | ||
| Build(spec); | ||
| File.WriteAllText(spec.MarkerPath, spec.MarkerText); | ||
| Console.WriteLine("[FlatBase] Dataset build complete"); | ||
| return spec; | ||
| } |
There was a problem hiding this comment.
Medium — a marker mismatch silently rm -rfs a multi-hour, ~400 GB dataset from inside [GlobalSetup]
EnsureBuilt is called from Setup(), and any marker mismatch (or a build that was interrupted, leaving no marker) unconditionally does Directory.Delete(spec.DatasetDir, recursive: true) before rebuilding. At full scale the README says the build takes hours and needs ≥ 100 GB per backend, so:
- Ctrl-C'ing a build, then re-running, deletes everything and starts from zero — there is no resume and no warning.
- A
FormatVersionbump or a strayNETH_FLATBENCH_SCALEtypo triggers the same, from what looks like a routinedotnet runbenchmark command.
Given the documented cost, this deserves a guard: print what is about to be deleted (path + size) and require an explicit opt-in such as NETH_FLATBENCH_REBUILD=1 before deleting a non-empty existing dataset directory, otherwise throw with the command to run. The smoke path can keep the current behaviour.
Related, smaller: the marker is written only after all three backends are built and validated, so there is no way to distinguish "never built" from "half built" — recording per-backend markers would let an interrupted run skip the backends that already completed.
| // A hit pool read must return data and a miss pool read must not — fail loudly, not measure garbage. | ||
| using IFlatReadSession session = _backend.BeginSession(); | ||
| Span<byte> buffer = stackalloc byte[256]; | ||
| int length = session.GetAccount(_accountKeys.AsSpan(0, FlatBaseBenchmarkDatasetBuilder.AccountKeyLength), buffer); | ||
| if (Miss ? length != 0 : length == 0) | ||
| throw new InvalidOperationException($"Key pool sanity check failed: Miss={Miss}, first account read returned {length} bytes"); |
There was a problem hiding this comment.
Low — the pool sanity check only covers the account pool
The comment says "A hit pool read must return data and a miss pool read must not", but only _accountKeys[0] is probed. SlotPointRead is half the benchmark and the slot pool uses a different derivation (miss = live account + slotIndex ≥ SlotsPerAccount), so a regression there would silently make the Miss=false slot row measure all-misses (or vice versa) with no failure. Validate does not close this gap — it recomputes its own keys rather than reading the pool.
Two extra lines mirroring the account probe with session.GetSlot(_slotKeys.AsSpan(0, StorageKeyLength), buffer) would cover it.
| // Verbatim copy of Nethermind.State.Flat/PersistedSnapshots/Sorted/SortedTableReader.cs (internal in | ||
| // Nethermind.State.Flat, which has no InternalsVisibleTo for benchmarks). Benchmark-scoped | ||
| // prototype code — keep in sync with the original; do not diverge. |
There was a problem hiding this comment.
Medium — "verbatim copy … do not diverge" is not mechanically checkable, and the copies already diverge
The PR asks for a reviewer preference here, so: a third option beats both copying and InternalsVisibleTo — link the originals into the benchmark project without duplicating them:
<Compile Include="..\Nethermind.State.Flat\PersistedSnapshots\Sorted\*.cs"
Exclude="..\Nethermind.State.Flat\PersistedSnapshots\Sorted\SortedTableEnumerator.cs"
Link="State\FlatBase\Sorted\%(Filename)%(Extension)" />That keeps production visibility unchanged (the linked types compile as internal into the benchmark assembly, and the referenced assembly's own internals stay invisible), removes the drift risk entirely, and drops ~700 lines from the diff. Worth at least trying before merging 7 copies.
If copies do stay, the "keep in sync; do not diverge" instruction needs to be enforceable, and right now it isn't:
- The copies strip the
<remarks>blocks fromBlock.cs,SortedTable.csandSortedTableBuilder.cs(the wire-format documentation — the single most valuable thing to keep in sync), and dropTryReadRecordRangefrom both block readers. Sodiffagainst the originals is already noisy and a real behavioural drift would be indistinguishable from the intentional edits. - Nothing fails when the original changes. Consider a small test in
Nethermind.State.Flat.Testthat asserts the copies match the originals modulo the header/namespace lines, so a future format change to the productionSortedTablebreaks the build instead of silently invalidating a benchmark that is being used to make an architectural go/no-go call.
| DbConfig dbConfig = new(); | ||
| RocksDbConfigFactory baseFactory = new( | ||
| dbConfig, new PruningConfig(), new TestHardwareInfo(), NullLogManager.Instance); | ||
| CacheInjectingConfigFactory factory = new(baseFactory, _accountCache, _storageCache); | ||
|
|
||
| _db = new ColumnsDb<FlatDbColumns>( | ||
| basePath, new DbSettings("Flat", "flat"), dbConfig, factory, NullLogManager.Instance, | ||
| Enum.GetValues<FlatDbColumns>()); | ||
| } |
There was a problem hiding this comment.
Low — worth documenting how faithful the "production-tuned" baseline actually is
I checked the two places where this could silently diverge from production and both turn out fine, but they are non-obvious enough that a one-line comment would save the next reader the same walk:
new TestHardwareInfo()reportsAvailableMemoryBytes = 10 MB.RocksDbConfigFactoryonly consults that insideif (databaseName.StartsWith("State")), and this DB is named"Flat", so the low value has no effect here. ✅MaxOpenFilesLimitisnull, so the auto-cap branch is skipped andMaxOpenFilesstays unset (RocksDB-1). That matches production on a typical Linux server (ulimit ≫ 10000 ⇒ "leave unlimited"), so it is right for the target host — but it does mean all SST index and filter blocks are preloaded at DB open. For a ≥ 100 GB dataset that is several GB pinned outside the 1 GiB block cache, and it means a post-drop_cachesrun still measures RocksDB with warm indexes/filters after open. That is generous to RocksDB, so it is the conservative direction for aarena ≥ 1.8× RocksDBgate — but it should be stated in the README next to "Peak RSS per backend", because RocksDB's real memory in that run will be well above the 1 GiB block-cache budget listed there.- Also note
FlatDbConfig.Layoutdefaults toFlatLayout.Flat, soFlatRocksDbConfigAdjuster'sadditionalConfigis""in production too — matching the""passed on line 127. Worth a comment, since a future default flip toFlatInTriewould make this mirror wrong without any compile error.
Minor: constructing the 1 GiB of HyperClockCaches on the bulk-load instance in FlatBaseBenchmarkDatasetBuilder.Build costs a GiB during an already memory-hungry build for no benefit; a separate small-cache write path would be cheap.
| public static SortedArenaBackend OpenRead(string dir) => new( | ||
| ShardedArenaReader.Open( | ||
| Path.Combine(dir, ShardedSortedTableArena.AccountArenaFile), | ||
| Path.Combine(dir, ShardedSortedTableArena.AccountDirFile)), | ||
| ShardedArenaReader.Open( | ||
| Path.Combine(dir, ShardedSortedTableArena.StorageArenaFile), | ||
| Path.Combine(dir, ShardedSortedTableArena.StorageDirFile))); |
There was a problem hiding this comment.
Low — the account ArenaFile leaks if opening the storage arena throws
If the second ShardedArenaReader.Open fails (missing/truncated storage.dir, a short storage.arena, mmap failure at full scale), the already-constructed account reader is never disposed, so its mmap view and SafeFileHandle stay alive until finalization. Same shape in LmdbFlatBackend.Open if the second tx.OpenDatabase throws, and in FlatBaseBenchmarkDatasetBuilder.Build where a throw partway through the using chain is handled correctly but a throw inside ShardedArenaWriter's constructor after the FileStream is opened is not.
Benchmark-scope, so low impact, but at full scale a leaked 130 GB mmap during a failed setup is not free, and .agents/rules/robustness.md asks for using on all IDisposables. A try { … } catch { accounts.Dispose(); throw; } in OpenRead covers it.
| ## Go/no-go thresholds (from planning) | ||
|
|
||
| On cold uniform-random reads at the `full` scale (≥ 100 GB per backend): | ||
|
|
||
| - **Go** if the arena is ≥ **1.8× RocksDB** on cold random hits **and** ≥ **0.85× LMDB**; | ||
| - miss cost must be ≤ **1.2× LMDB** (the arena has no bloom filters — misses still binary-search a | ||
| shard; if this fails, Phase 1 adds a per-shard filter before any production work); | ||
| - otherwise **no-go**: stop the flat-base workstream and record the numbers. |
There was a problem hiding this comment.
Low — state the "fresh bulk load" caveat next to the thresholds
Both the arena and RocksDB are measured immediately after a perfect, strictly-ascending bulk load (RocksDB additionally gets Flush → Compact → Flush), so both are in a best-case shape that a live node never sees. That is symmetric and therefore fair as a relative comparison, but the numbers are an upper bound on both backends, and the arena's advantage in particular comes partly from being a single perfectly-packed sorted image per shard — the property a production base tier would have to maintain through compaction.
A sentence under "Go/no-go thresholds" saying the gate measures a freshly-compacted steady state, and that write amplification / compaction cost is explicitly out of scope for Phase 0, would stop the ratio being read as a production speedup.
Two smaller README items:
- Total disk for the
fullrun is closer to ~400 GB across the three backends (arena alone: ~28 GB accounts + ~102 GB storage), not "≥ 100 GB per backend" read as ~300 GB. Worth being concrete, givenLmdbMapSizealsoftruncates a ~403 GB sparse file. - "Peak RSS per backend (… RocksDB holds a 1 GiB block cache)" understates RocksDB: with
max_open_filesunlimited it also pins every SST's index and filter blocks outside that budget.
1 participant
Changes
Phase 0 gate for evaluating an mmap-based (MDBX-like) base tier for flat state: a standalone micro-benchmark comparing cold/warm random point reads across three backends, before committing to any production backend work.
FlatBaseBenchmarkDatasetBuilder: deterministic synthetic dataset (keccak(counter) keys, realistic slim-RLP account values, 52-byte split-key slots) built once and reused via marker files. Scale viaNETH_FLATBENCH_SCALE:smoke(100k accounts / 500k slots, local-friendly) orfull(300M / 1.2B, ≥100 GB, for a Linux box). Root dir viaNETH_FLATBENCH_DIR.FlatBasePointReadBenchmark: BenchmarkDotNet, backend × {hit, guaranteed-miss} × threads {1, 8, 32}, account (20B) and slot (52B) reads, zero per-read allocations.DbConfigFlat* option strings and 300/700 MiB HyperClockCaches mirroringFlatRocksDbConfigAdjuster— the honest baseline;SortedTableformat over productionArenaFilemmap (the candidate base-tier design);LightningDB(added to CPM, referenced only by Nethermind.Benchmark) — the reth-configuration control.drop_cachesper pass), the metrics to record, and the go/no-go thresholds: arena ≥1.8× RocksDB on cold hits at ≥100 GB AND ≥0.85× LMDB; miss cost ≤1.2× LMDB.Indicative warm smoke numbers on Windows (mean ns/read, account hits, 1 thread): RocksDB ~1700, arena ~420, LMDB ~230; at 32 threads: 224 / 43 / 49. The decision numbers require the documented cold full-scale Linux run.
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing
Benchmarks.slnxrelease build: 0 warnings.Sorted/*files are verbatim copies of internalNethermind.State.Flatsorted-table machinery (header-noted) — copied rather than widening production visibility withInternalsVisibleTo. If reviewers preferInternalsVisibleTo, happy to switch.Documentation
Requires documentation update
Requires explanation in Release Notes
Remarks
Part of the BAL benchmark performance workstream (#12681, #12682, #12683). Next step: run the
fullscale cold procedure on a Linux benchmark host; the thresholds above decide whether the sorted-arena base tier (or an LMDB/libmdbx backend behind the sameIPersistenceseam) proceeds to a production prototype.🤖 Generated with Claude Code