From 3fa85f0cf2335fdffc236c2054e8249baa7dc35d Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Sun, 6 Oct 2024 07:57:00 +0800 Subject: [PATCH 01/15] Mode dirty cache to its own file --- .../Pruning/TreeStoreTests.cs | 2 +- .../Pruning/HashAndTinyPath.cs | 34 ++ .../Pruning/HashAndTinyPathAndHash.cs | 37 ++ .../Nethermind.Trie/Pruning/TrieStore.cs | 522 ------------------ .../Pruning/TrieStoreDirtyNodesCache.cs | 481 ++++++++++++++++ 5 files changed, 553 insertions(+), 523 deletions(-) create mode 100644 src/Nethermind/Nethermind.Trie/Pruning/HashAndTinyPath.cs create mode 100644 src/Nethermind/Nethermind.Trie/Pruning/HashAndTinyPathAndHash.cs create mode 100644 src/Nethermind/Nethermind.Trie/Pruning/TrieStoreDirtyNodesCache.cs diff --git a/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs b/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs index a452d58a7ca..3a1677afd69 100644 --- a/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs +++ b/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs @@ -774,7 +774,7 @@ public void ReadOnly_store_returns_copies(bool pruning) readOnlyNode.Key?.ToString().Should().Be(originalNode.Key?.ToString()); } - private long ExpectedPerNodeKeyMemorySize => _scheme == INodeStorage.KeyScheme.Hash ? 0 : TrieStore.TrieStoreDirtyNodesCache.Key.MemoryUsage; + private long ExpectedPerNodeKeyMemorySize => _scheme == INodeStorage.KeyScheme.Hash ? 0 : TrieStoreDirtyNodesCache.Key.MemoryUsage; [Test] public void After_commit_should_have_has_root() diff --git a/src/Nethermind/Nethermind.Trie/Pruning/HashAndTinyPath.cs b/src/Nethermind/Nethermind.Trie/Pruning/HashAndTinyPath.cs new file mode 100644 index 00000000000..09138b330be --- /dev/null +++ b/src/Nethermind/Nethermind.Trie/Pruning/HashAndTinyPath.cs @@ -0,0 +1,34 @@ +// SPDX-FileCopyrightText: 2024 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System; +using System.Runtime.InteropServices; +using Nethermind.Core.Crypto; + +namespace Nethermind.Trie.Pruning; + +[StructLayout(LayoutKind.Auto)] +internal readonly struct HashAndTinyPath : IEquatable +{ + public readonly ValueHash256 addr; + public readonly TinyTreePath path; + + public HashAndTinyPath(Hash256? hash, in TinyTreePath path) + { + addr = hash ?? default; + this.path = path; + } + public HashAndTinyPath(in ValueHash256 hash, in TinyTreePath path) + { + addr = hash; + this.path = path; + } + + public bool Equals(HashAndTinyPath other) => addr == other.addr && path.Equals(in other.path); + public override bool Equals(object? obj) => obj is HashAndTinyPath other && Equals(other); + public override int GetHashCode() + { + var addressHash = addr != default ? addr.GetHashCode() : 1; + return path.GetHashCode() ^ addressHash; + } +} diff --git a/src/Nethermind/Nethermind.Trie/Pruning/HashAndTinyPathAndHash.cs b/src/Nethermind/Nethermind.Trie/Pruning/HashAndTinyPathAndHash.cs new file mode 100644 index 00000000000..56f3c2baff0 --- /dev/null +++ b/src/Nethermind/Nethermind.Trie/Pruning/HashAndTinyPathAndHash.cs @@ -0,0 +1,37 @@ +// SPDX-FileCopyrightText: 2024 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System; +using System.Runtime.InteropServices; +using Nethermind.Core.Crypto; + +namespace Nethermind.Trie.Pruning; + +[StructLayout(LayoutKind.Auto)] +internal readonly struct HashAndTinyPathAndHash : IEquatable +{ + public readonly ValueHash256 hash; + public readonly TinyTreePath path; + public readonly ValueHash256 valueHash; + + public HashAndTinyPathAndHash(Hash256? hash, in TinyTreePath path, in ValueHash256 valueHash) + { + this.hash = hash ?? default; + this.path = path; + this.valueHash = valueHash; + } + public HashAndTinyPathAndHash(in ValueHash256 hash, in TinyTreePath path, in ValueHash256 valueHash) + { + this.hash = hash; + this.path = path; + this.valueHash = valueHash; + } + + public bool Equals(HashAndTinyPathAndHash other) => hash == other.hash && path.Equals(in other.path) && valueHash.Equals(in other.valueHash); + public override bool Equals(object? obj) => obj is HashAndTinyPath other && Equals(other); + public override int GetHashCode() + { + var hashHash = hash != default ? hash.GetHashCode() : 1; + return valueHash.GetChainedHashCode((uint)path.GetHashCode()) ^ hashHash; + } +} diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs index 4ef602fa244..50d71a17d5f 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs @@ -3,21 +3,17 @@ using System; using System.Collections.Concurrent; -using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Nethermind.Core; -using Nethermind.Core.Caching; using Nethermind.Core.Collections; using Nethermind.Core.Crypto; using Nethermind.Core.Extensions; using Nethermind.Logging; -using CollectionExtensions = Nethermind.Core.Collections.CollectionExtensions; namespace Nethermind.Trie.Pruning { @@ -27,469 +23,6 @@ namespace Nethermind.Trie.Pruning /// public class TrieStore : ITrieStore, IPruningTrieStore { - internal class TrieStoreDirtyNodesCache - { - private readonly TrieStore _trieStore; - private int _count = 0; - private readonly ILogger _logger; - private readonly bool _storeByHash; - private readonly ConcurrentDictionary _byKeyObjectCache; - private readonly ConcurrentDictionary _byHashObjectCache; - - // Track some of the persisted path hash. Used to be able to remove keys when it is replaced. - // If null, disable removing key. - private readonly ClockCache? _pastPathHash; - - // Track ALL of the recently re-committed persisted nodes. This is so that we don't accidentally remove - // recommitted persisted nodes (which will not get re-persisted). - private Dictionary? _persistedLastSeen; - - public readonly long KeyMemoryUsage; - - public TrieStoreDirtyNodesCache(TrieStore trieStore, int trackedPastKeyCount, bool storeByHash, ILogger logger) - { - _trieStore = trieStore; - _logger = logger; - // If the nodestore indicated that path is not required, - // we will use a map with hash as its key instead of the full Key to reduce memory usage. - _storeByHash = storeByHash; - int initialBuckets = TrieStore.HashHelpers.GetPrime(Math.Max(31, Environment.ProcessorCount * 16)); - if (_storeByHash) - { - _byHashObjectCache = new(CollectionExtensions.LockPartitions, initialBuckets); - } - else - { - _byKeyObjectCache = new(CollectionExtensions.LockPartitions, initialBuckets); - } - KeyMemoryUsage = _storeByHash ? 0 : Key.MemoryUsage; // 0 because previously it was not counted. - - if (trackedPastKeyCount > 0 && !storeByHash) - { - _persistedLastSeen = new(); - _pastPathHash = new(trackedPastKeyCount); - } - } - - public void SaveInCache(in Key key, TrieNode node) - { - Debug.Assert(node.Keccak is not null, "Cannot store in cache nodes without resolved key."); - if (TryAdd(key, node)) - { - Metrics.CachedNodesCount = Interlocked.Increment(ref _count); - _trieStore.MemoryUsedByDirtyCache += node.GetMemorySize(false) + KeyMemoryUsage; - } - } - - public TrieNode FindCachedOrUnknown(in Key key) - { - if (TryGetValue(key, out TrieNode trieNode)) - { - Metrics.LoadedFromCacheNodesCount++; - } - else - { - trieNode = new TrieNode(NodeType.Unknown, key.Keccak); - if (_logger.IsTrace) Trace(trieNode); - SaveInCache(key, trieNode); - } - - return trieNode; - - [MethodImpl(MethodImplOptions.NoInlining)] - void Trace(TrieNode trieNode) - { - _logger.Trace($"Creating new node {trieNode}"); - } - } - - public TrieNode FromCachedRlpOrUnknown(in Key key) - { - // ReSharper disable once ConditionIsAlwaysTrueOrFalse - if (TryGetValue(key, out TrieNode trieNode)) - { - if (trieNode!.FullRlp.IsNull) - { - // // this happens in SyncProgressResolver - // throw new InvalidAsynchronousStateException("Read only trie store is trying to read a transient node."); - return new TrieNode(NodeType.Unknown, key.Keccak); - } - - // we returning a copy to avoid multithreaded access - trieNode = new TrieNode(NodeType.Unknown, key.Keccak, trieNode.FullRlp); - trieNode.ResolveNode(_trieStore.GetTrieStore(key.AddressAsHash256), key.Path); - trieNode.Keccak = key.Keccak; - - Metrics.LoadedFromCacheNodesCount++; - } - else - { - trieNode = new TrieNode(NodeType.Unknown, key.Keccak); - } - - if (_logger.IsTrace) Trace(trieNode); - return trieNode; - - [MethodImpl(MethodImplOptions.NoInlining)] - void Trace(TrieNode trieNode) - { - _logger.Trace($"Creating new node {trieNode}"); - } - } - - public bool IsNodeCached(in Key key) - { - if (_storeByHash) return _byHashObjectCache.ContainsKey(key.Keccak); - return _byKeyObjectCache.ContainsKey(key); - } - - public IEnumerable> AllNodes - { - get - { - if (_storeByHash) - { - return _byHashObjectCache.Select( - pair => new KeyValuePair(new Key(null, TreePath.Empty, pair.Key.Value), pair.Value)); - } - - return _byKeyObjectCache; - } - } - - public bool TryGetValue(in Key key, out TrieNode node) - { - if (_storeByHash) - { - return _byHashObjectCache.TryGetValue(key.Keccak, out node); - } - return _byKeyObjectCache.TryGetValue(key, out node); - } - - public bool TryAdd(in Key key, TrieNode node) - { - if (_storeByHash) - { - return _byHashObjectCache.TryAdd(key.Keccak, node); - } - return _byKeyObjectCache.TryAdd(key, node); - } - - public void Remove(in Key key) - { - if (_storeByHash) - { - if (_byHashObjectCache.Remove(key.Keccak, out _)) - { - Metrics.CachedNodesCount = Interlocked.Decrement(ref _count); - } - - return; - } - if (_byKeyObjectCache.Remove(key, out _)) - { - Metrics.CachedNodesCount = Interlocked.Decrement(ref _count); - } - } - - private MapLock AcquireMapLock() - { - if (_storeByHash) - { - return new MapLock() - { - _storeByHash = _storeByHash, - _byHashLock = _byHashObjectCache.AcquireLock() - }; - } - return new MapLock() - { - _storeByHash = _storeByHash, - _byKeyLock = _byKeyObjectCache.AcquireLock() - }; - } - - public int Count => _count; - - /// - /// This method is responsible for reviewing the nodes that are directly in the cache and - /// removing ones that are either no longer referenced or already persisted. - /// - /// - public long PruneCache(bool skipRecalculateMemory = false) - { - bool shouldTrackPersistedNode = _pastPathHash is not null && !_trieStore.IsCurrentlyFullPruning; - long newMemory = 0; - - using (AcquireMapLock()) - { - foreach ((Key key, TrieNode node) in AllNodes) - { - if (node.IsPersisted) - { - if (_logger.IsTrace) _logger.Trace($"Removing persisted {node} from memory."); - - if (shouldTrackPersistedNode) - { - TrackPersistedNode(key, node); - } - - Hash256? keccak = node.Keccak; - if (keccak is null) - { - TreePath path2 = key.Path; - keccak = node.GenerateKey(_trieStore.GetTrieStore(key.AddressAsHash256), ref path2, isRoot: true); - if (keccak != key.Keccak) - { - throw new InvalidOperationException($"Persisted {node} {key} != {keccak}"); - } - - node.Keccak = keccak; - } - Remove(key); - - Metrics.PrunedPersistedNodesCount++; - } - else if (_trieStore.IsNoLongerNeeded(node)) - { - if (_logger.IsTrace) _logger.Trace($"Removing {node} from memory (no longer referenced)."); - if (node.Keccak is null) - { - throw new InvalidOperationException($"Removed {node}"); - } - Remove(key); - - Metrics.PrunedTransientNodesCount++; - } - else if (!skipRecalculateMemory) - { - node.PrunePersistedRecursively(1); - newMemory += node.GetMemorySize(false) + KeyMemoryUsage; - } - } - } - - return newMemory + (_persistedLastSeen?.Count ?? 0) * 48; - - void TrackPersistedNode(in TrieStoreDirtyNodesCache.Key key, TrieNode node) - { - if (key.Path.Length > TinyTreePath.MaxNibbleLength) return; - TinyTreePath treePath = new(key.Path); - // Persisted node with LastSeen is a node that has been re-committed, likely due to processing - // recalculated to the same hash. - if (node.LastSeen >= 0) - { - // Update _persistedLastSeen to later value. - HashAndTinyPathAndHash plsKey = new(key.Address, in treePath, key.Keccak); - if (!_persistedLastSeen.TryGetValue(plsKey, out var currentLastSeen) || currentLastSeen <= node.LastSeen) - { - _persistedLastSeen[plsKey] = node.LastSeen; - } - } - - // This persisted node is being removed from cache. Keep it in mind in case of an update to the same - // path. - _pastPathHash.Set(new(key.Address, in treePath), key.Keccak); - } - } - - - public void RemovePastKeys(ConcurrentDictionary persistedHashes, INodeStorage nodeStorage) - { - bool CanRemove(in ValueHash256 address, TinyTreePath path, in TreePath fullPath, in ValueHash256 keccak, Hash256? currentlyPersistingKeccak) - { - // Multiple current hash that we don't keep track for simplicity. Just ignore this case. - if (currentlyPersistingKeccak is null) return false; - - // The persisted hash is the same as currently persisting hash. Do nothing. - if ((ValueHash256)currentlyPersistingKeccak == keccak) return false; - - // We have it in cache and it is still needed. - if (TryGetValue(new TrieStoreDirtyNodesCache.Key(address, fullPath, keccak.ToCommitment()), out TrieNode node) && - !_trieStore.IsNoLongerNeeded(node)) return false; - - // We don't have it in cache, but we know it was re-committed, so if it is still needed, don't remove - if (_persistedLastSeen.TryGetValue(new(address, in path, in keccak), out long commitBlock) && - !_trieStore.IsNoLongerNeeded(commitBlock)) return false; - - return true; - } - - using (AcquireMapLock()) - { - INodeStorage.WriteBatch writeBatch = nodeStorage.StartWriteBatch(); - try - { - int round = 0; - foreach (KeyValuePair keyValuePair in persistedHashes) - { - HashAndTinyPath key = keyValuePair.Key; - if (_pastPathHash.TryGet(key, out ValueHash256 prevHash)) - { - TreePath fullPath = key.path.ToTreePath(); // Micro op to reduce double convert - if (CanRemove(key.addr, key.path, fullPath, prevHash, keyValuePair.Value)) - { - Metrics.RemovedNodeCount++; - Hash256? address = key.addr == default ? null : key.addr.ToCommitment(); - writeBatch.Set(address, fullPath, prevHash, default, WriteFlags.DisableWAL); - round++; - } - } - - // Batches of 256 - if (round > 256) - { - writeBatch.Dispose(); - writeBatch = nodeStorage.StartWriteBatch(); - round = 0; - } - } - } - catch (Exception ex) - { - if (_logger.IsError) _logger.Error($"Failed to remove past keys. {ex}"); - } - finally - { - writeBatch.Dispose(); - } - } - } - - public void CleanObsoletePersistedLastSeen() - { - Dictionary? persistedLastSeen = _persistedLastSeen; - - // The amount of nodes that is no longer needed is so high that creating a new dictionary is faster. - Dictionary newPersistedLastSeen = new(); - - foreach (KeyValuePair keyValuePair in persistedLastSeen) - { - if (!_trieStore.IsNoLongerNeeded(keyValuePair.Value)) - { - newPersistedLastSeen.Add(keyValuePair.Key, keyValuePair.Value); - } - } - - _persistedLastSeen = newPersistedLastSeen; - } - - public void PersistAll(INodeStorage nodeStorage, CancellationToken cancellationToken) - { - ConcurrentDictionary wasPersisted = new(); - - void PersistNode(TrieNode n, Hash256? address, TreePath path) - { - if (n.Keccak is null) return; - TrieStoreDirtyNodesCache.Key key = new TrieStoreDirtyNodesCache.Key(address, path, n.Keccak); - if (wasPersisted.TryAdd(key, true)) - { - nodeStorage.Set(address, path, n.Keccak, n.FullRlp); - n.IsPersisted = true; - } - } - - using (AcquireMapLock()) - { - foreach (KeyValuePair kv in AllNodes) - { - if (cancellationToken.IsCancellationRequested) return; - TrieStoreDirtyNodesCache.Key key = kv.Key; - TreePath path = key.Path; - Hash256? address = key.AddressAsHash256; - kv.Value.CallRecursively(PersistNode, address, ref path, _trieStore.GetTrieStore(address), false, _logger, resolveStorageRoot: false); - } - } - } - - public void Dump() - { - if (_logger.IsTrace) - { - _logger.Trace($"Trie node dirty cache ({Count})"); - foreach (KeyValuePair keyValuePair in AllNodes) - { - _logger.Trace($" {keyValuePair.Value}"); - } - } - } - - public void ClearLivePruningTracking() - { - _persistedLastSeen.Clear(); - _pastPathHash?.Clear(); - } - - public void Clear() - { - _byHashObjectCache.NoResizeClear(); - _byKeyObjectCache.NoResizeClear(); - Interlocked.Exchange(ref _count, 0); - Metrics.CachedNodesCount = 0; - _trieStore.MemoryUsedByDirtyCache = 0; - } - - internal readonly struct Key : IEquatable - { - internal const long MemoryUsage = 8 + 36 + 8; // (address (probably shared), path, keccak pointer (shared with TrieNode)) - public readonly ValueHash256 Address; - public Hash256? AddressAsHash256 => Address == default ? null : Address.ToCommitment(); - // Direct member rather than property for large struct, so members are called directly, - // rather than struct copy through the property. Could also return a ref through property. - public readonly TreePath Path; - public Hash256 Keccak { get; } - - public Key(Hash256? address, in TreePath path, Hash256 keccak) - { - Address = address ?? default; - Path = path; - Keccak = keccak; - } - public Key(in ValueHash256 address, in TreePath path, Hash256 keccak) - { - Address = address; - Path = path; - Keccak = keccak; - } - - [SkipLocalsInit] - public override int GetHashCode() - { - var addressHash = Address != default ? Address.GetHashCode() : 1; - return Keccak.ValueHash256.GetChainedHashCode((uint)Path.GetHashCode()) ^ addressHash; - } - - public bool Equals(Key other) - { - return other.Keccak == Keccak && other.Path == Path && other.Address == Address; - } - - public override bool Equals(object? obj) - { - return obj is Key other && Equals(other); - } - } - - internal ref struct MapLock - { - public bool _storeByHash; - public ConcurrentDictionaryLock.Lock _byHashLock; - public ConcurrentDictionaryLock.Lock _byKeyLock; - - public readonly void Dispose() - { - if (_storeByHash) - { - _byHashLock.Dispose(); - } - else - { - _byKeyLock.Dispose(); - } - } - } - } - private const int ShardedDirtyNodeCount = 256; private int _isFirst; @@ -1711,59 +1244,4 @@ public static int GetPrime(int min) ]; } } - - [StructLayout(LayoutKind.Auto)] - internal readonly struct HashAndTinyPathAndHash : IEquatable - { - public readonly ValueHash256 hash; - public readonly TinyTreePath path; - public readonly ValueHash256 valueHash; - - public HashAndTinyPathAndHash(Hash256? hash, in TinyTreePath path, in ValueHash256 valueHash) - { - this.hash = hash ?? default; - this.path = path; - this.valueHash = valueHash; - } - public HashAndTinyPathAndHash(in ValueHash256 hash, in TinyTreePath path, in ValueHash256 valueHash) - { - this.hash = hash; - this.path = path; - this.valueHash = valueHash; - } - - public bool Equals(HashAndTinyPathAndHash other) => hash == other.hash && path.Equals(in other.path) && valueHash.Equals(in other.valueHash); - public override bool Equals(object? obj) => obj is HashAndTinyPath other && Equals(other); - public override int GetHashCode() - { - var hashHash = hash != default ? hash.GetHashCode() : 1; - return valueHash.GetChainedHashCode((uint)path.GetHashCode()) ^ hashHash; - } - } - - [StructLayout(LayoutKind.Auto)] - internal readonly struct HashAndTinyPath : IEquatable - { - public readonly ValueHash256 addr; - public readonly TinyTreePath path; - - public HashAndTinyPath(Hash256? hash, in TinyTreePath path) - { - addr = hash ?? default; - this.path = path; - } - public HashAndTinyPath(in ValueHash256 hash, in TinyTreePath path) - { - addr = hash; - this.path = path; - } - - public bool Equals(HashAndTinyPath other) => addr == other.addr && path.Equals(in other.path); - public override bool Equals(object? obj) => obj is HashAndTinyPath other && Equals(other); - public override int GetHashCode() - { - var addressHash = addr != default ? addr.GetHashCode() : 1; - return path.GetHashCode() ^ addressHash; - } - } } diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TrieStoreDirtyNodesCache.cs b/src/Nethermind/Nethermind.Trie/Pruning/TrieStoreDirtyNodesCache.cs new file mode 100644 index 00000000000..57d3d7f83da --- /dev/null +++ b/src/Nethermind/Nethermind.Trie/Pruning/TrieStoreDirtyNodesCache.cs @@ -0,0 +1,481 @@ +// SPDX-FileCopyrightText: 2024 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Threading; +using Nethermind.Core; +using Nethermind.Core.Caching; +using Nethermind.Core.Collections; +using Nethermind.Core.Crypto; +using Nethermind.Logging; +using CollectionExtensions = Nethermind.Core.Collections.CollectionExtensions; + +namespace Nethermind.Trie.Pruning; + +internal class TrieStoreDirtyNodesCache +{ + private readonly TrieStore _trieStore; + private int _count = 0; + private readonly ILogger _logger; + private readonly bool _storeByHash; + private readonly ConcurrentDictionary _byKeyObjectCache; + private readonly ConcurrentDictionary _byHashObjectCache; + + // Track some of the persisted path hash. Used to be able to remove keys when it is replaced. + // If null, disable removing key. + private readonly ClockCache? _pastPathHash; + + // Track ALL of the recently re-committed persisted nodes. This is so that we don't accidentally remove + // recommitted persisted nodes (which will not get re-persisted). + private Dictionary? _persistedLastSeen; + + public readonly long KeyMemoryUsage; + + public TrieStoreDirtyNodesCache(TrieStore trieStore, int trackedPastKeyCount, bool storeByHash, ILogger logger) + { + _trieStore = trieStore; + _logger = logger; + // If the nodestore indicated that path is not required, + // we will use a map with hash as its key instead of the full Key to reduce memory usage. + _storeByHash = storeByHash; + int initialBuckets = TrieStore.HashHelpers.GetPrime(Math.Max(31, Environment.ProcessorCount * 16)); + if (_storeByHash) + { + _byHashObjectCache = new(CollectionExtensions.LockPartitions, initialBuckets); + } + else + { + _byKeyObjectCache = new(CollectionExtensions.LockPartitions, initialBuckets); + } + KeyMemoryUsage = _storeByHash ? 0 : Key.MemoryUsage; // 0 because previously it was not counted. + + if (trackedPastKeyCount > 0 && !storeByHash) + { + _persistedLastSeen = new(); + _pastPathHash = new(trackedPastKeyCount); + } + } + + public void SaveInCache(in Key key, TrieNode node) + { + Debug.Assert(node.Keccak is not null, "Cannot store in cache nodes without resolved key."); + if (TryAdd(key, node)) + { + Metrics.CachedNodesCount = Interlocked.Increment(ref _count); + _trieStore.MemoryUsedByDirtyCache += node.GetMemorySize(false) + KeyMemoryUsage; + } + } + + public TrieNode FindCachedOrUnknown(in Key key) + { + if (TryGetValue(key, out TrieNode trieNode)) + { + Metrics.LoadedFromCacheNodesCount++; + } + else + { + trieNode = new TrieNode(NodeType.Unknown, key.Keccak); + if (_logger.IsTrace) Trace(trieNode); + SaveInCache(key, trieNode); + } + + return trieNode; + + [MethodImpl(MethodImplOptions.NoInlining)] + void Trace(TrieNode trieNode) + { + _logger.Trace($"Creating new node {trieNode}"); + } + } + + public TrieNode FromCachedRlpOrUnknown(in Key key) + { + // ReSharper disable once ConditionIsAlwaysTrueOrFalse + if (TryGetValue(key, out TrieNode trieNode)) + { + if (trieNode!.FullRlp.IsNull) + { + // // this happens in SyncProgressResolver + // throw new InvalidAsynchronousStateException("Read only trie store is trying to read a transient node."); + return new TrieNode(NodeType.Unknown, key.Keccak); + } + + // we returning a copy to avoid multithreaded access + trieNode = new TrieNode(NodeType.Unknown, key.Keccak, trieNode.FullRlp); + trieNode.ResolveNode(_trieStore.GetTrieStore(key.AddressAsHash256), key.Path); + trieNode.Keccak = key.Keccak; + + Metrics.LoadedFromCacheNodesCount++; + } + else + { + trieNode = new TrieNode(NodeType.Unknown, key.Keccak); + } + + if (_logger.IsTrace) Trace(trieNode); + return trieNode; + + [MethodImpl(MethodImplOptions.NoInlining)] + void Trace(TrieNode trieNode) + { + _logger.Trace($"Creating new node {trieNode}"); + } + } + + public bool IsNodeCached(in Key key) + { + if (_storeByHash) return _byHashObjectCache.ContainsKey(key.Keccak); + return _byKeyObjectCache.ContainsKey(key); + } + + public IEnumerable> AllNodes + { + get + { + if (_storeByHash) + { + return _byHashObjectCache.Select( + pair => new KeyValuePair(new Key(null, TreePath.Empty, pair.Key.Value), pair.Value)); + } + + return _byKeyObjectCache; + } + } + + public bool TryGetValue(in Key key, out TrieNode node) + { + if (_storeByHash) + { + return _byHashObjectCache.TryGetValue(key.Keccak, out node); + } + return _byKeyObjectCache.TryGetValue(key, out node); + } + + public bool TryAdd(in Key key, TrieNode node) + { + if (_storeByHash) + { + return _byHashObjectCache.TryAdd(key.Keccak, node); + } + return _byKeyObjectCache.TryAdd(key, node); + } + + public void Remove(in Key key) + { + if (_storeByHash) + { + if (_byHashObjectCache.Remove(key.Keccak, out _)) + { + Metrics.CachedNodesCount = Interlocked.Decrement(ref _count); + } + + return; + } + if (_byKeyObjectCache.Remove(key, out _)) + { + Metrics.CachedNodesCount = Interlocked.Decrement(ref _count); + } + } + + private MapLock AcquireMapLock() + { + if (_storeByHash) + { + return new MapLock() + { + _storeByHash = _storeByHash, + _byHashLock = _byHashObjectCache.AcquireLock() + }; + } + return new MapLock() + { + _storeByHash = _storeByHash, + _byKeyLock = _byKeyObjectCache.AcquireLock() + }; + } + + public int Count => _count; + + /// + /// This method is responsible for reviewing the nodes that are directly in the cache and + /// removing ones that are either no longer referenced or already persisted. + /// + /// + public long PruneCache(bool skipRecalculateMemory = false) + { + bool shouldTrackPersistedNode = _pastPathHash is not null && !_trieStore.IsCurrentlyFullPruning; + long newMemory = 0; + + using (AcquireMapLock()) + { + foreach ((Key key, TrieNode node) in AllNodes) + { + if (node.IsPersisted) + { + if (_logger.IsTrace) _logger.Trace($"Removing persisted {node} from memory."); + + if (shouldTrackPersistedNode) + { + TrackPersistedNode(key, node); + } + + Hash256? keccak = node.Keccak; + if (keccak is null) + { + TreePath path2 = key.Path; + keccak = node.GenerateKey(_trieStore.GetTrieStore(key.AddressAsHash256), ref path2, isRoot: true); + if (keccak != key.Keccak) + { + throw new InvalidOperationException($"Persisted {node} {key} != {keccak}"); + } + + node.Keccak = keccak; + } + Remove(key); + + Metrics.PrunedPersistedNodesCount++; + } + else if (_trieStore.IsNoLongerNeeded(node)) + { + if (_logger.IsTrace) _logger.Trace($"Removing {node} from memory (no longer referenced)."); + if (node.Keccak is null) + { + throw new InvalidOperationException($"Removed {node}"); + } + Remove(key); + + Metrics.PrunedTransientNodesCount++; + } + else if (!skipRecalculateMemory) + { + node.PrunePersistedRecursively(1); + newMemory += node.GetMemorySize(false) + KeyMemoryUsage; + } + } + } + + return newMemory + (_persistedLastSeen?.Count ?? 0) * 48; + + void TrackPersistedNode(in TrieStoreDirtyNodesCache.Key key, TrieNode node) + { + if (key.Path.Length > TinyTreePath.MaxNibbleLength) return; + TinyTreePath treePath = new(key.Path); + // Persisted node with LastSeen is a node that has been re-committed, likely due to processing + // recalculated to the same hash. + if (node.LastSeen >= 0) + { + // Update _persistedLastSeen to later value. + HashAndTinyPathAndHash plsKey = new(key.Address, in treePath, key.Keccak); + if (!_persistedLastSeen.TryGetValue(plsKey, out var currentLastSeen) || currentLastSeen <= node.LastSeen) + { + _persistedLastSeen[plsKey] = node.LastSeen; + } + } + + // This persisted node is being removed from cache. Keep it in mind in case of an update to the same + // path. + _pastPathHash.Set(new(key.Address, in treePath), key.Keccak); + } + } + + + public void RemovePastKeys(ConcurrentDictionary persistedHashes, INodeStorage nodeStorage) + { + bool CanRemove(in ValueHash256 address, TinyTreePath path, in TreePath fullPath, in ValueHash256 keccak, Hash256? currentlyPersistingKeccak) + { + // Multiple current hash that we don't keep track for simplicity. Just ignore this case. + if (currentlyPersistingKeccak is null) return false; + + // The persisted hash is the same as currently persisting hash. Do nothing. + if ((ValueHash256)currentlyPersistingKeccak == keccak) return false; + + // We have it in cache and it is still needed. + if (TryGetValue(new TrieStoreDirtyNodesCache.Key(address, fullPath, keccak.ToCommitment()), out TrieNode node) && + !_trieStore.IsNoLongerNeeded(node)) return false; + + // We don't have it in cache, but we know it was re-committed, so if it is still needed, don't remove + if (_persistedLastSeen.TryGetValue(new(address, in path, in keccak), out long commitBlock) && + !_trieStore.IsNoLongerNeeded(commitBlock)) return false; + + return true; + } + + using (AcquireMapLock()) + { + INodeStorage.WriteBatch writeBatch = nodeStorage.StartWriteBatch(); + try + { + int round = 0; + foreach (KeyValuePair keyValuePair in persistedHashes) + { + HashAndTinyPath key = keyValuePair.Key; + if (_pastPathHash.TryGet(key, out ValueHash256 prevHash)) + { + TreePath fullPath = key.path.ToTreePath(); // Micro op to reduce double convert + if (CanRemove(key.addr, key.path, fullPath, prevHash, keyValuePair.Value)) + { + Metrics.RemovedNodeCount++; + Hash256? address = key.addr == default ? null : key.addr.ToCommitment(); + writeBatch.Set(address, fullPath, prevHash, default, WriteFlags.DisableWAL); + round++; + } + } + + // Batches of 256 + if (round > 256) + { + writeBatch.Dispose(); + writeBatch = nodeStorage.StartWriteBatch(); + round = 0; + } + } + } + catch (Exception ex) + { + if (_logger.IsError) _logger.Error($"Failed to remove past keys. {ex}"); + } + finally + { + writeBatch.Dispose(); + } + } + } + + public void CleanObsoletePersistedLastSeen() + { + Dictionary? persistedLastSeen = _persistedLastSeen; + + // The amount of nodes that is no longer needed is so high that creating a new dictionary is faster. + Dictionary newPersistedLastSeen = new(); + + foreach (KeyValuePair keyValuePair in persistedLastSeen) + { + if (!_trieStore.IsNoLongerNeeded(keyValuePair.Value)) + { + newPersistedLastSeen.Add(keyValuePair.Key, keyValuePair.Value); + } + } + + _persistedLastSeen = newPersistedLastSeen; + } + + public void PersistAll(INodeStorage nodeStorage, CancellationToken cancellationToken) + { + ConcurrentDictionary wasPersisted = new(); + + void PersistNode(TrieNode n, Hash256? address, TreePath path) + { + if (n.Keccak is null) return; + TrieStoreDirtyNodesCache.Key key = new TrieStoreDirtyNodesCache.Key(address, path, n.Keccak); + if (wasPersisted.TryAdd(key, true)) + { + nodeStorage.Set(address, path, n.Keccak, n.FullRlp); + n.IsPersisted = true; + } + } + + using (AcquireMapLock()) + { + foreach (KeyValuePair kv in AllNodes) + { + if (cancellationToken.IsCancellationRequested) return; + TrieStoreDirtyNodesCache.Key key = kv.Key; + TreePath path = key.Path; + Hash256? address = key.AddressAsHash256; + kv.Value.CallRecursively(PersistNode, address, ref path, _trieStore.GetTrieStore(address), false, _logger, resolveStorageRoot: false); + } + } + } + + public void Dump() + { + if (_logger.IsTrace) + { + _logger.Trace($"Trie node dirty cache ({Count})"); + foreach (KeyValuePair keyValuePair in AllNodes) + { + _logger.Trace($" {keyValuePair.Value}"); + } + } + } + + public void ClearLivePruningTracking() + { + _persistedLastSeen.Clear(); + _pastPathHash?.Clear(); + } + + public void Clear() + { + _byHashObjectCache.NoResizeClear(); + _byKeyObjectCache.NoResizeClear(); + Interlocked.Exchange(ref _count, 0); + Metrics.CachedNodesCount = 0; + _trieStore.MemoryUsedByDirtyCache = 0; + } + + internal readonly struct Key : IEquatable + { + internal const long MemoryUsage = 8 + 36 + 8; // (address (probably shared), path, keccak pointer (shared with TrieNode)) + public readonly ValueHash256 Address; + public Hash256? AddressAsHash256 => Address == default ? null : Address.ToCommitment(); + // Direct member rather than property for large struct, so members are called directly, + // rather than struct copy through the property. Could also return a ref through property. + public readonly TreePath Path; + public Hash256 Keccak { get; } + + public Key(Hash256? address, in TreePath path, Hash256 keccak) + { + Address = address ?? default; + Path = path; + Keccak = keccak; + } + public Key(in ValueHash256 address, in TreePath path, Hash256 keccak) + { + Address = address; + Path = path; + Keccak = keccak; + } + + [SkipLocalsInit] + public override int GetHashCode() + { + var addressHash = Address != default ? Address.GetHashCode() : 1; + return Keccak.ValueHash256.GetChainedHashCode((uint)Path.GetHashCode()) ^ addressHash; + } + + public bool Equals(Key other) + { + return other.Keccak == Keccak && other.Path == Path && other.Address == Address; + } + + public override bool Equals(object? obj) + { + return obj is Key other && Equals(other); + } + } + + internal ref struct MapLock + { + public bool _storeByHash; + public ConcurrentDictionaryLock.Lock _byHashLock; + public ConcurrentDictionaryLock.Lock _byKeyLock; + + public readonly void Dispose() + { + if (_storeByHash) + { + _byHashLock.Dispose(); + } + else + { + _byKeyLock.Dispose(); + } + } + } +} From 486cca622f4519d7a1aca7dba3b67231baa8e752 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Mon, 7 Oct 2024 15:30:14 +0800 Subject: [PATCH 02/15] Interface change --- .../SyncServerTests.cs | 6 ++- .../Pruning/TreeStoreTests.cs | 53 +++++++++++++------ .../Nethermind.Trie.Test/TrieNodeTests.cs | 2 + .../Nethermind.Trie/CachedTrieStore.cs | 9 +--- .../Nethermind.Trie/PatriciaTree.cs | 25 ++++----- .../Nethermind.Trie/PreCachedTrieStore.cs | 9 +--- .../Pruning/IScopedTrieStore.cs | 13 +++-- .../Nethermind.Trie/Pruning/ITrieStore.cs | 5 +- .../Nethermind.Trie/Pruning/NullTrieStore.cs | 20 +++++-- .../Pruning/ReadOnlyTrieStore.cs | 14 +++-- .../Pruning/ScopedTrieStore.cs | 9 +--- .../Nethermind.Trie/Pruning/TrieStore.cs | 24 +++++++++ .../Nethermind.Trie/TrieStoreWithReadFlags.cs | 9 +--- 13 files changed, 118 insertions(+), 80 deletions(-) diff --git a/src/Nethermind/Nethermind.Synchronization.Test/SyncServerTests.cs b/src/Nethermind/Nethermind.Synchronization.Test/SyncServerTests.cs index e3540377184..f782fbc8aa5 100644 --- a/src/Nethermind/Nethermind.Synchronization.Test/SyncServerTests.cs +++ b/src/Nethermind/Nethermind.Synchronization.Test/SyncServerTests.cs @@ -702,8 +702,10 @@ public void GetNodeData_returns_cached_trie_nodes() Hash256 nodeKey = TestItem.KeccakA; TrieNode node = new(NodeType.Leaf, nodeKey, TestItem.KeccakB.Bytes); IScopedTrieStore scopedTrieStore = trieStore.GetTrieStore(null); - scopedTrieStore.CommitNode(1, new NodeCommitInfo(node, TreePath.Empty)); - scopedTrieStore.FinishBlockCommit(TrieType.State, 1, node); + using (ICommitter committer = scopedTrieStore.BeginCommit(TrieType.State, 1, node)) + { + committer.CommitNode(new NodeCommitInfo(node, TreePath.Empty)); + } stateDb.KeyExists(nodeKey).Should().BeFalse(); ctx.SyncServer.GetNodeData(new[] { nodeKey }, CancellationToken.None, NodeDataType.All).Should().BeEquivalentTo(new[] { TestItem.KeccakB.BytesToArray() }); diff --git a/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs b/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs index 3a1677afd69..f51536709b3 100644 --- a/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs +++ b/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs @@ -70,7 +70,10 @@ public void Memory_with_one_node_is_288() using TrieStore fullTrieStore = CreateTrieStore(pruningStrategy: new TestPruningStrategy(true)); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.CommitNode(1234, new NodeCommitInfo(trieNode, TreePath.Empty)); + using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1234, null)) + { + committer.CommitNode(new NodeCommitInfo(trieNode, TreePath.Empty)); + } fullTrieStore.MemoryUsedByDirtyCache.Should().Be( trieNode.GetMemorySize(false) + ExpectedPerNodeKeyMemorySize); } @@ -85,10 +88,15 @@ public void Pruning_off_cache_should_not_change_commit_node() using TrieStore fullTrieStore = CreateTrieStore(); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.CommitNode(1234, new NodeCommitInfo(trieNode, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 1234, trieNode); - trieStore.CommitNode(124, new NodeCommitInfo(trieNode2, TreePath.Empty)); - trieStore.CommitNode(11234, new NodeCommitInfo(trieNode3, TreePath.Empty)); + using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1234, trieNode)) + { + committer.CommitNode(new NodeCommitInfo(trieNode, TreePath.Empty)); + } + using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1235, trieNode)) + { + committer.CommitNode(new NodeCommitInfo(trieNode2, TreePath.Empty)); + committer.CommitNode(new NodeCommitInfo(trieNode3, TreePath.Empty)); + } fullTrieStore.MemoryUsedByDirtyCache.Should().Be(0); } @@ -101,8 +109,11 @@ public void When_commit_forward_write_flag_if_available() using TrieStore fullTrieStore = CreateTrieStore(kvStore: testMemDb); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.CommitNode(1234, new NodeCommitInfo(trieNode, TreePath.Empty), WriteFlags.LowPriority); - trieStore.FinishBlockCommit(TrieType.State, 1234, trieNode, WriteFlags.LowPriority); + + using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1234, trieNode, WriteFlags.LowPriority)) + { + committer.CommitNode(new NodeCommitInfo(trieNode, TreePath.Empty)); + } if (_scheme == INodeStorage.KeyScheme.HalfPath) { @@ -123,13 +134,13 @@ public void Should_always_announce_block_number_when_pruning_disabled_and_persis using TrieStore fullTrieStore = CreateTrieStore(persistenceStrategy: Archive.Instance); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); fullTrieStore.ReorgBoundaryReached += (_, e) => reorgBoundaryCount += e.BlockNumber; - trieStore.FinishBlockCommit(TrieType.State, 1, trieNode); + trieStore.BeginCommit(TrieType.State, 1, trieNode).Dispose(); reorgBoundaryCount.Should().Be(0); - trieStore.FinishBlockCommit(TrieType.State, 2, trieNode); + trieStore.BeginCommit(TrieType.State, 2, trieNode).Dispose(); reorgBoundaryCount.Should().Be(1); - trieStore.FinishBlockCommit(TrieType.State, 3, trieNode); + trieStore.BeginCommit(TrieType.State, 3, trieNode).Dispose(); reorgBoundaryCount.Should().Be(3); - trieStore.FinishBlockCommit(TrieType.State, 4, trieNode); + trieStore.BeginCommit(TrieType.State, 4, trieNode).Dispose(); reorgBoundaryCount.Should().Be(6); } @@ -142,10 +153,10 @@ public void Should_always_announce_zero_when_not_persisting() using TrieStore fullTrieStore = CreateTrieStore(); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); fullTrieStore.ReorgBoundaryReached += (_, e) => reorgBoundaryCount += e.BlockNumber; - trieStore.FinishBlockCommit(TrieType.State, 1, trieNode); - trieStore.FinishBlockCommit(TrieType.State, 2, trieNode); - trieStore.FinishBlockCommit(TrieType.State, 3, trieNode); - trieStore.FinishBlockCommit(TrieType.State, 4, trieNode); + trieStore.BeginCommit(TrieType.State, 1, trieNode).Dispose(); + trieStore.BeginCommit(TrieType.State, 2, trieNode).Dispose(); + trieStore.BeginCommit(TrieType.State, 3, trieNode).Dispose(); + trieStore.BeginCommit(TrieType.State, 4, trieNode).Dispose(); reorgBoundaryCount.Should().Be(0L); } @@ -189,13 +200,17 @@ public void Memory_with_two_nodes_is_correct() using TrieStore fullTrieStore = CreateTrieStore(pruningStrategy: new TestPruningStrategy(true)); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.CommitNode(1234, new NodeCommitInfo(trieNode1, TreePath.Empty)); - trieStore.CommitNode(1234, new NodeCommitInfo(trieNode2, TreePath.Empty)); + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 1234, null)) + { + committer.CommitNode(new NodeCommitInfo(trieNode1, TreePath.Empty)); + committer.CommitNode(new NodeCommitInfo(trieNode2, TreePath.Empty)); + } fullTrieStore.MemoryUsedByDirtyCache.Should().Be( trieNode1.GetMemorySize(false) + ExpectedPerNodeKeyMemorySize + trieNode2.GetMemorySize(false) + ExpectedPerNodeKeyMemorySize); } + /* [Test] public void Memory_with_two_times_two_nodes_is_correct() { @@ -773,9 +788,12 @@ public void ReadOnly_store_returns_copies(bool pruning) readOnlyNode.Key?.ToString().Should().Be(originalNode.Key?.ToString()); } + */ private long ExpectedPerNodeKeyMemorySize => _scheme == INodeStorage.KeyScheme.Hash ? 0 : TrieStoreDirtyNodesCache.Key.MemoryUsage; + /* + [Test] public void After_commit_should_have_has_root() { @@ -916,5 +934,6 @@ public async Task Will_NotRemove_ReCommittedNode() memDb.Count.Should().Be(4); } + */ } } diff --git a/src/Nethermind/Nethermind.Trie.Test/TrieNodeTests.cs b/src/Nethermind/Nethermind.Trie.Test/TrieNodeTests.cs index 6ee564db88a..1c3a67a5988 100644 --- a/src/Nethermind/Nethermind.Trie.Test/TrieNodeTests.cs +++ b/src/Nethermind/Nethermind.Trie.Test/TrieNodeTests.cs @@ -925,6 +925,7 @@ void CheckChildren() [Test] public void Rlp_is_cloned_when_cloning() { + /* IScopedTrieStore trieStore = new TrieStore(new MemDb(), NullLogManager.Instance).GetTrieStore(null); TrieNode leaf1 = new(NodeType.Leaf); @@ -956,6 +957,7 @@ public void Rlp_is_cloned_when_cloning() restoredLeaf1.Should().NotBeNull(); restoredLeaf1.ResolveNode(trieStore, TreePath.Empty); restoredLeaf1.Value.ToArray().Should().BeEquivalentTo(leaf1.Value.ToArray()); + */ } [Test] diff --git a/src/Nethermind/Nethermind.Trie/CachedTrieStore.cs b/src/Nethermind/Nethermind.Trie/CachedTrieStore.cs index d9a1769c5aa..b1429a4bb5a 100644 --- a/src/Nethermind/Nethermind.Trie/CachedTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/CachedTrieStore.cs @@ -42,14 +42,9 @@ public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256? address) public INodeStorage.KeyScheme Scheme => @base.Scheme; - public void CommitNode(long blockNumber, NodeCommitInfo nodeCommitInfo, WriteFlags writeFlags = WriteFlags.None) + public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) { - @base.CommitNode(blockNumber, nodeCommitInfo, writeFlags); - } - - public void FinishBlockCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) - { - @base.FinishBlockCommit(trieType, blockNumber, root, writeFlags); + return @base.BeginCommit(trieType, blockNumber, root, writeFlags); } public bool IsPersisted(in TreePath path, in ValueHash256 keccak) diff --git a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs index 3ab8c9d1127..4a058cd3863 100644 --- a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs +++ b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs @@ -138,23 +138,24 @@ public void Commit(long blockNumber, bool skipRoot = false, WriteFlags writeFlag ThrowReadOnlyTrieException(); } - if (RootRef is not null && RootRef.IsDirty) + using (ICommitter committer = TrieStore.BeginCommit(TrieType, blockNumber, RootRef, writeFlags)) { - Commit(new NodeCommitInfo(RootRef, TreePath.Empty), skipSelf: skipRoot); - while (TryDequeueCommit(out NodeCommitInfo node)) + if (RootRef is not null && RootRef.IsDirty) { - if (_logger.IsTrace) Trace(blockNumber, node); - TrieStore.CommitNode(blockNumber, node, writeFlags: writeFlags); - } + Commit(new NodeCommitInfo(RootRef, TreePath.Empty), skipSelf: skipRoot); + while (TryDequeueCommit(out NodeCommitInfo node)) + { + if (_logger.IsTrace) Trace(blockNumber, node); + committer.CommitNode(node); + } - // reset objects - TreePath path = TreePath.Empty; - RootRef!.ResolveKey(TrieStore, ref path, true, bufferPool: _bufferPool); - SetRootHash(RootRef.Keccak!, true); + // reset objects + TreePath path = TreePath.Empty; + RootRef!.ResolveKey(TrieStore, ref path, true, bufferPool: _bufferPool); + SetRootHash(RootRef.Keccak!, true); + } } - TrieStore.FinishBlockCommit(TrieType, blockNumber, RootRef, writeFlags); - if (_logger.IsDebug) Debug(blockNumber); bool TryDequeueCommit(out NodeCommitInfo value) diff --git a/src/Nethermind/Nethermind.Trie/PreCachedTrieStore.cs b/src/Nethermind/Nethermind.Trie/PreCachedTrieStore.cs index efb9d242748..82d83a3e8f3 100644 --- a/src/Nethermind/Nethermind.Trie/PreCachedTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/PreCachedTrieStore.cs @@ -34,14 +34,9 @@ public void Dispose() _inner.Dispose(); } - public void CommitNode(long blockNumber, Hash256? address, in NodeCommitInfo nodeCommitInfo, WriteFlags writeFlags = WriteFlags.None) + public ICommitter BeginCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags) { - _inner.CommitNode(blockNumber, address, in nodeCommitInfo, writeFlags); - } - - public void FinishBlockCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) - { - _inner.FinishBlockCommit(trieType, blockNumber, address, root, writeFlags); + return _inner.BeginCommit(trieType, blockNumber, address, root, writeFlags); } public bool IsPersisted(Hash256? address, in TreePath path, in ValueHash256 keccak) diff --git a/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs index f8d2384fd0e..e8957678493 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs @@ -13,11 +13,7 @@ namespace Nethermind.Trie.Pruning; /// public interface IScopedTrieStore : ITrieNodeResolver { - // TODO: Commit and FinishBlockCommit is unnecessary. Geth just compile the changes and return it in a batch, - // which get committed in a single call. - void CommitNode(long blockNumber, NodeCommitInfo nodeCommitInfo, WriteFlags writeFlags = WriteFlags.None); - - void FinishBlockCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None); + ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None); // Only used by snap provider, so ValueHash instead of Hash bool IsPersisted(in TreePath path, in ValueHash256 keccak); @@ -25,3 +21,10 @@ public interface IScopedTrieStore : ITrieNodeResolver // Used for trie node recovery void Set(in TreePath path, in ValueHash256 keccak, byte[] rlp); } + +public interface ICommitter: IDisposable +{ + // TODO: Commit and FinishBlockCommit is unnecessary. Geth just compile the changes and return it in a batch, + // which get committed in a single call. + void CommitNode(NodeCommitInfo nodeCommitInfo); +} diff --git a/src/Nethermind/Nethermind.Trie/Pruning/ITrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/ITrieStore.cs index a7ec0699ec1..f9f2f59d55e 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/ITrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/ITrieStore.cs @@ -13,10 +13,6 @@ namespace Nethermind.Trie.Pruning /// public interface ITrieStore : IDisposable { - void CommitNode(long blockNumber, Hash256? address, in NodeCommitInfo nodeCommitInfo, WriteFlags writeFlags = WriteFlags.None); - - void FinishBlockCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags = WriteFlags.None); - bool IsPersisted(Hash256? address, in TreePath path, in ValueHash256 keccak); IReadOnlyTrieStore AsReadOnly(INodeStorage? keyValueStore = null); @@ -37,6 +33,7 @@ public interface ITrieStore : IDisposable byte[]? LoadRlp(Hash256? address, in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None); byte[]? TryLoadRlp(Hash256? address, in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None); INodeStorage.KeyScheme Scheme { get; } + ICommitter BeginCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags); } public interface IPruningTrieStore diff --git a/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs index 4982d7166b8..02ec3404ce3 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs @@ -15,16 +15,17 @@ private NullTrieStore() { } public static NullTrieStore Instance { get; } = new(); - public void CommitNode(long blockNumber, NodeCommitInfo nodeCommitInfo, WriteFlags flags = WriteFlags.None) { } - - public void FinishBlockCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags flags = WriteFlags.None) { } - public TrieNode FindCachedOrUnknown(in TreePath treePath, Hash256 hash) => new(NodeType.Unknown, hash); public byte[] LoadRlp(in TreePath treePath, Hash256 hash, ReadFlags flags = ReadFlags.None) => Array.Empty(); public byte[]? TryLoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) => Array.Empty(); + public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) + { + return new NullCommitter(); + } + public bool IsPersisted(in TreePath path, in ValueHash256 keccak) => true; public void Set(in TreePath path, in ValueHash256 keccak, byte[] rlp) @@ -37,5 +38,16 @@ public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256 storageRoot) } public INodeStorage.KeyScheme Scheme => INodeStorage.KeyScheme.HalfPath; + + internal class NullCommitter: ICommitter + { + public void Dispose() + { + } + + public void CommitNode(NodeCommitInfo nodeCommitInfo) + { + } + } } } diff --git a/src/Nethermind/Nethermind.Trie/Pruning/ReadOnlyTrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/ReadOnlyTrieStore.cs index 3152aad59cd..b168bcfd34a 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/ReadOnlyTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/ReadOnlyTrieStore.cs @@ -39,9 +39,10 @@ public IReadOnlyTrieStore AsReadOnly(INodeStorage nodeStore) return new ReadOnlyTrieStore(_trieStore, nodeStore); } - public void CommitNode(long blockNumber, Hash256? address, in NodeCommitInfo nodeCommitInfo, WriteFlags flags = WriteFlags.None) { } - - public void FinishBlockCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags flags = WriteFlags.None) { } + public ICommitter BeginCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags) + { + return new NullTrieStore.NullCommitter(); + } public event EventHandler ReorgBoundaryReached { @@ -101,12 +102,9 @@ public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256? address) public INodeStorage.KeyScheme Scheme => _trieStoreImplementation.Scheme; - public void CommitNode(long blockNumber, NodeCommitInfo nodeCommitInfo, WriteFlags writeFlags = WriteFlags.None) - { - } - - public void FinishBlockCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) + public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) { + return new NullTrieStore.NullCommitter(); } public bool IsPersisted(in TreePath path, in ValueHash256 keccak) diff --git a/src/Nethermind/Nethermind.Trie/Pruning/ScopedTrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/ScopedTrieStore.cs index 02dc3e4d85e..f53e1089b14 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/ScopedTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/ScopedTrieStore.cs @@ -40,14 +40,9 @@ public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256? address) public INodeStorage.KeyScheme Scheme => _trieStoreImplementation.Scheme; - public void CommitNode(long blockNumber, NodeCommitInfo nodeCommitInfo, WriteFlags writeFlags = WriteFlags.None) + public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) { - _trieStoreImplementation.CommitNode(blockNumber, _address, nodeCommitInfo, writeFlags); - } - - public void FinishBlockCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) - { - _trieStoreImplementation.FinishBlockCommit(trieType, blockNumber, _address, root, writeFlags); + return _trieStoreImplementation.BeginCommit(trieType, blockNumber, _address, root, writeFlags); } public bool IsPersisted(in TreePath path, in ValueHash256 keccak) diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs index 50d71a17d5f..15dfddb8b38 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs @@ -3,9 +3,11 @@ using System; using System.Collections.Concurrent; +using System.ComponentModel.Design.Serialization; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Net.Sockets; using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; @@ -286,6 +288,28 @@ static void ThrowNodeIsNotSame(TrieNode node, TrieNode cachedNodeCopy) => throw new InvalidOperationException($"The hash of replacement node {cachedNodeCopy} is not the same as the original {node}."); } + public ICommitter BeginCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags) + { + return new TrieStoreCommitter(this, trieType, blockNumber, address, root, writeFlags); + } + + private class TrieStoreCommitter(TrieStore trieStore, TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) : ICommitter + { + public void Dispose() + { + if (root is not null && root.IsDirty) + { + root = trieStore.FindCachedOrUnknown(address, TreePath.Empty, root.Keccak); + } + trieStore.FinishBlockCommit(trieType, blockNumber, address, root, writeFlags); + } + + public void CommitNode(NodeCommitInfo nodeCommitInfo) + { + trieStore.CommitNode(blockNumber, address, nodeCommitInfo); + } + } + public void FinishBlockCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) { ArgumentOutOfRangeException.ThrowIfNegative(blockNumber); diff --git a/src/Nethermind/Nethermind.Trie/TrieStoreWithReadFlags.cs b/src/Nethermind/Nethermind.Trie/TrieStoreWithReadFlags.cs index 557e519a36e..b97aff8991e 100644 --- a/src/Nethermind/Nethermind.Trie/TrieStoreWithReadFlags.cs +++ b/src/Nethermind/Nethermind.Trie/TrieStoreWithReadFlags.cs @@ -16,14 +16,9 @@ public TrieStoreWithReadFlags(IScopedTrieStore implementation, ReadFlags flags) _baseImplementation = implementation; } - public void CommitNode(long blockNumber, NodeCommitInfo nodeCommitInfo, WriteFlags writeFlags = WriteFlags.None) + public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) { - _baseImplementation.CommitNode(blockNumber, nodeCommitInfo, writeFlags); - } - - public void FinishBlockCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) - { - _baseImplementation.FinishBlockCommit(trieType, blockNumber, root, writeFlags); + return _baseImplementation.BeginCommit(trieType, blockNumber, root, writeFlags); } public bool IsPersisted(in TreePath path, in ValueHash256 keccak) From d118423de24c110c7fc6dea1382ef062228c5761 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Mon, 7 Oct 2024 16:21:48 +0800 Subject: [PATCH 03/15] Fix test --- src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs index 15dfddb8b38..be763ffb835 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs @@ -295,9 +295,10 @@ public ICommitter BeginCommit(TrieType trieType, long blockNumber, Hash256? addr private class TrieStoreCommitter(TrieStore trieStore, TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) : ICommitter { + private bool _needToResetRoot = root is not null && root.IsDirty; public void Dispose() { - if (root is not null && root.IsDirty) + if (_needToResetRoot) { root = trieStore.FindCachedOrUnknown(address, TreePath.Empty, root.Keccak); } From 1ba593f580b9de697343b33b63d5906a88ee08a6 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Mon, 7 Oct 2024 16:23:31 +0800 Subject: [PATCH 04/15] Remove commit queue --- .../Nethermind.Trie/PatriciaTree.cs | 40 ++++--------------- .../Nethermind.Trie/Pruning/TrieStore.cs | 2 +- 2 files changed, 8 insertions(+), 34 deletions(-) diff --git a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs index 4a058cd3863..2aa87af6434 100644 --- a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs +++ b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs @@ -41,7 +41,6 @@ public class PatriciaTree private Stack? _nodeStack; private ConcurrentQueue? _commitExceptions; - private ConcurrentQueue? _currentCommit; public IScopedTrieStore TrieStore { get; } public ICappedArrayPool? _bufferPool; @@ -142,12 +141,7 @@ public void Commit(long blockNumber, bool skipRoot = false, WriteFlags writeFlag { if (RootRef is not null && RootRef.IsDirty) { - Commit(new NodeCommitInfo(RootRef, TreePath.Empty), skipSelf: skipRoot); - while (TryDequeueCommit(out NodeCommitInfo node)) - { - if (_logger.IsTrace) Trace(blockNumber, node); - committer.CommitNode(node); - } + Commit(committer, new NodeCommitInfo(RootRef, TreePath.Empty), skipSelf: skipRoot); // reset objects TreePath path = TreePath.Empty; @@ -158,18 +152,6 @@ public void Commit(long blockNumber, bool skipRoot = false, WriteFlags writeFlag if (_logger.IsDebug) Debug(blockNumber); - bool TryDequeueCommit(out NodeCommitInfo value) - { - Unsafe.SkipInit(out value); - return _currentCommit?.TryDequeue(out value) ?? false; - } - - [MethodImpl(MethodImplOptions.NoInlining)] - void Trace(long blockNumber, in NodeCommitInfo node) - { - _logger.Trace($"Committing {node} in {blockNumber}"); - } - [MethodImpl(MethodImplOptions.NoInlining)] void Debug(long blockNumber) { @@ -177,7 +159,7 @@ void Debug(long blockNumber) } } - private void Commit(NodeCommitInfo nodeCommitInfo, bool skipSelf = false) + private void Commit(ICommitter committer, NodeCommitInfo nodeCommitInfo, bool skipSelf = false) { if (!_allowCommits) { @@ -196,7 +178,7 @@ private void Commit(NodeCommitInfo nodeCommitInfo, bool skipSelf = false) if (node.IsChildDirty(i)) { TreePath childPath = node.GetChildPath(nodeCommitInfo.Path, i); - Commit(new NodeCommitInfo(node.GetChildWithChildPath(TrieStore, ref childPath, i)!, node, childPath, i)); + Commit(committer, new NodeCommitInfo(node.GetChildWithChildPath(TrieStore, ref childPath, i)!, node, childPath, i)); } else { @@ -233,7 +215,7 @@ private void Commit(NodeCommitInfo nodeCommitInfo, bool skipSelf = false) { try { - Commit(nodesToCommit[i]); + Commit(committer, nodesToCommit[i]); } catch (Exception e) { @@ -250,7 +232,7 @@ private void Commit(NodeCommitInfo nodeCommitInfo, bool skipSelf = false) { for (int i = 0; i < nodesToCommit.Count; i++) { - Commit(nodesToCommit[i]); + Commit(committer, nodesToCommit[i]); } } } @@ -266,7 +248,7 @@ private void Commit(NodeCommitInfo nodeCommitInfo, bool skipSelf = false) if (extensionChild.IsDirty) { - Commit(new NodeCommitInfo(extensionChild, node, childPath, 0)); + Commit(committer, new NodeCommitInfo(extensionChild, node, childPath, 0)); } else { @@ -281,7 +263,7 @@ private void Commit(NodeCommitInfo nodeCommitInfo, bool skipSelf = false) { if (!skipSelf) { - EnqueueCommit(nodeCommitInfo); + committer.CommitNode(nodeCommitInfo); } } else @@ -289,14 +271,6 @@ private void Commit(NodeCommitInfo nodeCommitInfo, bool skipSelf = false) if (_logger.IsTrace) TraceSkipInlineNode(node); } - void EnqueueCommit(in NodeCommitInfo value) - { - ConcurrentQueue queue = Volatile.Read(ref _currentCommit); - // Allocate queue if first commit made - queue ??= CreateQueue(ref _currentCommit); - queue.Enqueue(value); - } - void ClearExceptions() => _commitExceptions?.Clear(); bool WereExceptions() => _commitExceptions?.IsEmpty == false; diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs index be763ffb835..0065fb21998 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs @@ -307,7 +307,7 @@ public void Dispose() public void CommitNode(NodeCommitInfo nodeCommitInfo) { - trieStore.CommitNode(blockNumber, address, nodeCommitInfo); + trieStore.CommitNode(blockNumber, address, nodeCommitInfo, writeFlags: writeFlags); } } From 8b1ec781fa819f44d400b59e2e362e877c5ba65d Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Mon, 7 Oct 2024 16:34:20 +0800 Subject: [PATCH 05/15] Pass tree path by ref --- .../Pruning/TreeStoreTests.cs | 18 ++++++---- .../Nethermind.Trie/NodeCommitInfo.cs | 7 +--- .../Nethermind.Trie/PatriciaTree.cs | 34 +++++++++++-------- .../Pruning/IScopedTrieStore.cs | 4 +-- .../Nethermind.Trie/Pruning/NullTrieStore.cs | 2 +- .../Nethermind.Trie/Pruning/TrieStore.cs | 15 ++++---- 6 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs b/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs index f51536709b3..5dbef0febc9 100644 --- a/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs +++ b/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs @@ -70,9 +70,10 @@ public void Memory_with_one_node_is_288() using TrieStore fullTrieStore = CreateTrieStore(pruningStrategy: new TestPruningStrategy(true)); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); + TreePath path = TreePath.Empty; using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1234, null)) { - committer.CommitNode(new NodeCommitInfo(trieNode, TreePath.Empty)); + committer.CommitNode(ref path, new NodeCommitInfo(trieNode)); } fullTrieStore.MemoryUsedByDirtyCache.Should().Be( trieNode.GetMemorySize(false) + ExpectedPerNodeKeyMemorySize); @@ -88,14 +89,15 @@ public void Pruning_off_cache_should_not_change_commit_node() using TrieStore fullTrieStore = CreateTrieStore(); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); + TreePath path = TreePath.Empty; using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1234, trieNode)) { - committer.CommitNode(new NodeCommitInfo(trieNode, TreePath.Empty)); + committer.CommitNode(ref path, new NodeCommitInfo(trieNode)); } using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1235, trieNode)) { - committer.CommitNode(new NodeCommitInfo(trieNode2, TreePath.Empty)); - committer.CommitNode(new NodeCommitInfo(trieNode3, TreePath.Empty)); + committer.CommitNode(ref path, new NodeCommitInfo(trieNode2)); + committer.CommitNode(ref path, new NodeCommitInfo(trieNode3)); } fullTrieStore.MemoryUsedByDirtyCache.Should().Be(0); } @@ -110,9 +112,10 @@ public void When_commit_forward_write_flag_if_available() using TrieStore fullTrieStore = CreateTrieStore(kvStore: testMemDb); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); + TreePath path = TreePath.Empty; using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1234, trieNode, WriteFlags.LowPriority)) { - committer.CommitNode(new NodeCommitInfo(trieNode, TreePath.Empty)); + committer.CommitNode(ref path, new NodeCommitInfo(trieNode)); } if (_scheme == INodeStorage.KeyScheme.HalfPath) @@ -200,10 +203,11 @@ public void Memory_with_two_nodes_is_correct() using TrieStore fullTrieStore = CreateTrieStore(pruningStrategy: new TestPruningStrategy(true)); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); + TreePath path = TreePath.Empty; using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 1234, null)) { - committer.CommitNode(new NodeCommitInfo(trieNode1, TreePath.Empty)); - committer.CommitNode(new NodeCommitInfo(trieNode2, TreePath.Empty)); + committer.CommitNode(ref path, new NodeCommitInfo(trieNode1)); + committer.CommitNode(ref path, new NodeCommitInfo(trieNode2)); } fullTrieStore.MemoryUsedByDirtyCache.Should().Be( trieNode1.GetMemorySize(false) + ExpectedPerNodeKeyMemorySize + diff --git a/src/Nethermind/Nethermind.Trie/NodeCommitInfo.cs b/src/Nethermind/Nethermind.Trie/NodeCommitInfo.cs index bf85db2709d..42260715717 100644 --- a/src/Nethermind/Nethermind.Trie/NodeCommitInfo.cs +++ b/src/Nethermind/Nethermind.Trie/NodeCommitInfo.cs @@ -3,30 +3,25 @@ namespace Nethermind.Trie public readonly struct NodeCommitInfo { public NodeCommitInfo( - TrieNode node, - in TreePath path + TrieNode node ) { ChildPositionAtParent = 0; Node = node; - Path = path; NodeParent = null; } public NodeCommitInfo( TrieNode node, TrieNode nodeParent, - in TreePath path, int childPositionAtParent) { ChildPositionAtParent = childPositionAtParent; Node = node; - Path = path; NodeParent = nodeParent; } public TrieNode? Node { get; } - public readonly TreePath Path; public TrieNode? NodeParent { get; } diff --git a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs index 2aa87af6434..ea57be9f1eb 100644 --- a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs +++ b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs @@ -141,10 +141,10 @@ public void Commit(long blockNumber, bool skipRoot = false, WriteFlags writeFlag { if (RootRef is not null && RootRef.IsDirty) { - Commit(committer, new NodeCommitInfo(RootRef, TreePath.Empty), skipSelf: skipRoot); + TreePath path = TreePath.Empty; + Commit(committer, ref path, new NodeCommitInfo(RootRef), skipSelf: skipRoot); // reset objects - TreePath path = TreePath.Empty; RootRef!.ResolveKey(TrieStore, ref path, true, bufferPool: _bufferPool); SetRootHash(RootRef.Keccak!, true); } @@ -159,7 +159,7 @@ void Debug(long blockNumber) } } - private void Commit(ICommitter committer, NodeCommitInfo nodeCommitInfo, bool skipSelf = false) + private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo nodeCommitInfo, bool skipSelf = false) { if (!_allowCommits) { @@ -167,7 +167,6 @@ private void Commit(ICommitter committer, NodeCommitInfo nodeCommitInfo, bool sk } TrieNode node = nodeCommitInfo.Node; - TreePath path = nodeCommitInfo.Path; if (node!.IsBranch) { // idea from EthereumJ - testing parallel branches @@ -177,8 +176,10 @@ private void Commit(ICommitter committer, NodeCommitInfo nodeCommitInfo, bool sk { if (node.IsChildDirty(i)) { - TreePath childPath = node.GetChildPath(nodeCommitInfo.Path, i); - Commit(committer, new NodeCommitInfo(node.GetChildWithChildPath(TrieStore, ref childPath, i)!, node, childPath, i)); + path.AppendMut(i); + TrieNode childNode = node.GetChildWithChildPath(TrieStore, ref path, i); + Commit(committer, ref path, new NodeCommitInfo(childNode!, node, i)); + path.TruncateOne(); } else { @@ -191,13 +192,13 @@ private void Commit(ICommitter committer, NodeCommitInfo nodeCommitInfo, bool sk } else { - List nodesToCommit = new(16); + List<(TreePath, NodeCommitInfo)> nodesToCommit = new(16); for (int i = 0; i < 16; i++) { if (node.IsChildDirty(i)) { - TreePath childPath = node.GetChildPath(nodeCommitInfo.Path, i); - nodesToCommit.Add(new NodeCommitInfo(node.GetChildWithChildPath(TrieStore, ref childPath, i)!, node, childPath, i)); + TreePath childPath = node.GetChildPath(path, i); + nodesToCommit.Add((childPath, new NodeCommitInfo(node.GetChildWithChildPath(TrieStore, ref childPath, i)!, node, i))); } else { @@ -215,7 +216,8 @@ private void Commit(ICommitter committer, NodeCommitInfo nodeCommitInfo, bool sk { try { - Commit(committer, nodesToCommit[i]); + (TreePath childPath, NodeCommitInfo commitInfo) = nodesToCommit[i]; + Commit(committer, ref childPath, commitInfo); } catch (Exception e) { @@ -232,15 +234,16 @@ private void Commit(ICommitter committer, NodeCommitInfo nodeCommitInfo, bool sk { for (int i = 0; i < nodesToCommit.Count; i++) { - Commit(committer, nodesToCommit[i]); + (TreePath childPath, NodeCommitInfo commitInfo) = nodesToCommit[i]; + Commit(committer, ref childPath, commitInfo); } } } } else if (node.NodeType == NodeType.Extension) { - TreePath childPath = node.GetChildPath(nodeCommitInfo.Path, 0); - TrieNode extensionChild = node.GetChildWithChildPath(TrieStore, ref childPath, 0); + int previousPathLength = node.AppendChildPath(ref path, 0); + TrieNode extensionChild = node.GetChildWithChildPath(TrieStore, ref path, 0); if (extensionChild is null) { ThrowInvalidExtension(); @@ -248,12 +251,13 @@ private void Commit(ICommitter committer, NodeCommitInfo nodeCommitInfo, bool sk if (extensionChild.IsDirty) { - Commit(committer, new NodeCommitInfo(extensionChild, node, childPath, 0)); + Commit(committer, ref path, new NodeCommitInfo(extensionChild, node, 0)); } else { if (_logger.IsTrace) TraceExtensionSkip(extensionChild); } + path.TruncateMut(previousPathLength); } node.ResolveKey(TrieStore, ref path, nodeCommitInfo.IsRoot, bufferPool: _bufferPool); @@ -263,7 +267,7 @@ private void Commit(ICommitter committer, NodeCommitInfo nodeCommitInfo, bool sk { if (!skipSelf) { - committer.CommitNode(nodeCommitInfo); + committer.CommitNode(ref path, nodeCommitInfo); } } else diff --git a/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs index e8957678493..63f4d69957d 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs @@ -24,7 +24,5 @@ public interface IScopedTrieStore : ITrieNodeResolver public interface ICommitter: IDisposable { - // TODO: Commit and FinishBlockCommit is unnecessary. Geth just compile the changes and return it in a batch, - // which get committed in a single call. - void CommitNode(NodeCommitInfo nodeCommitInfo); + void CommitNode(ref TreePath path, NodeCommitInfo nodeCommitInfo); } diff --git a/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs index 02ec3404ce3..47b0d924bab 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs @@ -45,7 +45,7 @@ public void Dispose() { } - public void CommitNode(NodeCommitInfo nodeCommitInfo) + public void CommitNode(ref TreePath path, NodeCommitInfo nodeCommitInfo) { } } diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs index 0065fb21998..42bd2f06070 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs @@ -151,7 +151,7 @@ public int CachedNodesCount } } - public void CommitNode(long blockNumber, Hash256? address, in NodeCommitInfo nodeCommitInfo, WriteFlags writeFlags = WriteFlags.None) + private void CommitNode(long blockNumber, Hash256? address, ref TreePath path, in NodeCommitInfo nodeCommitInfo, WriteFlags writeFlags = WriteFlags.None) { ArgumentOutOfRangeException.ThrowIfNegative(blockNumber); EnsureCommitSetExistsForBlock(blockNumber); @@ -176,12 +176,12 @@ public void CommitNode(long blockNumber, Hash256? address, in NodeCommitInfo nod ThrowNodeHasBeenSeen(blockNumber, node); } - node = SaveOrReplaceInDirtyNodesCache(address, nodeCommitInfo, node); + node = SaveOrReplaceInDirtyNodesCache(address, ref path, nodeCommitInfo, node); node.LastSeen = Math.Max(blockNumber, node.LastSeen); if (!_pruningStrategy.PruningEnabled) { - PersistNode(address, nodeCommitInfo.Path, node, blockNumber, writeFlags); + PersistNode(address, path, node, blockNumber, writeFlags); } CommittedNodesCount++; @@ -241,18 +241,17 @@ private TrieNode DirtyNodesFromCachedRlpOrUnknown(TrieStoreDirtyNodesCache.Key k private TrieNode DirtyNodesFindCachedOrUnknown(TrieStoreDirtyNodesCache.Key key) => GetDirtyNodeShard(key).FindCachedOrUnknown(key); - private TrieNode SaveOrReplaceInDirtyNodesCache(Hash256? address, NodeCommitInfo nodeCommitInfo, TrieNode node) + private TrieNode SaveOrReplaceInDirtyNodesCache(Hash256? address, ref TreePath path, NodeCommitInfo nodeCommitInfo, TrieNode node) { if (_pruningStrategy.PruningEnabled) { - TrieStoreDirtyNodesCache.Key key = new TrieStoreDirtyNodesCache.Key(address, nodeCommitInfo.Path, node.Keccak); + TrieStoreDirtyNodesCache.Key key = new TrieStoreDirtyNodesCache.Key(address, path, node.Keccak); if (DirtyNodesTryGetValue(in key, out TrieNode cachedNodeCopy)) { Metrics.LoadedFromCacheNodesCount++; if (!ReferenceEquals(cachedNodeCopy, node)) { if (_logger.IsTrace) Trace(node, cachedNodeCopy); - TreePath path = nodeCommitInfo.Path; cachedNodeCopy.ResolveKey(GetTrieStore(address), ref path, nodeCommitInfo.IsRoot); if (node.Keccak != cachedNodeCopy.Keccak) { @@ -305,9 +304,9 @@ public void Dispose() trieStore.FinishBlockCommit(trieType, blockNumber, address, root, writeFlags); } - public void CommitNode(NodeCommitInfo nodeCommitInfo) + public void CommitNode(ref TreePath path, NodeCommitInfo nodeCommitInfo) { - trieStore.CommitNode(blockNumber, address, nodeCommitInfo, writeFlags: writeFlags); + trieStore.CommitNode(blockNumber, address, ref path, nodeCommitInfo, writeFlags: writeFlags); } } From 10dac394b79ac9a4a0db1ec478307cd079b2c66d Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Mon, 7 Oct 2024 16:56:38 +0800 Subject: [PATCH 06/15] Parallelize --- .../Nethermind.Trie/PatriciaTree.cs | 45 +++++++++++++++++++ .../Pruning/IScopedTrieStore.cs | 3 ++ .../Nethermind.Trie/Pruning/TrieStore.cs | 37 ++++++++++++++- 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs index ea57be9f1eb..575fa24d69b 100644 --- a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs +++ b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs @@ -40,7 +40,9 @@ public class PatriciaTree private Stack? _nodeStack; +#pragma warning disable CS0169 // Field is never used private ConcurrentQueue? _commitExceptions; +#pragma warning restore CS0169 // Field is never used public IScopedTrieStore TrieStore { get; } public ICappedArrayPool? _bufferPool; @@ -170,6 +172,46 @@ private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo node if (node!.IsBranch) { // idea from EthereumJ - testing parallel branches + + List? childTasks = null; + + for (int i = 0; i < 16; i++) + { + if (node.IsChildDirty(i)) + { + if (committer.CanSpawnTask()) + { + childTasks ??= new List(); + TreePath childPath = path.Append(i); + TrieNode childNode = node.GetChildWithChildPath(TrieStore, ref childPath, i); + childTasks.Add(Task.Run(() => + { + Commit(committer, ref childPath, new NodeCommitInfo(childNode!, node, i)); + committer.ReturnConcurrencyQuota(); + })); + } + else + { + path.AppendMut(i); + TrieNode childNode = node.GetChildWithChildPath(TrieStore, ref path, i); + Commit(committer, ref path, new NodeCommitInfo(childNode!, node, i)); + path.TruncateOne(); + } + } + else + { + if (_logger.IsTrace) + { + Trace(node, ref path, i); + } + } + } + + if (childTasks != null) { + Task.WaitAll(childTasks.ToArray()); + } + + /* if (!_parallelBranches || !nodeCommitInfo.IsRoot) { for (int i = 0; i < 16; i++) @@ -239,6 +281,7 @@ private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo node } } } + */ } else if (node.NodeType == NodeType.Extension) { @@ -275,6 +318,7 @@ private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo node if (_logger.IsTrace) TraceSkipInlineNode(node); } + /* void ClearExceptions() => _commitExceptions?.Clear(); bool WereExceptions() => _commitExceptions?.IsEmpty == false; @@ -297,6 +341,7 @@ ConcurrentQueue CreateQueue(ref ConcurrentQueue queueRef) [DoesNotReturn] [StackTraceHidden] void ThrowAggregateExceptions() => throw new AggregateException(_commitExceptions); + */ [DoesNotReturn] [StackTraceHidden] diff --git a/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs index 63f4d69957d..a44cd2081af 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs @@ -25,4 +25,7 @@ public interface IScopedTrieStore : ITrieNodeResolver public interface ICommitter: IDisposable { void CommitNode(ref TreePath path, NodeCommitInfo nodeCommitInfo); + + bool CanSpawnTask() => false; + void ReturnConcurrencyQuota() {} } diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs index 42bd2f06070..cf0649281d8 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs @@ -289,12 +289,27 @@ static void ThrowNodeIsNotSame(TrieNode node, TrieNode cachedNodeCopy) => public ICommitter BeginCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags) { - return new TrieStoreCommitter(this, trieType, blockNumber, address, root, writeFlags); + int concurrency = 0; + if (_pruningStrategy.PruningEnabled) + { + concurrency = Environment.ProcessorCount; + } + return new TrieStoreCommitter(this, trieType, blockNumber, address, root, writeFlags, concurrency); } - private class TrieStoreCommitter(TrieStore trieStore, TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) : ICommitter + private class TrieStoreCommitter( + TrieStore trieStore, + TrieType trieType, + long blockNumber, + Hash256? address, + TrieNode? root, + WriteFlags writeFlags, + int concurrency + ) : ICommitter { private bool _needToResetRoot = root is not null && root.IsDirty; + private int _concurrency = concurrency; + public void Dispose() { if (_needToResetRoot) @@ -308,6 +323,24 @@ public void CommitNode(ref TreePath path, NodeCommitInfo nodeCommitInfo) { trieStore.CommitNode(blockNumber, address, ref path, nodeCommitInfo, writeFlags: writeFlags); } + + public bool CanSpawnTask() + { + if (Interlocked.Decrement(ref _concurrency) < 0) + { + ReturnConcurrencyQuota(); + return false; + } + else + { + return true; + } + } + + public void ReturnConcurrencyQuota() + { + Interlocked.Increment(ref _concurrency); + } } public void FinishBlockCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) From 657791e9f18422bc7420f56a097597ab1f06c947 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Mon, 7 Oct 2024 20:20:31 +0800 Subject: [PATCH 07/15] More tuning --- .../Nethermind.Trie/PatriciaTree.cs | 148 ++++++------------ .../Nethermind.Trie/Pruning/TrieStore.cs | 58 +++---- 2 files changed, 76 insertions(+), 130 deletions(-) diff --git a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs index 575fa24d69b..69b902fdfbd 100644 --- a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs +++ b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs @@ -14,7 +14,6 @@ using System.Threading.Tasks; using Nethermind.Core; using Nethermind.Core.Buffers; -using Nethermind.Core.Cpu; using Nethermind.Core.Crypto; using Nethermind.Core.Extensions; using Nethermind.Logging; @@ -57,6 +56,9 @@ public class PatriciaTree public TrieNode? RootRef { get; set; } + // Used to estimate if parallelization is needed during commit + private long _writeBeforeCommit = 0; + /// /// Only used in EthereumTests /// @@ -139,12 +141,24 @@ public void Commit(long blockNumber, bool skipRoot = false, WriteFlags writeFlag ThrowReadOnlyTrieException(); } + int maxLevelForConcurrentCommit = -1; + _writeBeforeCommit /= 64; + if (_writeBeforeCommit > 0) + { + maxLevelForConcurrentCommit++; // Ok, we separate at top level + if (_writeBeforeCommit/16 > 0) + { + maxLevelForConcurrentCommit++; // Another level + } + } + _writeBeforeCommit = 0; + using (ICommitter committer = TrieStore.BeginCommit(TrieType, blockNumber, RootRef, writeFlags)) { if (RootRef is not null && RootRef.IsDirty) { TreePath path = TreePath.Empty; - Commit(committer, ref path, new NodeCommitInfo(RootRef), skipSelf: skipRoot); + Commit(committer, ref path, new NodeCommitInfo(RootRef), skipSelf: skipRoot, maxLevelForConcurrentCommit: maxLevelForConcurrentCommit); // reset objects RootRef!.ResolveKey(TrieStore, ref path, true, bufferPool: _bufferPool); @@ -161,7 +175,7 @@ void Debug(long blockNumber) } } - private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo nodeCommitInfo, bool skipSelf = false) + private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo nodeCommitInfo, bool skipSelf = false, int maxLevelForConcurrentCommit = -1) { if (!_allowCommits) { @@ -171,48 +185,7 @@ private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo node TrieNode node = nodeCommitInfo.Node; if (node!.IsBranch) { - // idea from EthereumJ - testing parallel branches - - List? childTasks = null; - - for (int i = 0; i < 16; i++) - { - if (node.IsChildDirty(i)) - { - if (committer.CanSpawnTask()) - { - childTasks ??= new List(); - TreePath childPath = path.Append(i); - TrieNode childNode = node.GetChildWithChildPath(TrieStore, ref childPath, i); - childTasks.Add(Task.Run(() => - { - Commit(committer, ref childPath, new NodeCommitInfo(childNode!, node, i)); - committer.ReturnConcurrencyQuota(); - })); - } - else - { - path.AppendMut(i); - TrieNode childNode = node.GetChildWithChildPath(TrieStore, ref path, i); - Commit(committer, ref path, new NodeCommitInfo(childNode!, node, i)); - path.TruncateOne(); - } - } - else - { - if (_logger.IsTrace) - { - Trace(node, ref path, i); - } - } - } - - if (childTasks != null) { - Task.WaitAll(childTasks.ToArray()); - } - - /* - if (!_parallelBranches || !nodeCommitInfo.IsRoot) + if (path.Length > maxLevelForConcurrentCommit) { for (int i = 0; i < 16; i++) { @@ -234,13 +207,35 @@ private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo node } else { - List<(TreePath, NodeCommitInfo)> nodesToCommit = new(16); + Task CreateTaskForPath(TreePath childPath, TrieNode childNode, int idx) + { + return Task.Run(() => + { + Commit(committer, ref childPath, new NodeCommitInfo(childNode!, node, idx)); + committer.ReturnConcurrencyQuota(); + }); + } + + List? childTasks = null; + for (int i = 0; i < 16; i++) { if (node.IsChildDirty(i)) { - TreePath childPath = node.GetChildPath(path, i); - nodesToCommit.Add((childPath, new NodeCommitInfo(node.GetChildWithChildPath(TrieStore, ref childPath, i)!, node, i))); + if (i < 15 && committer.CanSpawnTask()) + { + childTasks ??= new List(); + TreePath childPath = path.Append(i); + TrieNode childNode = node.GetChildWithChildPath(TrieStore, ref childPath, i); + childTasks.Add(CreateTaskForPath(childPath, childNode, i)); + } + else + { + path.AppendMut(i); + TrieNode childNode = node.GetChildWithChildPath(TrieStore, ref path, i); + Commit(committer, ref path, new NodeCommitInfo(childNode!, node, i)); + path.TruncateOne(); + } } else { @@ -251,37 +246,11 @@ private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo node } } - if (nodesToCommit.Count >= 4) - { - ClearExceptions(); - Parallel.For(0, nodesToCommit.Count, RuntimeInformation.ParallelOptionsLogicalCores, i => - { - try - { - (TreePath childPath, NodeCommitInfo commitInfo) = nodesToCommit[i]; - Commit(committer, ref childPath, commitInfo); - } - catch (Exception e) - { - AddException(e); - } - }); - - if (WereExceptions()) - { - ThrowAggregateExceptions(); - } - } - else + if (childTasks != null) { - for (int i = 0; i < nodesToCommit.Count; i++) - { - (TreePath childPath, NodeCommitInfo commitInfo) = nodesToCommit[i]; - Commit(committer, ref childPath, commitInfo); - } + Task.WaitAll(childTasks.ToArray()); } } - */ } else if (node.NodeType == NodeType.Extension) { @@ -318,31 +287,6 @@ private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo node if (_logger.IsTrace) TraceSkipInlineNode(node); } - /* - void ClearExceptions() => _commitExceptions?.Clear(); - bool WereExceptions() => _commitExceptions?.IsEmpty == false; - - void AddException(Exception value) - { - ConcurrentQueue queue = Volatile.Read(ref _commitExceptions); - // Allocate queue if first exception thrown - queue ??= CreateQueue(ref _commitExceptions); - queue.Enqueue(value); - } - - [MethodImpl(MethodImplOptions.NoInlining)] - ConcurrentQueue CreateQueue(ref ConcurrentQueue queueRef) - { - ConcurrentQueue queue = new(); - ConcurrentQueue current = Interlocked.CompareExchange(ref queueRef, queue, null); - return (current is null) ? queue : current; - } - - [DoesNotReturn] - [StackTraceHidden] - void ThrowAggregateExceptions() => throw new AggregateException(_commitExceptions); - */ - [DoesNotReturn] [StackTraceHidden] static void ThrowInvalidExtension() => throw new InvalidOperationException("An attempt to store an extension without a child."); @@ -517,6 +461,8 @@ public virtual void Set(ReadOnlySpan rawKey, in CappedArray value) ThrowNonConcurrentWrites(); } + _writeBeforeCommit++; + try { int nibblesCount = 2 * rawKey.Length; diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs index cf0649281d8..c20453347d6 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs @@ -153,9 +153,6 @@ public int CachedNodesCount private void CommitNode(long blockNumber, Hash256? address, ref TreePath path, in NodeCommitInfo nodeCommitInfo, WriteFlags writeFlags = WriteFlags.None) { - ArgumentOutOfRangeException.ThrowIfNegative(blockNumber); - EnsureCommitSetExistsForBlock(blockNumber); - if (_logger.IsTrace) Trace(blockNumber, in nodeCommitInfo); if (!nodeCommitInfo.IsEmptyBlockMarker && !nodeCommitInfo.Node.IsBoundaryProofNode) { @@ -176,13 +173,15 @@ private void CommitNode(long blockNumber, Hash256? address, ref TreePath path, i ThrowNodeHasBeenSeen(blockNumber, node); } - node = SaveOrReplaceInDirtyNodesCache(address, ref path, nodeCommitInfo, node); - node.LastSeen = Math.Max(blockNumber, node.LastSeen); - - if (!_pruningStrategy.PruningEnabled) + if (_pruningStrategy.PruningEnabled) + { + node = SaveOrReplaceInDirtyNodesCache(address, ref path, nodeCommitInfo, node); + } + else { PersistNode(address, path, node, blockNumber, writeFlags); } + node.LastSeen = Math.Max(blockNumber, node.LastSeen); CommittedNodesCount++; } @@ -243,35 +242,32 @@ private TrieNode DirtyNodesFindCachedOrUnknown(TrieStoreDirtyNodesCache.Key key) private TrieNode SaveOrReplaceInDirtyNodesCache(Hash256? address, ref TreePath path, NodeCommitInfo nodeCommitInfo, TrieNode node) { - if (_pruningStrategy.PruningEnabled) + TrieStoreDirtyNodesCache.Key key = new TrieStoreDirtyNodesCache.Key(address, path, node.Keccak); + if (DirtyNodesTryGetValue(in key, out TrieNode cachedNodeCopy)) { - TrieStoreDirtyNodesCache.Key key = new TrieStoreDirtyNodesCache.Key(address, path, node.Keccak); - if (DirtyNodesTryGetValue(in key, out TrieNode cachedNodeCopy)) + Metrics.LoadedFromCacheNodesCount++; + if (!ReferenceEquals(cachedNodeCopy, node)) { - Metrics.LoadedFromCacheNodesCount++; - if (!ReferenceEquals(cachedNodeCopy, node)) + if (_logger.IsTrace) Trace(node, cachedNodeCopy); + cachedNodeCopy.ResolveKey(GetTrieStore(address), ref path, nodeCommitInfo.IsRoot); + if (node.Keccak != cachedNodeCopy.Keccak) { - if (_logger.IsTrace) Trace(node, cachedNodeCopy); - cachedNodeCopy.ResolveKey(GetTrieStore(address), ref path, nodeCommitInfo.IsRoot); - if (node.Keccak != cachedNodeCopy.Keccak) - { - ThrowNodeIsNotSame(node, cachedNodeCopy); - } - - if (!nodeCommitInfo.IsRoot) - { - nodeCommitInfo.NodeParent!.ReplaceChildRef(nodeCommitInfo.ChildPositionAtParent, cachedNodeCopy); - } + ThrowNodeIsNotSame(node, cachedNodeCopy); + } - node = cachedNodeCopy; - Metrics.ReplacedNodesCount++; + if (!nodeCommitInfo.IsRoot) + { + nodeCommitInfo.NodeParent!.ReplaceChildRef(nodeCommitInfo.ChildPositionAtParent, cachedNodeCopy); } - } - else - { - DirtyNodesSaveInCache(key, node); + + node = cachedNodeCopy; + Metrics.ReplacedNodesCount++; } } + else + { + DirtyNodesSaveInCache(key, node); + } return node; @@ -292,8 +288,12 @@ public ICommitter BeginCommit(TrieType trieType, long blockNumber, Hash256? addr int concurrency = 0; if (_pruningStrategy.PruningEnabled) { + // The write batch when pruning is not enabled is not concurrent safe concurrency = Environment.ProcessorCount; } + ArgumentOutOfRangeException.ThrowIfNegative(blockNumber); + EnsureCommitSetExistsForBlock(blockNumber); + return new TrieStoreCommitter(this, trieType, blockNumber, address, root, writeFlags, concurrency); } From 49c3bbd5fcaf5f5e31cffbb0bc14a7f4b8d9c7df Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Tue, 8 Oct 2024 10:20:09 +0800 Subject: [PATCH 08/15] Array pool and fix non pruning issue on debug --- src/Nethermind/Nethermind.Trie/PatriciaTree.cs | 7 +++++-- src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs | 6 ++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs index 69b902fdfbd..ed23c9579b9 100644 --- a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs +++ b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs @@ -8,12 +8,14 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.IO; +using System.Linq; using System.Runtime.CompilerServices; using System.Runtime.ExceptionServices; using System.Threading; using System.Threading.Tasks; using Nethermind.Core; using Nethermind.Core.Buffers; +using Nethermind.Core.Collections; using Nethermind.Core.Crypto; using Nethermind.Core.Extensions; using Nethermind.Logging; @@ -216,7 +218,7 @@ Task CreateTaskForPath(TreePath childPath, TrieNode childNode, int idx) }); } - List? childTasks = null; + ArrayPoolList? childTasks = null; for (int i = 0; i < 16; i++) { @@ -224,7 +226,7 @@ Task CreateTaskForPath(TreePath childPath, TrieNode childNode, int idx) { if (i < 15 && committer.CanSpawnTask()) { - childTasks ??= new List(); + childTasks ??= new ArrayPoolList(15); TreePath childPath = path.Append(i); TrieNode childNode = node.GetChildWithChildPath(TrieStore, ref childPath, i); childTasks.Add(CreateTaskForPath(childPath, childNode, i)); @@ -249,6 +251,7 @@ Task CreateTaskForPath(TreePath childPath, TrieNode childNode, int idx) if (childTasks != null) { Task.WaitAll(childTasks.ToArray()); + childTasks.Dispose(); } } } diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs index c20453347d6..1e250b9ec0a 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs @@ -177,11 +177,13 @@ private void CommitNode(long blockNumber, Hash256? address, ref TreePath path, i { node = SaveOrReplaceInDirtyNodesCache(address, ref path, nodeCommitInfo, node); } - else + + node.LastSeen = Math.Max(blockNumber, node.LastSeen); + + if (!_pruningStrategy.PruningEnabled) { PersistNode(address, path, node, blockNumber, writeFlags); } - node.LastSeen = Math.Max(blockNumber, node.LastSeen); CommittedNodesCount++; } From 2a587b4b9926c94a928bec131331da9f42a7a7fa Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Tue, 8 Oct 2024 12:25:55 +0800 Subject: [PATCH 09/15] Fix testts --- .../SyncServerTests.cs | 3 +- .../Pruning/TreeStoreTests.cs | 351 +++++++++++------- .../Nethermind.Trie.Test/TrieNodeTests.cs | 13 +- 3 files changed, 219 insertions(+), 148 deletions(-) diff --git a/src/Nethermind/Nethermind.Synchronization.Test/SyncServerTests.cs b/src/Nethermind/Nethermind.Synchronization.Test/SyncServerTests.cs index f782fbc8aa5..95cde2d1a4e 100644 --- a/src/Nethermind/Nethermind.Synchronization.Test/SyncServerTests.cs +++ b/src/Nethermind/Nethermind.Synchronization.Test/SyncServerTests.cs @@ -704,7 +704,8 @@ public void GetNodeData_returns_cached_trie_nodes() IScopedTrieStore scopedTrieStore = trieStore.GetTrieStore(null); using (ICommitter committer = scopedTrieStore.BeginCommit(TrieType.State, 1, node)) { - committer.CommitNode(new NodeCommitInfo(node, TreePath.Empty)); + TreePath path = TreePath.Empty; + committer.CommitNode(ref path, new NodeCommitInfo(node)); } stateDb.KeyExists(nodeKey).Should().BeFalse(); diff --git a/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs b/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs index 5dbef0febc9..3fac9693afa 100644 --- a/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs +++ b/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs @@ -70,10 +70,10 @@ public void Memory_with_one_node_is_288() using TrieStore fullTrieStore = CreateTrieStore(pruningStrategy: new TestPruningStrategy(true)); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - TreePath path = TreePath.Empty; + TreePath emptyPath = TreePath.Empty; using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1234, null)) { - committer.CommitNode(ref path, new NodeCommitInfo(trieNode)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode)); } fullTrieStore.MemoryUsedByDirtyCache.Should().Be( trieNode.GetMemorySize(false) + ExpectedPerNodeKeyMemorySize); @@ -89,15 +89,15 @@ public void Pruning_off_cache_should_not_change_commit_node() using TrieStore fullTrieStore = CreateTrieStore(); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - TreePath path = TreePath.Empty; + TreePath emptyPath = TreePath.Empty; using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1234, trieNode)) { - committer.CommitNode(ref path, new NodeCommitInfo(trieNode)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode)); } using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1235, trieNode)) { - committer.CommitNode(ref path, new NodeCommitInfo(trieNode2)); - committer.CommitNode(ref path, new NodeCommitInfo(trieNode3)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode2)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode3)); } fullTrieStore.MemoryUsedByDirtyCache.Should().Be(0); } @@ -112,10 +112,10 @@ public void When_commit_forward_write_flag_if_available() using TrieStore fullTrieStore = CreateTrieStore(kvStore: testMemDb); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - TreePath path = TreePath.Empty; + TreePath emptyPath = TreePath.Empty; using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1234, trieNode, WriteFlags.LowPriority)) { - committer.CommitNode(ref path, new NodeCommitInfo(trieNode)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode)); } if (_scheme == INodeStorage.KeyScheme.HalfPath) @@ -203,18 +203,17 @@ public void Memory_with_two_nodes_is_correct() using TrieStore fullTrieStore = CreateTrieStore(pruningStrategy: new TestPruningStrategy(true)); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - TreePath path = TreePath.Empty; + TreePath emptyPath = TreePath.Empty; using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 1234, null)) { - committer.CommitNode(ref path, new NodeCommitInfo(trieNode1)); - committer.CommitNode(ref path, new NodeCommitInfo(trieNode2)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode1)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode2)); } fullTrieStore.MemoryUsedByDirtyCache.Should().Be( trieNode1.GetMemorySize(false) + ExpectedPerNodeKeyMemorySize + trieNode2.GetMemorySize(false) + ExpectedPerNodeKeyMemorySize); } - /* [Test] public void Memory_with_two_times_two_nodes_is_correct() { @@ -225,11 +224,18 @@ public void Memory_with_two_times_two_nodes_is_correct() using TrieStore fullTrieStore = CreateTrieStore(pruningStrategy: new TestPruningStrategy(true)); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.CommitNode(1234, new NodeCommitInfo(trieNode1, TreePath.Empty)); - trieStore.CommitNode(1234, new NodeCommitInfo(trieNode2, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 1234, trieNode2); - trieStore.CommitNode(1235, new NodeCommitInfo(trieNode3, TreePath.Empty)); - trieStore.CommitNode(1235, new NodeCommitInfo(trieNode4, TreePath.Empty)); + TreePath emptyPath = TreePath.Empty; + using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1234, trieNode2)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode1)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode2)); + } + + using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 1235, trieNode2)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode3)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode4)); + } // depending on whether the node gets resolved it gives different values here in debugging and run // needs some attention @@ -255,13 +261,21 @@ public void Dispatcher_will_try_to_clear_memory() using TrieStore fullTrieStore = CreateTrieStore(pruningStrategy: new MemoryLimit(640)); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.CommitNode(1234, new NodeCommitInfo(trieNode1, TreePath.Empty)); - trieStore.CommitNode(1234, new NodeCommitInfo(trieNode2, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 1234, trieNode2); - trieStore.CommitNode(1235, new NodeCommitInfo(trieNode3, TreePath.Empty)); - trieStore.CommitNode(1235, new NodeCommitInfo(trieNode4, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 1235, trieNode2); - trieStore.FinishBlockCommit(TrieType.State, 1236, trieNode2); + + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 1234, trieNode2)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode1)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode2)); + } + + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 1235, trieNode2)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode3)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode4)); + } + + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 1236, trieNode2)) {} + fullTrieStore.MemoryUsedByDirtyCache.Should().Be( trieNode1.GetMemorySize(false) + ExpectedPerNodeKeyMemorySize + trieNode2.GetMemorySize(false) + ExpectedPerNodeKeyMemorySize + @@ -286,11 +300,19 @@ public void Dispatcher_will_try_to_clear_memory_the_soonest_possible() using TrieStore fullTrieStore = CreateTrieStore(pruningStrategy: new MemoryLimit(512)); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.CommitNode(1234, new NodeCommitInfo(trieNode1, TreePath.Empty)); - trieStore.CommitNode(1234, new NodeCommitInfo(trieNode2, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 1234, trieNode2); - trieStore.CommitNode(1235, new NodeCommitInfo(trieNode3, TreePath.Empty)); - trieStore.CommitNode(1235, new NodeCommitInfo(trieNode4, TreePath.Empty)); + + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 1234, trieNode2)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode1)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode2)); + } + + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 1235, trieNode2)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode3)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode4)); + } + fullTrieStore.MemoryUsedByDirtyCache.Should().Be( trieNode1.GetMemorySize(false) + ExpectedPerNodeKeyMemorySize + trieNode2.GetMemorySize(false) + ExpectedPerNodeKeyMemorySize + @@ -306,16 +328,17 @@ public void Dispatcher_will_always_try_to_clear_memory() TreePath emptyPath = TreePath.Empty; for (int i = 0; i < 1024; i++) { - for (int j = 0; j < 1 + i % 3; j++) - { - TrieNode trieNode = new(NodeType.Leaf, new byte[0]); // 192B - trieNode.ResolveKey(NullTrieNodeResolver.Instance, ref emptyPath, true); - trieStore.CommitNode(i, new NodeCommitInfo(trieNode, TreePath.Empty)); - } - TrieNode fakeRoot = new(NodeType.Leaf, new byte[0]); // 192B fakeRoot.ResolveKey(NullTrieNodeResolver.Instance, ref emptyPath, true); - trieStore.FinishBlockCommit(TrieType.State, i, fakeRoot); + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, i, fakeRoot)) + { + for (int j = 0; j < 1 + i % 3; j++) + { + TrieNode trieNode = new(NodeType.Leaf, new byte[0]); // 192B + trieNode.ResolveKey(NullTrieNodeResolver.Instance, ref emptyPath, true); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode)); + } + } } fullTrieStore.MemoryUsedByDirtyCache.Should().BeLessThan(512 * 2); @@ -337,12 +360,14 @@ public void Dispatcher_will_save_to_db_everything_from_snapshot_blocks() persistenceStrategy: new ConstantInterval(4)); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.CommitNode(0, new NodeCommitInfo(a, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 0, a); - trieStore.FinishBlockCommit(TrieType.State, 1, a); - trieStore.FinishBlockCommit(TrieType.State, 2, a); - trieStore.FinishBlockCommit(TrieType.State, 3, a); - trieStore.FinishBlockCommit(TrieType.State, 4, a); + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 0, a)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(a)); + } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 1, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, a)) {} storage.Get(null, TreePath.Empty, a.Keccak).Should().NotBeNull(); fullTrieStore.IsNodeCached(null, TreePath.Empty, a.Keccak).Should().BeTrue(); @@ -361,11 +386,13 @@ public void Stays_in_memory_until_persisted() using TrieStore fullTrieStore = CreateTrieStore(pruningStrategy: new MemoryLimit(16.MB())); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.CommitNode(0, new NodeCommitInfo(a, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 0, a); - trieStore.FinishBlockCommit(TrieType.State, 1, a); - trieStore.FinishBlockCommit(TrieType.State, 2, a); - trieStore.FinishBlockCommit(TrieType.State, 3, a); + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 0, a)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(a)); + } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 1, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, a)) {} // <- do not persist in this test storage.Get(null, TreePath.Empty, a.Keccak).Should().BeNull(); @@ -400,16 +427,18 @@ public void Will_get_persisted_on_snapshot_if_referenced() ); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.FinishBlockCommit(TrieType.State, 0, null); - trieStore.CommitNode(1, new NodeCommitInfo(a, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 1, a); - trieStore.FinishBlockCommit(TrieType.State, 2, a); - trieStore.FinishBlockCommit(TrieType.State, 3, a); - trieStore.FinishBlockCommit(TrieType.State, 4, a); - trieStore.FinishBlockCommit(TrieType.State, 5, a); - trieStore.FinishBlockCommit(TrieType.State, 6, a); - trieStore.FinishBlockCommit(TrieType.State, 7, a); - trieStore.FinishBlockCommit(TrieType.State, 8, a); + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 0, null)) { } + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 1, a)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(a)); + } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 7, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, a)) {} storage.Get(null, TreePath.Empty, a.Keccak).Should().NotBeNull(); fullTrieStore.IsNodeCached(null, TreePath.Empty, a.Keccak).Should().BeTrue(); @@ -435,17 +464,21 @@ public void Will_not_get_dropped_on_snapshot_if_unreferenced_in_later_blocks() IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.FinishBlockCommit(TrieType.State, 0, null); - trieStore.CommitNode(1, new NodeCommitInfo(a, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 1, a); - trieStore.FinishBlockCommit(TrieType.State, 2, a); - trieStore.FinishBlockCommit(TrieType.State, 3, a); - trieStore.FinishBlockCommit(TrieType.State, 4, a); - trieStore.FinishBlockCommit(TrieType.State, 5, a); - trieStore.FinishBlockCommit(TrieType.State, 6, a); - trieStore.CommitNode(7, new NodeCommitInfo(b, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 7, b); - trieStore.FinishBlockCommit(TrieType.State, 8, b); + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 0, null)) { } + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 1, a)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(a)); + } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, a)) {} + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 7, a)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(b)); + } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, a)) {} nodeStorage.Get(null, TreePath.Empty, a.Keccak).Should().NotBeNull(); fullTrieStore.IsNodeCached(null, TreePath.Empty, a.Keccak).Should().BeTrue(); @@ -470,17 +503,21 @@ public void Will_get_dropped_on_snapshot_if_it_was_a_transient_node() IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.FinishBlockCommit(TrieType.State, 0, null); - trieStore.CommitNode(1, new NodeCommitInfo(a, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 1, a); - trieStore.FinishBlockCommit(TrieType.State, 2, a); - trieStore.CommitNode(3, new NodeCommitInfo(b, TreePath.Empty)); // <- new root - trieStore.FinishBlockCommit(TrieType.State, 3, b); - trieStore.FinishBlockCommit(TrieType.State, 4, b); // should be 'a' to test properly - trieStore.FinishBlockCommit(TrieType.State, 5, b); // should be 'a' to test properly - trieStore.FinishBlockCommit(TrieType.State, 6, b); // should be 'a' to test properly - trieStore.FinishBlockCommit(TrieType.State, 7, b); // should be 'a' to test properly - trieStore.FinishBlockCommit(TrieType.State, 8, b); // should be 'a' to test properly + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 0, null)) { } + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 1, a)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(a)); + } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) {} + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 3, a)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(b)); // <- new root + } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, b)) {} // should be 'a' to test properly + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, b)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, b)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 7, b)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, b)) {} memDb[a.Keccak!.Bytes].Should().BeNull(); fullTrieStore.IsNodeCached(null, TreePath.Empty, a.Keccak).Should().BeTrue(); @@ -563,21 +600,24 @@ public void Will_store_storage_on_snapshot() persistenceStrategy: new ConstantInterval(4)); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.FinishBlockCommit(TrieType.State, 0, null); - trieStore.CommitNode(1, new NodeCommitInfo(a, TreePath.Empty)); - fullTrieStore.GetTrieStore(TestItem.KeccakA) - .CommitNode(1, new NodeCommitInfo(storage1, TreePath.Empty)); - fullTrieStore.GetTrieStore(TestItem.KeccakA) - .FinishBlockCommit(TrieType.Storage, 1, storage1); - trieStore.FinishBlockCommit(TrieType.Storage, 1, storage1); - trieStore.FinishBlockCommit(TrieType.State, 1, a); - trieStore.FinishBlockCommit(TrieType.State, 2, a); - trieStore.FinishBlockCommit(TrieType.State, 3, a); - trieStore.FinishBlockCommit(TrieType.State, 4, a); - trieStore.FinishBlockCommit(TrieType.State, 5, a); - trieStore.FinishBlockCommit(TrieType.State, 6, a); - trieStore.FinishBlockCommit(TrieType.State, 7, a); - trieStore.FinishBlockCommit(TrieType.State, 8, a); + + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 0, null)) { } + using (ICommitter committer = fullTrieStore.GetTrieStore(TestItem.KeccakA).BeginCommit(TrieType.Storage, 1, storage1)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(storage1)); + } + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 1, a)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(a)); + } + + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 7, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, a)) { } asStorage.Get(null, TreePath.Empty, a.Keccak).Should().NotBeNull(); asStorage.Get(TestItem.KeccakA, TreePath.Empty, storage1.Keccak).Should().NotBeNull(); @@ -610,19 +650,27 @@ public void Will_drop_transient_storage() IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.FinishBlockCommit(TrieType.State, 0, null); - trieStore.CommitNode(1, new NodeCommitInfo(a, TreePath.Empty)); - trieStore.CommitNode(1, new NodeCommitInfo(storage1, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.Storage, 1, storage1); - trieStore.FinishBlockCommit(TrieType.State, 1, a); - trieStore.FinishBlockCommit(TrieType.State, 2, a); - trieStore.CommitNode(3, new NodeCommitInfo(b, TreePath.Empty)); // <- new root - trieStore.FinishBlockCommit(TrieType.State, 3, b); - trieStore.FinishBlockCommit(TrieType.State, 4, b); // should be 'a' to test properly - trieStore.FinishBlockCommit(TrieType.State, 5, b); // should be 'a' to test properly - trieStore.FinishBlockCommit(TrieType.State, 6, b); // should be 'a' to test properly - trieStore.FinishBlockCommit(TrieType.State, 7, b); // should be 'a' to test properly - trieStore.FinishBlockCommit(TrieType.State, 8, b); // should be 'a' to test properly + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 0, null)) { } + + using (ICommitter committer = trieStore.BeginCommit(TrieType.Storage, 1, storage1)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(a)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(storage1)); + } + + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 1, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) { } + + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 2, b)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(b)); // <- new root + } + + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, b)) { } // Should be 'a' to test properly + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, b)) { } // Should be 'a' to test properly + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, b)) { } // Should be 'a' to test properly + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 7, b)) { } // Should be 'a' to test properly + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, b)) { } // Should be 'a' to test properly memDb[a.Keccak!.Bytes].Should().BeNull(); memDb[storage1.Keccak!.Bytes].Should().BeNull(); @@ -672,27 +720,32 @@ public void Will_combine_same_storage() IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.FinishBlockCommit(TrieType.State, 0, null); + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 0, null)) { } - IScopedTrieStore storageTrieStore = fullTrieStore.GetTrieStore(new Hash256(Nibbles.ToBytes(storage1Nib))); - storageTrieStore.CommitNode(1, new NodeCommitInfo(storage1, TreePath.Empty)); - storageTrieStore.FinishBlockCommit(TrieType.Storage, 1, storage1); + using (ICommitter committer = fullTrieStore.GetTrieStore(new Hash256(Nibbles.ToBytes(storage1Nib))).BeginCommit(TrieType.Storage, 1, storage1)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(storage1)); + } - storageTrieStore = fullTrieStore.GetTrieStore(new Hash256(Nibbles.ToBytes(storage2Nib))); - storageTrieStore.CommitNode(1, new NodeCommitInfo(storage2, TreePath.Empty)); - storageTrieStore.FinishBlockCommit(TrieType.Storage, 1, storage2); + using (ICommitter committer = fullTrieStore.GetTrieStore(new Hash256(Nibbles.ToBytes(storage2Nib))).BeginCommit(TrieType.Storage, 1, storage2)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(storage2)); + } - trieStore.CommitNode(1, new NodeCommitInfo(a, TreePath.Empty)); - trieStore.CommitNode(1, new NodeCommitInfo(b, TreePath.Empty)); - trieStore.CommitNode(1, new NodeCommitInfo(branch, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 1, branch); - trieStore.FinishBlockCommit(TrieType.State, 2, branch); - trieStore.FinishBlockCommit(TrieType.State, 3, branch); - trieStore.FinishBlockCommit(TrieType.State, 4, branch); - trieStore.FinishBlockCommit(TrieType.State, 5, branch); - trieStore.FinishBlockCommit(TrieType.State, 6, branch); - trieStore.FinishBlockCommit(TrieType.State, 7, branch); - trieStore.FinishBlockCommit(TrieType.State, 8, branch); + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 1, branch)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(a)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(b)); + committer.CommitNode(ref emptyPath, new NodeCommitInfo(branch)); + } + + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, branch)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, branch)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, branch)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, branch)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, branch)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 7, branch)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, branch)) {} storage.Get(null, TreePath.FromNibble(new byte[] { 0 }), a.Keccak).Should().NotBeNull(); storage.Get(new Hash256(Nibbles.ToBytes(storage1Nib)), TreePath.Empty, storage1.Keccak).Should().NotBeNull(); @@ -721,7 +774,10 @@ public async Task Read_only_trie_store_is_allowing_many_thread_to_work_with_the_ IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); TreePath emptyPath = TreePath.Empty; trieNode.ResolveKey(trieStore, ref emptyPath, false); - trieStore.CommitNode(1, new NodeCommitInfo(trieNode, TreePath.Empty)); + using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 0, trieNode)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode)); + } if (beThreadSafe) { @@ -774,8 +830,12 @@ public void ReadOnly_store_returns_copies(bool pruning) using TrieStore fullTrieStore = CreateTrieStore(pruningStrategy: new TestPruningStrategy(pruning)); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); - trieStore.CommitNode(0, new NodeCommitInfo(node, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 0, node); + + using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 0, node)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(node)); + } + var originalNode = trieStore.FindCachedOrUnknown(TreePath.Empty, node.Keccak); IReadOnlyTrieStore readOnlyTrieStore = fullTrieStore.AsReadOnly(); @@ -792,12 +852,9 @@ public void ReadOnly_store_returns_copies(bool pruning) readOnlyNode.Key?.ToString().Should().Be(originalNode.Key?.ToString()); } - */ private long ExpectedPerNodeKeyMemorySize => _scheme == INodeStorage.KeyScheme.Hash ? 0 : TrieStoreDirtyNodesCache.Key.MemoryUsage; - /* - [Test] public void After_commit_should_have_has_root() { @@ -829,12 +886,15 @@ public async Task Will_RemovePastKeys_OnSnapshot() persistenceStrategy: No.Persistence); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); + TreePath emptyPath = TreePath.Empty; for (int i = 0; i < 64; i++) { TrieNode node = new(NodeType.Leaf, TestItem.Keccaks[i], new byte[2]); - trieStore.CommitNode(i, new NodeCommitInfo(node, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, i, node); + using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, i, node)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(node)); + } // Pruning is done in background await Task.Delay(TimeSpan.FromMilliseconds(10)); @@ -864,12 +924,15 @@ public async Task Will_Trigger_ReorgBoundaryEvent_On_Prune() fullTrieStore.ReorgBoundaryReached += (sender, reached) => reorgBoundary = reached.BlockNumber; IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); + TreePath emptyPath = TreePath.Empty; for (int i = 0; i < 64; i++) { TrieNode node = new(NodeType.Leaf, TestItem.Keccaks[i], new byte[2]); - trieStore.CommitNode(i, new NodeCommitInfo(node, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, i, node); + using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, i, node)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(node)); + } if (i > 4) { @@ -897,12 +960,15 @@ public async Task Will_Not_RemovePastKeys_OnSnapshot_DuringFullPruning() persistenceStrategy: isPruningPersistenceStrategy); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); + TreePath emptyPath = TreePath.Empty; for (int i = 0; i < 64; i++) { TrieNode node = new(NodeType.Leaf, TestItem.Keccaks[i], new byte[2]); - trieStore.CommitNode(i, new NodeCommitInfo(node, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, i, node); + using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, i, node)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(node)); + } // Pruning is done in background await Task.Delay(TimeSpan.FromMilliseconds(10)); @@ -924,13 +990,15 @@ public async Task Will_NotRemove_ReCommittedNode() persistenceStrategy: No.Persistence); IScopedTrieStore trieStore = fullTrieStore.GetTrieStore(null); + TreePath emptyPath = TreePath.Empty; for (int i = 0; i < 64; i++) { TrieNode node = new(NodeType.Leaf, TestItem.Keccaks[i % 4], new byte[2]); - trieStore.CommitNode(i, new NodeCommitInfo(node, TreePath.Empty)); - node = trieStore.FindCachedOrUnknown(TreePath.Empty, node.Keccak); - trieStore.FinishBlockCommit(TrieType.State, i, node); + using (ICommitter committer = trieStore.BeginCommit(TrieType.State, i, node)) + { + committer.CommitNode(ref emptyPath, new NodeCommitInfo(node)); + } // Pruning is done in background await Task.Delay(TimeSpan.FromMilliseconds(10)); @@ -938,6 +1006,5 @@ public async Task Will_NotRemove_ReCommittedNode() memDb.Count.Should().Be(4); } - */ } } diff --git a/src/Nethermind/Nethermind.Trie.Test/TrieNodeTests.cs b/src/Nethermind/Nethermind.Trie.Test/TrieNodeTests.cs index 1c3a67a5988..001be11d7d7 100644 --- a/src/Nethermind/Nethermind.Trie.Test/TrieNodeTests.cs +++ b/src/Nethermind/Nethermind.Trie.Test/TrieNodeTests.cs @@ -925,7 +925,6 @@ void CheckChildren() [Test] public void Rlp_is_cloned_when_cloning() { - /* IScopedTrieStore trieStore = new TrieStore(new MemDb(), NullLogManager.Instance).GetTrieStore(null); TrieNode leaf1 = new(NodeType.Leaf); @@ -934,15 +933,20 @@ public void Rlp_is_cloned_when_cloning() TreePath emptyPath = TreePath.Empty; leaf1.ResolveKey(trieStore, ref emptyPath, false); leaf1.Seal(); - trieStore.CommitNode(0, new NodeCommitInfo(leaf1, TreePath.Empty)); TrieNode leaf2 = new(NodeType.Leaf); leaf2.Key = Bytes.FromHexString("abd"); leaf2.Value = new byte[222]; leaf2.ResolveKey(trieStore, ref emptyPath, false); leaf2.Seal(); - trieStore.CommitNode(0, new NodeCommitInfo(leaf2, TreePath.Empty)); - trieStore.FinishBlockCommit(TrieType.State, 0, leaf2); + + TreePath path = TreePath.Empty; + + using (ICommitter? committer = trieStore.BeginCommit(TrieType.State, 0, leaf2)) + { + committer.CommitNode(ref path, new NodeCommitInfo(leaf1)); + committer.CommitNode(ref path, new NodeCommitInfo(leaf2)); + } TrieNode trieNode = new(NodeType.Branch); trieNode.SetChild(1, leaf1); @@ -957,7 +961,6 @@ public void Rlp_is_cloned_when_cloning() restoredLeaf1.Should().NotBeNull(); restoredLeaf1.ResolveNode(trieStore, TreePath.Empty); restoredLeaf1.Value.ToArray().Should().BeEquivalentTo(leaf1.Value.ToArray()); - */ } [Test] From 421bfcdb0c85505915b0f74a49aec5b3815c0aa9 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Tue, 8 Oct 2024 12:49:14 +0800 Subject: [PATCH 10/15] Remove unnecessary check --- .../Nethermind.Trie/Pruning/TrieStore.cs | 109 ++++++++---------- 1 file changed, 50 insertions(+), 59 deletions(-) diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs index 1e250b9ec0a..9d897f0c368 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs @@ -163,11 +163,6 @@ private void CommitNode(long blockNumber, Hash256? address, ref TreePath path, i ThrowUnknownHash(node); } - if (CurrentPackage is null) - { - ThrowUnknownPackage(blockNumber, node); - } - if (node!.LastSeen >= 0) { ThrowNodeHasBeenSeen(blockNumber, node); @@ -198,10 +193,6 @@ void Trace(long blockNumber, in NodeCommitInfo nodeCommitInfo) [StackTraceHidden] static void ThrowUnknownHash(TrieNode node) => throw new TrieStoreException($"The hash of {node} should be known at the time of committing."); - [DoesNotReturn] - [StackTraceHidden] - static void ThrowUnknownPackage(long blockNumber, TrieNode node) => throw new TrieStoreException($"{nameof(CurrentPackage)} is NULL when committing {node} at {blockNumber}."); - [DoesNotReturn] [StackTraceHidden] static void ThrowNodeHasBeenSeen(long blockNumber, TrieNode node) => throw new TrieStoreException($"{nameof(TrieNode.LastSeen)} set on {node} committed at {blockNumber}."); @@ -299,57 +290,8 @@ public ICommitter BeginCommit(TrieType trieType, long blockNumber, Hash256? addr return new TrieStoreCommitter(this, trieType, blockNumber, address, root, writeFlags, concurrency); } - private class TrieStoreCommitter( - TrieStore trieStore, - TrieType trieType, - long blockNumber, - Hash256? address, - TrieNode? root, - WriteFlags writeFlags, - int concurrency - ) : ICommitter - { - private bool _needToResetRoot = root is not null && root.IsDirty; - private int _concurrency = concurrency; - - public void Dispose() - { - if (_needToResetRoot) - { - root = trieStore.FindCachedOrUnknown(address, TreePath.Empty, root.Keccak); - } - trieStore.FinishBlockCommit(trieType, blockNumber, address, root, writeFlags); - } - - public void CommitNode(ref TreePath path, NodeCommitInfo nodeCommitInfo) - { - trieStore.CommitNode(blockNumber, address, ref path, nodeCommitInfo, writeFlags: writeFlags); - } - - public bool CanSpawnTask() - { - if (Interlocked.Decrement(ref _concurrency) < 0) - { - ReturnConcurrencyQuota(); - return false; - } - else - { - return true; - } - } - - public void ReturnConcurrencyQuota() - { - Interlocked.Increment(ref _concurrency); - } - } - - public void FinishBlockCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) + private void FinishBlockCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) { - ArgumentOutOfRangeException.ThrowIfNegative(blockNumber); - EnsureCommitSetExistsForBlock(blockNumber); - try { if (trieType == TrieType.State) // storage tries happen before state commits @@ -1177,6 +1119,55 @@ public bool HasRoot(Hash256 stateRoot) return true; } + private class TrieStoreCommitter( + TrieStore trieStore, + TrieType trieType, + long blockNumber, + Hash256? address, + TrieNode? root, + WriteFlags writeFlags, + int concurrency + ) : ICommitter + { + private bool _needToResetRoot = root is not null && root.IsDirty; + private int _concurrency = concurrency; + + public void Dispose() + { + if (_needToResetRoot) + { + // During commit it PatriciaTrie, the root may get resolved to an existing node (same keccak). + // This ensure that the root that we use here is the same. + root = trieStore.FindCachedOrUnknown(address, TreePath.Empty, root.Keccak); + } + trieStore.FinishBlockCommit(trieType, blockNumber, address, root, writeFlags); + } + + public void CommitNode(ref TreePath path, NodeCommitInfo nodeCommitInfo) + { + trieStore.CommitNode(blockNumber, address, ref path, nodeCommitInfo, writeFlags: writeFlags); + } + + public bool CanSpawnTask() + { + if (Interlocked.Decrement(ref _concurrency) < 0) + { + ReturnConcurrencyQuota(); + return false; + } + else + { + return true; + } + } + + public void ReturnConcurrencyQuota() + { + Interlocked.Increment(ref _concurrency); + } + } + + internal static class HashHelpers { private const int HashPrime = 101; From 59071c0e8868fc87fcbed199a1fee52f876013a2 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Tue, 8 Oct 2024 13:02:09 +0800 Subject: [PATCH 11/15] Minor cleanup --- src/Nethermind/Nethermind.Trie/PatriciaTree.cs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs index ed23c9579b9..7f32793bd20 100644 --- a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs +++ b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs @@ -40,11 +40,6 @@ public class PatriciaTree public TrieType TrieType { get; init; } private Stack? _nodeStack; - -#pragma warning disable CS0169 // Field is never used - private ConcurrentQueue? _commitExceptions; -#pragma warning restore CS0169 // Field is never used - public IScopedTrieStore TrieStore { get; } public ICappedArrayPool? _bufferPool; @@ -177,7 +172,7 @@ void Debug(long blockNumber) } } - private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo nodeCommitInfo, bool skipSelf = false, int maxLevelForConcurrentCommit = -1) + private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo nodeCommitInfo, int maxLevelForConcurrentCommit, bool skipSelf = false) { if (!_allowCommits) { @@ -195,7 +190,7 @@ private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo node { path.AppendMut(i); TrieNode childNode = node.GetChildWithChildPath(TrieStore, ref path, i); - Commit(committer, ref path, new NodeCommitInfo(childNode!, node, i)); + Commit(committer, ref path, new NodeCommitInfo(childNode!, node, i), maxLevelForConcurrentCommit); path.TruncateOne(); } else @@ -213,7 +208,7 @@ Task CreateTaskForPath(TreePath childPath, TrieNode childNode, int idx) { return Task.Run(() => { - Commit(committer, ref childPath, new NodeCommitInfo(childNode!, node, idx)); + Commit(committer, ref childPath, new NodeCommitInfo(childNode!, node, idx), maxLevelForConcurrentCommit); committer.ReturnConcurrencyQuota(); }); } @@ -235,7 +230,7 @@ Task CreateTaskForPath(TreePath childPath, TrieNode childNode, int idx) { path.AppendMut(i); TrieNode childNode = node.GetChildWithChildPath(TrieStore, ref path, i); - Commit(committer, ref path, new NodeCommitInfo(childNode!, node, i)); + Commit(committer, ref path, new NodeCommitInfo(childNode!, node, i), maxLevelForConcurrentCommit); path.TruncateOne(); } } @@ -266,7 +261,7 @@ Task CreateTaskForPath(TreePath childPath, TrieNode childNode, int idx) if (extensionChild.IsDirty) { - Commit(committer, ref path, new NodeCommitInfo(extensionChild, node, 0)); + Commit(committer, ref path, new NodeCommitInfo(extensionChild, node, 0), maxLevelForConcurrentCommit); } else { From 2d260e6d35e92501e7b67207505d9dd19475313e Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Tue, 8 Oct 2024 13:09:21 +0800 Subject: [PATCH 12/15] Whitespace --- .../Pruning/TreeStoreTests.cs | 68 +++++++++---------- .../Nethermind.Trie/PatriciaTree.cs | 2 +- .../Pruning/IScopedTrieStore.cs | 4 +- .../Nethermind.Trie/Pruning/NullTrieStore.cs | 2 +- 4 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs b/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs index 3fac9693afa..7d78cc8443f 100644 --- a/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs +++ b/src/Nethermind/Nethermind.Trie.Test/Pruning/TreeStoreTests.cs @@ -274,7 +274,7 @@ public void Dispatcher_will_try_to_clear_memory() committer.CommitNode(ref emptyPath, new NodeCommitInfo(trieNode4)); } - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 1236, trieNode2)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 1236, trieNode2)) { } fullTrieStore.MemoryUsedByDirtyCache.Should().Be( trieNode1.GetMemorySize(false) + ExpectedPerNodeKeyMemorySize + @@ -364,10 +364,10 @@ public void Dispatcher_will_save_to_db_everything_from_snapshot_blocks() { committer.CommitNode(ref emptyPath, new NodeCommitInfo(a)); } - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 1, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 1, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, a)) { } storage.Get(null, TreePath.Empty, a.Keccak).Should().NotBeNull(); fullTrieStore.IsNodeCached(null, TreePath.Empty, a.Keccak).Should().BeTrue(); @@ -390,9 +390,9 @@ public void Stays_in_memory_until_persisted() { committer.CommitNode(ref emptyPath, new NodeCommitInfo(a)); } - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 1, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 1, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, a)) { } // <- do not persist in this test storage.Get(null, TreePath.Empty, a.Keccak).Should().BeNull(); @@ -432,13 +432,13 @@ public void Will_get_persisted_on_snapshot_if_referenced() { committer.CommitNode(ref emptyPath, new NodeCommitInfo(a)); } - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 7, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 7, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, a)) { } storage.Get(null, TreePath.Empty, a.Keccak).Should().NotBeNull(); fullTrieStore.IsNodeCached(null, TreePath.Empty, a.Keccak).Should().BeTrue(); @@ -469,16 +469,16 @@ public void Will_not_get_dropped_on_snapshot_if_unreferenced_in_later_blocks() { committer.CommitNode(ref emptyPath, new NodeCommitInfo(a)); } - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, a)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, a)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, a)) { } using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 7, a)) { committer.CommitNode(ref emptyPath, new NodeCommitInfo(b)); } - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, a)) { } nodeStorage.Get(null, TreePath.Empty, a.Keccak).Should().NotBeNull(); fullTrieStore.IsNodeCached(null, TreePath.Empty, a.Keccak).Should().BeTrue(); @@ -508,16 +508,16 @@ public void Will_get_dropped_on_snapshot_if_it_was_a_transient_node() { committer.CommitNode(ref emptyPath, new NodeCommitInfo(a)); } - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, a)) { } using (ICommitter committer = trieStore.BeginCommit(TrieType.State, 3, a)) { committer.CommitNode(ref emptyPath, new NodeCommitInfo(b)); // <- new root } - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, b)) {} // should be 'a' to test properly - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, b)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, b)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 7, b)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, b)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, b)) { } // should be 'a' to test properly + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, b)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, b)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 7, b)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, b)) { } memDb[a.Keccak!.Bytes].Should().BeNull(); fullTrieStore.IsNodeCached(null, TreePath.Empty, a.Keccak).Should().BeTrue(); @@ -739,13 +739,13 @@ public void Will_combine_same_storage() committer.CommitNode(ref emptyPath, new NodeCommitInfo(branch)); } - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, branch)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, branch)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, branch)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, branch)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, branch)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 7, branch)) {} - using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, branch)) {} + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 2, branch)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 3, branch)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 4, branch)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 5, branch)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 6, branch)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 7, branch)) { } + using (ICommitter _ = trieStore.BeginCommit(TrieType.State, 8, branch)) { } storage.Get(null, TreePath.FromNibble(new byte[] { 0 }), a.Keccak).Should().NotBeNull(); storage.Get(new Hash256(Nibbles.ToBytes(storage1Nib)), TreePath.Empty, storage1.Keccak).Should().NotBeNull(); diff --git a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs index 7f32793bd20..daf5568cc9a 100644 --- a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs +++ b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs @@ -143,7 +143,7 @@ public void Commit(long blockNumber, bool skipRoot = false, WriteFlags writeFlag if (_writeBeforeCommit > 0) { maxLevelForConcurrentCommit++; // Ok, we separate at top level - if (_writeBeforeCommit/16 > 0) + if (_writeBeforeCommit / 16 > 0) { maxLevelForConcurrentCommit++; // Another level } diff --git a/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs index a44cd2081af..3dec5d49e25 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/IScopedTrieStore.cs @@ -22,10 +22,10 @@ public interface IScopedTrieStore : ITrieNodeResolver void Set(in TreePath path, in ValueHash256 keccak, byte[] rlp); } -public interface ICommitter: IDisposable +public interface ICommitter : IDisposable { void CommitNode(ref TreePath path, NodeCommitInfo nodeCommitInfo); bool CanSpawnTask() => false; - void ReturnConcurrencyQuota() {} + void ReturnConcurrencyQuota() { } } diff --git a/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs index 47b0d924bab..b298f69611b 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs @@ -39,7 +39,7 @@ public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256 storageRoot) public INodeStorage.KeyScheme Scheme => INodeStorage.KeyScheme.HalfPath; - internal class NullCommitter: ICommitter + internal class NullCommitter : ICommitter { public void Dispose() { From c415a534e70df14f690b0c90ed3d856903794912 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Thu, 10 Oct 2024 10:40:46 +0200 Subject: [PATCH 13/15] Small refactors and simplifications --- .../Nethermind.Trie/CachedTrieStore.cs | 44 +++---- .../Nethermind.Trie/NodeCommitInfo.cs | 3 + .../Nethermind.Trie/PatriciaTree.cs | 28 ++--- .../Nethermind.Trie/Pruning/NullTrieStore.cs | 26 ++--- .../Pruning/ReadOnlyTrieStore.cs | 107 +++++------------- .../Pruning/ScopedTrieStore.cs | 70 ++++-------- .../Nethermind.Trie/Pruning/TreePath.cs | 6 +- .../Nethermind.Trie/Pruning/TrieStore.cs | 45 ++++---- .../Nethermind.Trie/TrieStoreWithReadFlags.cs | 28 ++--- 9 files changed, 119 insertions(+), 238 deletions(-) diff --git a/src/Nethermind/Nethermind.Trie/CachedTrieStore.cs b/src/Nethermind/Nethermind.Trie/CachedTrieStore.cs index b1429a4bb5a..5b23ba6ad92 100644 --- a/src/Nethermind/Nethermind.Trie/CachedTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/CachedTrieStore.cs @@ -20,41 +20,25 @@ public class CachedTrieStore(IScopedTrieStore @base) : IScopedTrieStore { private readonly NonBlocking.ConcurrentDictionary<(TreePath path, Hash256 hash), TrieNode> _cachedNode = new(); - public TrieNode FindCachedOrUnknown(in TreePath path, Hash256 hash) - { - return _cachedNode.GetOrAdd((path, hash), (key) => @base.FindCachedOrUnknown(key.path, key.hash)); - } - - public byte[]? LoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) - { - return @base.LoadRlp(in path, hash, flags); - } - - public byte[]? TryLoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) - { - return @base.TryLoadRlp(in path, hash, flags); - } - - public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256? address) - { + public TrieNode FindCachedOrUnknown(in TreePath path, Hash256 hash) => + _cachedNode.GetOrAdd((path, hash), (key) => @base.FindCachedOrUnknown(key.path, key.hash)); + + public byte[]? LoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) => + @base.LoadRlp(in path, hash, flags); + + public byte[]? TryLoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) => + @base.TryLoadRlp(in path, hash, flags); + + public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256? address) => throw new InvalidOperationException("unsupported"); - } public INodeStorage.KeyScheme Scheme => @base.Scheme; - public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) - { - return @base.BeginCommit(trieType, blockNumber, root, writeFlags); - } + public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) => + @base.BeginCommit(trieType, blockNumber, root, writeFlags); - public bool IsPersisted(in TreePath path, in ValueHash256 keccak) - { - return @base.IsPersisted(in path, in keccak); - } + public bool IsPersisted(in TreePath path, in ValueHash256 keccak) => @base.IsPersisted(in path, in keccak); - public void Set(in TreePath path, in ValueHash256 keccak, byte[] rlp) - { - @base.Set(in path, in keccak, rlp); - } + public void Set(in TreePath path, in ValueHash256 keccak, byte[] rlp) => @base.Set(in path, in keccak, rlp); } diff --git a/src/Nethermind/Nethermind.Trie/NodeCommitInfo.cs b/src/Nethermind/Nethermind.Trie/NodeCommitInfo.cs index 42260715717..265d81631fb 100644 --- a/src/Nethermind/Nethermind.Trie/NodeCommitInfo.cs +++ b/src/Nethermind/Nethermind.Trie/NodeCommitInfo.cs @@ -1,3 +1,5 @@ +using System.Diagnostics.CodeAnalysis; + namespace Nethermind.Trie { public readonly struct NodeCommitInfo @@ -27,6 +29,7 @@ public NodeCommitInfo( public int ChildPositionAtParent { get; } + [MemberNotNullWhen(false, nameof(Node))] public bool IsEmptyBlockMarker => Node is null; public bool IsRoot => !IsEmptyBlockMarker && NodeParent is null; diff --git a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs index daf5568cc9a..4438fbda157 100644 --- a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs +++ b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs @@ -138,16 +138,13 @@ public void Commit(long blockNumber, bool skipRoot = false, WriteFlags writeFlag ThrowReadOnlyTrieException(); } - int maxLevelForConcurrentCommit = -1; - _writeBeforeCommit /= 64; - if (_writeBeforeCommit > 0) + int maxLevelForConcurrentCommit = _writeBeforeCommit switch { - maxLevelForConcurrentCommit++; // Ok, we separate at top level - if (_writeBeforeCommit / 16 > 0) - { - maxLevelForConcurrentCommit++; // Another level - } - } + > 64 * 16 => 1, // we separate at two top levels + > 64 => 0, // we separate at top level + _ => -1 + }; + _writeBeforeCommit = 0; using (ICommitter committer = TrieStore.BeginCommit(TrieType, blockNumber, RootRef, writeFlags)) @@ -204,14 +201,11 @@ private void Commit(ICommitter committer, ref TreePath path, NodeCommitInfo node } else { - Task CreateTaskForPath(TreePath childPath, TrieNode childNode, int idx) + Task CreateTaskForPath(TreePath childPath, TrieNode childNode, int idx) => Task.Run(() => { - return Task.Run(() => - { - Commit(committer, ref childPath, new NodeCommitInfo(childNode!, node, idx), maxLevelForConcurrentCommit); - committer.ReturnConcurrencyQuota(); - }); - } + Commit(committer, ref childPath, new NodeCommitInfo(childNode!, node, idx), maxLevelForConcurrentCommit); + committer.ReturnConcurrencyQuota(); + }); ArrayPoolList? childTasks = null; @@ -243,7 +237,7 @@ Task CreateTaskForPath(TreePath childPath, TrieNode childNode, int idx) } } - if (childTasks != null) + if (childTasks is not null) { Task.WaitAll(childTasks.ToArray()); childTasks.Dispose(); diff --git a/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs index b298f69611b..585050f7150 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/NullTrieStore.cs @@ -17,37 +17,25 @@ private NullTrieStore() { } public TrieNode FindCachedOrUnknown(in TreePath treePath, Hash256 hash) => new(NodeType.Unknown, hash); - public byte[] LoadRlp(in TreePath treePath, Hash256 hash, ReadFlags flags = ReadFlags.None) => Array.Empty(); + public byte[] LoadRlp(in TreePath treePath, Hash256 hash, ReadFlags flags = ReadFlags.None) => []; - public byte[]? TryLoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) => Array.Empty(); + public byte[]? TryLoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) => []; - public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) - { - return new NullCommitter(); - } + public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) => new NullCommitter(); public bool IsPersisted(in TreePath path, in ValueHash256 keccak) => true; - public void Set(in TreePath path, in ValueHash256 keccak, byte[] rlp) - { - } + public void Set(in TreePath path, in ValueHash256 keccak, byte[] rlp) { } - public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256 storageRoot) - { - return this; - } + public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256 storageRoot) => this; public INodeStorage.KeyScheme Scheme => INodeStorage.KeyScheme.HalfPath; internal class NullCommitter : ICommitter { - public void Dispose() - { - } + public void Dispose() { } - public void CommitNode(ref TreePath path, NodeCommitInfo nodeCommitInfo) - { - } + public void CommitNode(ref TreePath path, NodeCommitInfo nodeCommitInfo) { } } } } diff --git a/src/Nethermind/Nethermind.Trie/Pruning/ReadOnlyTrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/ReadOnlyTrieStore.cs index b168bcfd34a..4bb2c8d537f 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/ReadOnlyTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/ReadOnlyTrieStore.cs @@ -12,37 +12,25 @@ namespace Nethermind.Trie.Pruning /// /// Safe to be reused for the same wrapped store. /// - public class ReadOnlyTrieStore : IReadOnlyTrieStore + public class ReadOnlyTrieStore(TrieStore trieStore, INodeStorage? readOnlyStore) : IReadOnlyTrieStore { - private readonly TrieStore _trieStore; - private readonly INodeStorage? _readOnlyStore; + private readonly TrieStore _trieStore = trieStore ?? throw new ArgumentNullException(nameof(trieStore)); public INodeStorage.KeyScheme Scheme => _trieStore.Scheme; - public ReadOnlyTrieStore(TrieStore trieStore, INodeStorage? readOnlyStore) - { - _trieStore = trieStore ?? throw new ArgumentNullException(nameof(trieStore)); - _readOnlyStore = readOnlyStore; - } - public TrieNode FindCachedOrUnknown(Hash256? address, in TreePath treePath, Hash256 hash) => _trieStore.FindCachedOrUnknown(address, treePath, hash, true); public byte[] LoadRlp(Hash256? address, in TreePath treePath, Hash256 hash, ReadFlags flags) => - _trieStore.LoadRlp(address, treePath, hash, _readOnlyStore, flags); + _trieStore.LoadRlp(address, treePath, hash, readOnlyStore, flags); public byte[]? TryLoadRlp(Hash256? address, in TreePath treePath, Hash256 hash, ReadFlags flags) => - _trieStore.TryLoadRlp(address, treePath, hash, _readOnlyStore, flags); + _trieStore.TryLoadRlp(address, treePath, hash, readOnlyStore, flags); public bool IsPersisted(Hash256? address, in TreePath path, in ValueHash256 keccak) => _trieStore.IsPersisted(address, path, keccak); - public IReadOnlyTrieStore AsReadOnly(INodeStorage nodeStore) - { - return new ReadOnlyTrieStore(_trieStore, nodeStore); - } + public IReadOnlyTrieStore AsReadOnly(INodeStorage nodeStore) => new ReadOnlyTrieStore(_trieStore, nodeStore); - public ICommitter BeginCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags) - { - return new NullTrieStore.NullCommitter(); - } + public ICommitter BeginCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags) => + new NullTrieStore.NullCommitter(); public event EventHandler ReorgBoundaryReached { @@ -52,69 +40,36 @@ public event EventHandler ReorgBoundaryReached public IReadOnlyKeyValueStore TrieNodeRlpStore => _trieStore.TrieNodeRlpStore; - public void Set(Hash256? address, in TreePath path, in ValueHash256 keccak, byte[] rlp) - { - } + public void Set(Hash256? address, in TreePath path, in ValueHash256 keccak, byte[] rlp) { } - public IScopedTrieStore GetTrieStore(Hash256? address) - { - return new ScopedReadOnlyTrieStore(this, address); - } + public IScopedTrieStore GetTrieStore(Hash256? address) => new ScopedReadOnlyTrieStore(this, address); - public bool HasRoot(Hash256 stateRoot) - { - return _trieStore.HasRoot(stateRoot); - } + public bool HasRoot(Hash256 stateRoot) => _trieStore.HasRoot(stateRoot); public void Dispose() { } - private class ScopedReadOnlyTrieStore : IScopedTrieStore + private class ScopedReadOnlyTrieStore(ReadOnlyTrieStore fullTrieStore, Hash256? address) : IScopedTrieStore { - private readonly ReadOnlyTrieStore _trieStoreImplementation; - private readonly Hash256? _address; - - public ScopedReadOnlyTrieStore(ReadOnlyTrieStore fullTrieStore, Hash256? address) - { - _trieStoreImplementation = fullTrieStore; - _address = address; - } - - public TrieNode FindCachedOrUnknown(in TreePath path, Hash256 hash) - { - return _trieStoreImplementation.FindCachedOrUnknown(_address, path, hash); - } - - public byte[]? LoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) - { - return _trieStoreImplementation.LoadRlp(_address, path, hash, flags); - } - - public byte[]? TryLoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) - { - return _trieStoreImplementation.TryLoadRlp(_address, path, hash, flags); - } - - public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256? address) - { - if (address == _address) return this; - return new ScopedReadOnlyTrieStore(_trieStoreImplementation, address); - } - - public INodeStorage.KeyScheme Scheme => _trieStoreImplementation.Scheme; - - public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) - { - return new NullTrieStore.NullCommitter(); - } - - public bool IsPersisted(in TreePath path, in ValueHash256 keccak) - { - return _trieStoreImplementation.IsPersisted(_address, path, in keccak); - } - - public void Set(in TreePath path, in ValueHash256 keccak, byte[] rlp) - { - } + public TrieNode FindCachedOrUnknown(in TreePath path, Hash256 hash) => + fullTrieStore.FindCachedOrUnknown(address, path, hash); + + public byte[]? LoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) => + fullTrieStore.LoadRlp(address, path, hash, flags); + + public byte[]? TryLoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) => fullTrieStore.TryLoadRlp(address, path, hash, flags); + + public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256? address1) => + address1 == address ? this : new ScopedReadOnlyTrieStore(fullTrieStore, address1); + + public INodeStorage.KeyScheme Scheme => fullTrieStore.Scheme; + + public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) => + new NullTrieStore.NullCommitter(); + + public bool IsPersisted(in TreePath path, in ValueHash256 keccak) => + fullTrieStore.IsPersisted(address, path, in keccak); + + public void Set(in TreePath path, in ValueHash256 keccak, byte[] rlp) { } } } } diff --git a/src/Nethermind/Nethermind.Trie/Pruning/ScopedTrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/ScopedTrieStore.cs index f53e1089b14..650ee399086 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/ScopedTrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/ScopedTrieStore.cs @@ -6,52 +6,28 @@ namespace Nethermind.Trie.Pruning; -public sealed class ScopedTrieStore : IScopedTrieStore +public sealed class ScopedTrieStore(ITrieStore fullTrieStore, Hash256? address) : IScopedTrieStore { - private readonly ITrieStore _trieStoreImplementation; - private readonly Hash256? _address; - - public ScopedTrieStore(ITrieStore fullTrieStore, Hash256? address) - { - _trieStoreImplementation = fullTrieStore; - _address = address; - } - - public TrieNode FindCachedOrUnknown(in TreePath path, Hash256 hash) - { - return _trieStoreImplementation.FindCachedOrUnknown(_address, path, hash); - } - - public byte[]? LoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) - { - return _trieStoreImplementation.LoadRlp(_address, path, hash, flags); - } - - public byte[]? TryLoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) - { - return _trieStoreImplementation.TryLoadRlp(_address, path, hash, flags); - } - - public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256? address) - { - if (address == _address) return this; - return new ScopedTrieStore(_trieStoreImplementation, address); - } - - public INodeStorage.KeyScheme Scheme => _trieStoreImplementation.Scheme; - - public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) - { - return _trieStoreImplementation.BeginCommit(trieType, blockNumber, _address, root, writeFlags); - } - - public bool IsPersisted(in TreePath path, in ValueHash256 keccak) - { - return _trieStoreImplementation.IsPersisted(_address, path, in keccak); - } - - public void Set(in TreePath path, in ValueHash256 keccak, byte[] rlp) - { - _trieStoreImplementation.Set(_address, path, keccak, rlp); - } + public TrieNode FindCachedOrUnknown(in TreePath path, Hash256 hash) => + fullTrieStore.FindCachedOrUnknown(address, path, hash); + + public byte[]? LoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) => + fullTrieStore.LoadRlp(address, path, hash, flags); + + public byte[]? TryLoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) => + fullTrieStore.TryLoadRlp(address, path, hash, flags); + + public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256? address1) => + address1 == address ? this : new ScopedTrieStore(fullTrieStore, address1); + + public INodeStorage.KeyScheme Scheme => fullTrieStore.Scheme; + + public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) => + fullTrieStore.BeginCommit(trieType, blockNumber, address, root, writeFlags); + + public bool IsPersisted(in TreePath path, in ValueHash256 keccak) => + fullTrieStore.IsPersisted(address, path, in keccak); + + public void Set(in TreePath path, in ValueHash256 keccak, byte[] rlp) => + fullTrieStore.Set(address, path, keccak, rlp); } diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TreePath.cs b/src/Nethermind/Nethermind.Trie/Pruning/TreePath.cs index ba768f5844a..36306f16241 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/TreePath.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/TreePath.cs @@ -27,7 +27,7 @@ public struct TreePath : IEquatable public const int MemorySize = 36; public ValueHash256 Path; - public static TreePath Empty => new TreePath(); + public static TreePath Empty => new(); public readonly Span Span => Path.BytesAsSpan; @@ -38,7 +38,7 @@ public TreePath(in ValueHash256 path, int length) Length = length; } - public int Length { get; internal set; } + public int Length { get; private set; } public static TreePath FromPath(ReadOnlySpan pathHash) { @@ -46,7 +46,7 @@ public static TreePath FromPath(ReadOnlySpan pathHash) if (pathHash.Length == 32) return new TreePath(new ValueHash256(pathHash), 64); // Some of the test passes path directly to PatriciaTrie, but its not 32 byte. - TreePath newTreePath = new TreePath(); + TreePath newTreePath = new(); pathHash.CopyTo(newTreePath.Span); newTreePath.Length = pathHash.Length * 2; return newTreePath; diff --git a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs index f79971ea808..6ae6ca164fc 100644 --- a/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs +++ b/src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs @@ -154,14 +154,14 @@ private void CommitNode(long blockNumber, Hash256? address, ref TreePath path, i if (_logger.IsTrace) Trace(blockNumber, in nodeCommitInfo); if (!nodeCommitInfo.IsEmptyBlockMarker && !nodeCommitInfo.Node.IsBoundaryProofNode) { - TrieNode node = nodeCommitInfo.Node!; + TrieNode node = nodeCommitInfo.Node; - if (node!.Keccak is null) + if (node.Keccak is null) { ThrowUnknownHash(node); } - if (node!.LastSeen >= 0) + if (node.LastSeen >= 0) { ThrowNodeHasBeenSeen(blockNumber, node); } @@ -233,7 +233,7 @@ private TrieNode DirtyNodesFindCachedOrUnknown(TrieStoreDirtyNodesCache.Key key) private TrieNode SaveOrReplaceInDirtyNodesCache(Hash256? address, ref TreePath path, NodeCommitInfo nodeCommitInfo, TrieNode node) { - TrieStoreDirtyNodesCache.Key key = new TrieStoreDirtyNodesCache.Key(address, path, node.Keccak); + TrieStoreDirtyNodesCache.Key key = new(address, path, node.Keccak); if (DirtyNodesTryGetValue(in key, out TrieNode cachedNodeCopy)) { Metrics.LoadedFromCacheNodesCount++; @@ -276,15 +276,13 @@ static void ThrowNodeIsNotSame(TrieNode node, TrieNode cachedNodeCopy) => public ICommitter BeginCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags) { - int concurrency = 0; - if (_pruningStrategy.PruningEnabled) - { - // The write batch when pruning is not enabled is not concurrent safe - concurrency = Environment.ProcessorCount; - } ArgumentOutOfRangeException.ThrowIfNegative(blockNumber); EnsureCommitSetExistsForBlock(blockNumber); + int concurrency = _pruningStrategy.PruningEnabled + ? Environment.ProcessorCount + : 0; // The write batch when pruning is not enabled is not concurrent safe + return new TrieStoreCommitter(this, trieType, blockNumber, address, root, writeFlags, concurrency); } @@ -1127,8 +1125,9 @@ private class TrieStoreCommitter( int concurrency ) : ICommitter { - private bool _needToResetRoot = root is not null && root.IsDirty; + private readonly bool _needToResetRoot = root is not null && root.IsDirty; private int _concurrency = concurrency; + private TrieNode? _root = root; public void Dispose() { @@ -1136,33 +1135,27 @@ public void Dispose() { // During commit it PatriciaTrie, the root may get resolved to an existing node (same keccak). // This ensure that the root that we use here is the same. - root = trieStore.FindCachedOrUnknown(address, TreePath.Empty, root.Keccak); + _root = trieStore.FindCachedOrUnknown(address, TreePath.Empty, _root?.Keccak); } - trieStore.FinishBlockCommit(trieType, blockNumber, address, root, writeFlags); + + trieStore.FinishBlockCommit(trieType, blockNumber, address, _root, writeFlags); } - public void CommitNode(ref TreePath path, NodeCommitInfo nodeCommitInfo) - { + public void CommitNode(ref TreePath path, NodeCommitInfo nodeCommitInfo) => trieStore.CommitNode(blockNumber, address, ref path, nodeCommitInfo, writeFlags: writeFlags); - } public bool CanSpawnTask() { - if (Interlocked.Decrement(ref _concurrency) < 0) - { - ReturnConcurrencyQuota(); - return false; - } - else + if (Interlocked.Decrement(ref _concurrency) >= 0) { return true; } - } - public void ReturnConcurrencyQuota() - { - Interlocked.Increment(ref _concurrency); + ReturnConcurrencyQuota(); + return false; } + + public void ReturnConcurrencyQuota() => Interlocked.Increment(ref _concurrency); } diff --git a/src/Nethermind/Nethermind.Trie/TrieStoreWithReadFlags.cs b/src/Nethermind/Nethermind.Trie/TrieStoreWithReadFlags.cs index b97aff8991e..e93f1096d06 100644 --- a/src/Nethermind/Nethermind.Trie/TrieStoreWithReadFlags.cs +++ b/src/Nethermind/Nethermind.Trie/TrieStoreWithReadFlags.cs @@ -7,27 +7,15 @@ namespace Nethermind.Trie; -public class TrieStoreWithReadFlags : TrieNodeResolverWithReadFlags, IScopedTrieStore +public class TrieStoreWithReadFlags(IScopedTrieStore implementation, ReadFlags flags) + : TrieNodeResolverWithReadFlags(implementation, flags), IScopedTrieStore { - private readonly IScopedTrieStore _baseImplementation; + public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) => + implementation.BeginCommit(trieType, blockNumber, root, writeFlags); - public TrieStoreWithReadFlags(IScopedTrieStore implementation, ReadFlags flags) : base(implementation, flags) - { - _baseImplementation = implementation; - } + public bool IsPersisted(in TreePath path, in ValueHash256 keccak) => + implementation.IsPersisted(in path, in keccak); - public ICommitter BeginCommit(TrieType trieType, long blockNumber, TrieNode? root, WriteFlags writeFlags = WriteFlags.None) - { - return _baseImplementation.BeginCommit(trieType, blockNumber, root, writeFlags); - } - - public bool IsPersisted(in TreePath path, in ValueHash256 keccak) - { - return _baseImplementation.IsPersisted(in path, in keccak); - } - - public void Set(in TreePath path, in ValueHash256 keccak, byte[] rlp) - { - _baseImplementation.Set(in path, in keccak, rlp); - } + public void Set(in TreePath path, in ValueHash256 keccak, byte[] rlp) => + implementation.Set(in path, in keccak, rlp); } From b8fd198a254f44cb62f66f72874da94e44cf6d39 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Thu, 10 Oct 2024 10:43:37 +0200 Subject: [PATCH 14/15] Add .net 9 todo's --- src/Nethermind/Nethermind.Core/PubSub/CompositePublisher.cs | 1 + src/Nethermind/Nethermind.Network/CompositeNodeSource.cs | 3 ++- src/Nethermind/Nethermind.Trie/BatchedTrieVisitor.cs | 3 ++- src/Nethermind/Nethermind.Trie/PatriciaTree.cs | 1 + 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Nethermind/Nethermind.Core/PubSub/CompositePublisher.cs b/src/Nethermind/Nethermind.Core/PubSub/CompositePublisher.cs index 31261d6a34d..cfb3541fa4b 100644 --- a/src/Nethermind/Nethermind.Core/PubSub/CompositePublisher.cs +++ b/src/Nethermind/Nethermind.Core/PubSub/CompositePublisher.cs @@ -16,6 +16,7 @@ public CompositePublisher(params IPublisher[] publishers) public async Task PublishAsync(T data) where T : class { + // TODO: .Net 9 stackalloc Task[] tasks = new Task[_publishers.Length]; for (int i = 0; i < _publishers.Length; i++) { diff --git a/src/Nethermind/Nethermind.Network/CompositeNodeSource.cs b/src/Nethermind/Nethermind.Network/CompositeNodeSource.cs index a56329f4731..1402ebcf725 100644 --- a/src/Nethermind/Nethermind.Network/CompositeNodeSource.cs +++ b/src/Nethermind/Nethermind.Network/CompositeNodeSource.cs @@ -20,7 +20,8 @@ public async IAsyncEnumerable DiscoverNodes([EnumeratorCancellation] Cance { Channel ch = Channel.CreateBounded(1); - Task[] feedTasks = _nodeSources.Select(async (innerSource) => + // TODO: .Net 9 stackalloc + Task[] feedTasks = _nodeSources.Select(async innerSource => { await foreach (Node node in innerSource.DiscoverNodes(cancellationToken)) { diff --git a/src/Nethermind/Nethermind.Trie/BatchedTrieVisitor.cs b/src/Nethermind/Nethermind.Trie/BatchedTrieVisitor.cs index ef0706a09cf..9b0dc53abb7 100644 --- a/src/Nethermind/Nethermind.Trie/BatchedTrieVisitor.cs +++ b/src/Nethermind/Nethermind.Trie/BatchedTrieVisitor.cs @@ -142,8 +142,9 @@ public void Start( try { + // TODO: .Net 9 stackalloc Task[]? tasks = Enumerable.Range(0, trieVisitContext.MaxDegreeOfParallelism) - .Select((_) => Task.Run(BatchedThread)) + .Select(_ => Task.Run(BatchedThread)) .ToArray(); Task.WhenAll(tasks).Wait(); diff --git a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs index 4438fbda157..88b443a3983 100644 --- a/src/Nethermind/Nethermind.Trie/PatriciaTree.cs +++ b/src/Nethermind/Nethermind.Trie/PatriciaTree.cs @@ -207,6 +207,7 @@ Task CreateTaskForPath(TreePath childPath, TrieNode childNode, int idx) => Task. committer.ReturnConcurrencyQuota(); }); + // TODO: .Net 9 stackalloc ArrayPoolList? childTasks = null; for (int i = 0; i < 16; i++) From 0e81778addc304498a5ec7a7669aa3ea023b80a0 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Thu, 10 Oct 2024 10:45:48 +0200 Subject: [PATCH 15/15] WhenAll().Wait() ->WaitAll() --- src/Nethermind/Nethermind.Trie/BatchedTrieVisitor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Trie/BatchedTrieVisitor.cs b/src/Nethermind/Nethermind.Trie/BatchedTrieVisitor.cs index 9b0dc53abb7..99bff648354 100644 --- a/src/Nethermind/Nethermind.Trie/BatchedTrieVisitor.cs +++ b/src/Nethermind/Nethermind.Trie/BatchedTrieVisitor.cs @@ -147,7 +147,7 @@ public void Start( .Select(_ => Task.Run(BatchedThread)) .ToArray(); - Task.WhenAll(tasks).Wait(); + Task.WaitAll(tasks); } catch (Exception) {