Open
Conversation
asdacap
force-pushed
the
perf/simple-flat
branch
from
1b0746e to
ef809c6
Compare
Member
Seems to continue fine; but not happy if restarted |
Member
|
Maybe Pruning, unless it doesn't output a compete message? |
Member
|
Different source continuing |
Contributor
Author
|
Yea, dont worry about that for now. That was for debugging before. The pruning is the trie cache pruning. It uses some shards of concurrent dictionary and clear them shard by shard until the total memory is under budget. |
asdacap
force-pushed
the
perf/simple-flat
branch
from
4e946fa to
87a1777
Compare
asdacap
force-pushed
the
perf/simple-flat
branch
from
9cede2c to
ffe9bb7
Compare
asdacap
force-pushed
the
perf/simple-flat
branch
3 times, most recently
from
e1ded6d to
0a87340
Compare
12 tasks
This was referenced Jan 19, 2026
asdacap
force-pushed
the
perf/simple-flat
branch
4 times, most recently
from
a0e4951 to
0deb695
Compare
asdacap
force-pushed
the
perf/simple-flat
branch
2 times, most recently
from
b1b7a35 to
5ff55a4
Compare
asdacap
requested review from
a team,
LukaszRozmej,
MarekM25,
benaadams,
damian-orzechowski,
flcl42 and
rubo
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive FlatDB storage architecture as an alternative to the existing pruning trie store. The implementation provides ~20% performance improvement over HalfPath with three different persistence layouts (Flat, FlatInTrie, PreimageFlat) to support different hardware configurations.
Changes:
- Adds complete FlatDB implementation with snapshot-based state management, compaction, and trie node caching
- Introduces new persistence abstraction layer supporting multiple storage layouts
- Adds trie warming infrastructure and ring buffer concurrency primitives
- Integrates FlatDB into DI container with configuration support
Reviewed changes
Copilot reviewed 118 out of 119 changed files in this pull request and generated 63 comments.
Show a summary per file
| File | Description |
|---|---|
| Nethermind.State.Flat/* | New FlatDB core implementation including snapshots, persistence, scope providers, and trie integration |
| Nethermind.Trie/* | Extended trie functionality with warmup paths, leaf iteration, and child reference control |
| Nethermind.State/* | Modified state management to support pluggable world state backends |
| Nethermind.Db/* | Added FlatDB configuration and database layout enums |
| Nethermind.Init/Modules/* | DI registration for FlatDB components and RocksDB tuning |
| Nethermind.Core/* | New utility classes for ref counting, read-write locks, and concurrency control |
| Tests | Test infrastructure updates and new test suites for FlatDB components |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+96
to
+94
| foreach (StateId id in _snapshotRepository.GetStatesAtBlockNumber(blockNumber - _compactSize)) | ||
| { | ||
| if (_snapshotRepository.RemoveAndReleaseCompactedKnownState(id)) | ||
| { | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
Comment on lines
246
to
253
| if (path.Length <= ShortenedPathThreshold) | ||
| { | ||
| return storageNodes.Get(EncodeShortenedStorageNodeKey(stackalloc byte[ShortenedStorageNodesKeyLength], address, in path), flags: flags); | ||
| } | ||
| else | ||
| { | ||
| return fallbackNodes.Get(EncodeFullStorageNodeKey(stackalloc byte[FullStorageNodesKeyLength], address, in path), flags: flags); | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement return - consider using '?' to express intent better.
Comment on lines
232
to
241
| if ((ulong)numBlocks <= uint.MaxValue) | ||
| { | ||
| // FastRange32-style: floor(h1 * numBlocks / 2^32) | ||
| block = (((ulong)h1 * (ulong)(uint)numBlocks) >> 32); | ||
| } | ||
| else | ||
| { | ||
| // 64-bit multiply-high: floor(x * numBlocks / 2^64) | ||
| block = (ulong)(((UInt128)x * (ulong)numBlocks) >> 64); | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Comment on lines
+326
to
+333
| if (value is null || Bytes.AreEqual(value, StorageTree.ZeroBytes)) | ||
| { | ||
| _changedSlots[(address, index)] = null; | ||
| } | ||
| else | ||
| { | ||
| _changedSlots[(address, index)] = SlotValue.FromSpanWithoutLeadingZero(value); | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Comment on lines
63
to
70
| if (address is null) | ||
| { | ||
| shardIdx = path.Path.Bytes[0]; | ||
| } | ||
| else | ||
| { | ||
| shardIdx = address.Bytes[0]; | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Comment on lines
54
to
64
| if (_printNodes) | ||
| { | ||
| return $"{_operationName,-25} {percentage.ToString("P2", CultureInfo.InvariantCulture),8} " + | ||
| Progress.GetMeter(percentage, 1) + | ||
| $" nodes: {workStr,8}"; | ||
| } | ||
| else | ||
| { | ||
| return $"{_operationName,-25} {percentage.ToString("P2", CultureInfo.InvariantCulture),8} " + | ||
| Progress.GetMeter(percentage, 1); | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement return - consider using '?' to express intent better.
asdacap
force-pushed
the
perf/simple-flat
branch
from
22de046 to
6eab9d0
Compare
benaadams
reviewed
Jan 27, 2026
asdacap
force-pushed
the
perf/simple-flat
branch
from
315bcdd to
35e5c1d
Compare
Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk>
Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk>
Contributor
Author
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Contributor
14 tasks
flcl42
reviewed
Jan 28, 2026
| /// Make storing slot value smaller than a byte[]. | ||
| /// </summary> | ||
| [StructLayout(LayoutKind.Sequential, Pack = 32, Size = 32)] | ||
| public struct SlotValue |
Contributor
There was a problem hiding this comment.
Might be inline array, not a big diff though
flcl42
reviewed
Jan 28, 2026
Comment on lines
+71
to
+74
| /// <summary> | ||
| /// Disposes it once, decreasing the lease count by 1. | ||
| /// </summary> | ||
| public void Dispose() => ReleaseLeaseOnce(); |
Member
There was a problem hiding this comment.
A bit weird semantics.
Normally Dispose i dispose and we are done with the object.
Maybe Aquire should return something disposable that would decrease the count?
Contributor
Author
There was a problem hiding this comment.
Maybe dont know. This is from paprika btw.
Comment on lines
21
to
22
| // Temporary disable to see if it fix crash | ||
| // RocksDbSharp.Native.Instance.rocksdb_cache_destroy(handle); |
Contributor
Author
There was a problem hiding this comment.
Actually it still crash even with this comment
src/Nethermind/Nethermind.State.Flat/Persistence/BaseFlatPersistence.cs
Outdated
Show resolved
Hide resolved
src/Nethermind/Nethermind.State.Flat/ScopeProvider/TrieWarmer.cs
Outdated
Show resolved
Hide resolved
| FlatLayout.Flat => ctx.Resolve<RocksDbPersistence>(), | ||
| FlatLayout.FlatInTrie => ctx.Resolve<FlatInTriePersistence>(), | ||
| FlatLayout.PreimageFlat => ctx.Resolve<PreimageRocksdbPersistence>(), | ||
| _ => throw new Exception($"Unsupported layout {flatDbConfig.Layout}") |
Comment on lines
145
to
146
| HashSet<Address> addressToClear = new HashSet<Address>(); | ||
| HashSet<Hash256AsKey> addressHashToClear = new HashSet<Hash256AsKey>(); |
Member
There was a problem hiding this comment.
Looks like big opportunity for some pooling?
We can used PooledCollections which has pooled hash set.
Contributor
There was a problem hiding this comment.
(If you mean https://github.com/jtmueller/Collections.Pooled, we need to add it back)
* Some initial refactors and optimizations * more
16 tasks
Contributor
…eModule (#10346) * Initial plan * Replace generic Exception with NotSupportedException Co-authored-by: asdacap <1841324+asdacap@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: asdacap <1841324+asdacap@users.noreply.github.com>
flcl42
reviewed
Jan 29, 2026
* Parallelize FlatTrieVerifier with partition-based verification Partition the 256-bit key space into 8 ranges for parallel account co-iteration in hashed mode. Add bounded iterator support to IPersistenceReader and TrieLeafIterator. Add HashVerifyingTrieStore wrapper for hash integrity checks during verification. Improve diagnostic logging for trie path traversal issues. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add parameterless iterator defaults to IPersistenceReader Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Revert 3 files to use parameterless iterator overloads NoopPersistenceReader, RefCountingPersistenceReader, and PersistenceScenario don't need bounded signatures — the default interface methods on IPersistenceReader handle the dispatch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Merge TrieLeafIterator constructors and fix build errors Combine two nearly-identical constructors into one with optional startPath/endPath parameters. Fix NoopPersistenceReader and RefCountingPersistenceReader to implement the required ranged iterator overloads. Remove unused System.Linq import from tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Simplify TrieLeafIterator comparators using TreePath bound methods Replace ~80 lines of manual nibble-level byte comparison logic with TreePath.ToUpperBoundPath()/ToLowerBoundPath() one-liners. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix spelling: Childs -> Children, simplify GetStartChildIndex Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove redundant CompareWithCorrectLength after upstream fix The upstream fix in 190f688 corrected Bytes.BytesComparer.Compare to use SequenceCompareTo, making CompareWithCorrectLength redundant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
IPersistencefor very different db scheme.Note
Design
FlatDbManagerIPersistencewhich represent the persistence layer.Snapshot, backed bySnapshotContent.TrieNodeCachewhich is a specialTrieNode... cache. It stores the node in a sharded hashtable array. This cache is addressed by both path and hash. Its put in front because its updated on the head instead of on persist. But its reorg safe as the trie node still need keccak to match.ReadOnlySnapshotBundleis a bundle of snapshot.StateReaderand the various world state. Its read only, refcounted and shared.ReadOnlySnapshotBundleto have a small-ish number of layers to traverse which is about 10, instead of 128 on mainnet. You can make it lower at expense of about 50% of a full CPU core and some good amount of memory, but it does not look like it help much.ReadOnlySnapshotBundle.HintGetmethod is added to the world state scope interface which allow it to trigger the trie node warmer on what key it should warm up.WarmUpOutOfScopeis added to the existing scope provider interface which allow prewarmer to send key to trie node warmer from outside the scope (prewarmer kinda runs outside the main block processing).Persistence
Flat
<20-byte-hashed-address-prefix>(first 20 bytes of the hashed address).<4-byte-hashed-address-prefix><32-byte-hashed-slotindex><16-byte-hashed-address-suffix>(52 bytes total). The address hash is split to help RocksDB's comparator skip bytes during comparison and help with index key shortening.<3-byte-path>where the last 4 bits encode the path length.<8-byte-path>where the last 4 bits encode the path length.<4-byte-hashed-address-prefix><8-byte-path><16-byte-hashed-address-suffix>(28 bytes total).<0x00><32-byte-path><1-byte-length>(34 bytes total).<0x01><4-byte-address-prefix><32-byte-path><1-byte-length><16-byte-address-suffix>(54 bytes total).--FlatDb.BlockCacheSizeBudgetis set to the account and storage db at a 30/70 split.optimize_filters_for_hitsis set to false to keep last level bloom, which mean even more memory usage.PreimageFlat
FlatInTrie
StateNodesandStorageNodes(reusing the same columns as trie nodes). The key encoding is the same.optimize_filters_for_hitsis turned on. This significantly reduce memory at the expense of latency.optimize_filters_for_hitsmeans last level SST does not have bloom which slows down null slot check.Performance
Types of changes
Testing
Requires testing
If yes, did you write tests?