Skip to content

Commit

Permalink
Improve TrieStore Dictionary use and Hashing (#6979)
Browse files Browse the repository at this point in the history
  • Loading branch information
@benaadams
benaadams authored May 4, 2024
1 parent 981fbb1 commit 50df67d
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 43 deletions.
10 changes: 7 additions & 3 deletions src/Nethermind/Nethermind.Core/Crypto/Hash256.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,12 @@ public ValueHash256(ReadOnlySpan<byte> bytes)

public override int GetHashCode()
{
uint hash = s_instanceRandom;
hash = BitOperations.Crc32C(hash, Unsafe.As<Vector256<byte>, ulong>(ref Unsafe.AsRef(in _bytes)));
return GetChainedHashCode(s_instanceRandom);
}

public int GetChainedHashCode(uint previousHash)
{
uint hash = BitOperations.Crc32C(previousHash, Unsafe.As<Vector256<byte>, ulong>(ref Unsafe.AsRef(in _bytes)));
hash = BitOperations.Crc32C(hash, Unsafe.Add(ref Unsafe.As<Vector256<byte>, ulong>(ref Unsafe.AsRef(in _bytes)), 1));
hash = BitOperations.Crc32C(hash, Unsafe.Add(ref Unsafe.As<Vector256<byte>, ulong>(ref Unsafe.AsRef(in _bytes)), 2));
hash = BitOperations.Crc32C(hash, Unsafe.Add(ref Unsafe.As<Vector256<byte>, ulong>(ref Unsafe.AsRef(in _bytes)), 3));
Expand Down Expand Up @@ -135,7 +139,7 @@ public readonly struct Hash256AsKey(Hash256 key) : IEquatable<Hash256AsKey>

[JsonConverter(typeof(Hash256Converter))]
[DebuggerStepThrough]
public class Hash256 : IEquatable<Hash256>, IComparable<Hash256>
public sealed class Hash256 : IEquatable<Hash256>, IComparable<Hash256>
{
public const int Size = 32;

Expand Down
3 changes: 1 addition & 2 deletions src/Nethermind/Nethermind.Trie/Pruning/ITrieStore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Threading;
using System.Threading.Tasks;
using Nethermind.Core;
using Nethermind.Core.Crypto;

Expand All @@ -14,7 +13,7 @@ namespace Nethermind.Trie.Pruning
/// </summary>
public interface ITrieStore : IDisposable
{
void CommitNode(long blockNumber, Hash256? address, NodeCommitInfo nodeCommitInfo, WriteFlags writeFlags = WriteFlags.None);
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);

Expand Down
2 changes: 1 addition & 1 deletion src/Nethermind/Nethermind.Trie/Pruning/ReadOnlyTrieStore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public IReadOnlyTrieStore AsReadOnly(INodeStorage nodeStore)
return new ReadOnlyTrieStore(_trieStore, nodeStore);
}

public void CommitNode(long blockNumber, Hash256? address, NodeCommitInfo nodeCommitInfo, WriteFlags flags = WriteFlags.None) { }
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) { }

Expand Down
5 changes: 3 additions & 2 deletions src/Nethermind/Nethermind.Trie/Pruning/TreePath.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Nethermind.Core.Attributes;
using Nethermind.Core.Crypto;
using Nethermind.Core.Extensions;
using System.Numerics;

namespace Nethermind.Trie;

Expand Down Expand Up @@ -255,7 +256,7 @@ public readonly override string ToString()

public readonly bool Equals(in TreePath other)
{
return Path.Equals(other.Path) && Length == other.Length;
return Length == other.Length && Path.Equals(in other.Path);
}

public readonly override bool Equals(object? obj)
Expand All @@ -265,7 +266,7 @@ public readonly override bool Equals(object? obj)

public readonly override int GetHashCode()
{
return HashCode.Combine(Path, Length);
return (int)BitOperations.Crc32C((uint)Path.GetHashCode(), (uint)Length);
}

/// <summary>
Expand Down
148 changes: 113 additions & 35 deletions src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Numerics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;
using System.Threading.Tasks.Dataflow;
Expand Down Expand Up @@ -60,11 +61,17 @@ public TrieNode FindCachedOrUnknown(in Key key)
else
{
trieNode = new TrieNode(NodeType.Unknown, key.Keccak);
if (_trieStore._logger.IsTrace) _trieStore._logger.Trace($"Creating new node {trieNode}");
if (_trieStore._logger.IsTrace) Trace(trieNode);
SaveInCache(key, trieNode);
}

return trieNode;

[MethodImpl(MethodImplOptions.NoInlining)]
void Trace(TrieNode trieNode)
{
_trieStore._logger.Trace($"Creating new node {trieNode}");
}
}

public TrieNode FromCachedRlpOrUnknown(in Key key)
Expand All @@ -91,8 +98,14 @@ public TrieNode FromCachedRlpOrUnknown(in Key key)
trieNode = new TrieNode(NodeType.Unknown, key.Keccak);
}

if (_trieStore._logger.IsTrace) _trieStore._logger.Trace($"Creating new node {trieNode}");
if (_trieStore._logger.IsTrace) Trace(trieNode);
return trieNode;

[MethodImpl(MethodImplOptions.NoInlining)]
void Trace(TrieNode trieNode)
{
_trieStore._logger.Trace($"Creating new node {trieNode}");
}
}

private readonly ConcurrentDictionary<Key, TrieNode> _byKeyObjectCache = new();
Expand Down Expand Up @@ -199,7 +212,9 @@ public void Clear()
{
internal const long MemoryUsage = 8 + 36 + 8; // (address (probably shared), path, keccak pointer (shared with TrieNode))
public Hash256? Address { get; }
public TreePath Path { get; }
// 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)
Expand All @@ -212,7 +227,14 @@ public Key(Hash256? address, in TreePath path, Hash256 keccak)
[SkipLocalsInit]
public override int GetHashCode()
{
return (int)BitOperations.Crc32C((uint)Path.GetHashCode(), (uint)Keccak.GetHashCode()) + (Address?.GetHashCode() ?? 0);
Hash256? address = Address;
var addressHash = 0;
if (address is not null)
{
addressHash = address.ValueHash256.GetHashCode();
}

return Keccak.ValueHash256.GetChainedHashCode((uint)Path.GetHashCode()) ^ addressHash;
}

public bool Equals(Key other)
Expand Down Expand Up @@ -363,29 +385,29 @@ public int CachedNodesCount
}
}

public void CommitNode(long blockNumber, Hash256? address, NodeCommitInfo nodeCommitInfo, WriteFlags writeFlags = WriteFlags.None)
public void CommitNode(long blockNumber, Hash256? address, in NodeCommitInfo nodeCommitInfo, WriteFlags writeFlags = WriteFlags.None)
{
ArgumentOutOfRangeException.ThrowIfNegative(blockNumber);
EnsureCommitSetExistsForBlock(blockNumber);

if (_logger.IsTrace) _logger.Trace($"Committing {nodeCommitInfo} at {blockNumber}");
if (_logger.IsTrace) Trace(blockNumber, in nodeCommitInfo);
if (!nodeCommitInfo.IsEmptyBlockMarker && !nodeCommitInfo.Node.IsBoundaryProofNode)
{
TrieNode node = nodeCommitInfo.Node!;

if (node!.Keccak is null)
{
throw new TrieStoreException($"The hash of {node} should be known at the time of committing.");
ThrowUnknownHash(node);
}

if (CurrentPackage is null)
{
throw new TrieStoreException($"{nameof(CurrentPackage)} is NULL when committing {node} at {blockNumber}.");
ThrowUnknownPackage(blockNumber, node);
}

if (node!.LastSeen.HasValue)
{
throw new TrieStoreException($"{nameof(TrieNode.LastSeen)} set on {node} committed at {blockNumber}.");
ThrowNodeHasBeenSeen(blockNumber, node);
}

node = SaveOrReplaceInDirtyNodesCache(address, nodeCommitInfo, node);
Expand All @@ -398,6 +420,33 @@ public void CommitNode(long blockNumber, Hash256? address, NodeCommitInfo nodeCo

CommittedNodesCount++;
}

[MethodImpl(MethodImplOptions.NoInlining)]
void Trace(long blockNumber, in NodeCommitInfo nodeCommitInfo)
{
_logger.Trace($"Committing {nodeCommitInfo} at {blockNumber}");
}

[DoesNotReturn]
[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}.");
}
}

private TrieNode SaveOrReplaceInDirtyNodesCache(Hash256? address, NodeCommitInfo nodeCommitInfo, TrieNode node)
Expand All @@ -410,12 +459,12 @@ private TrieNode SaveOrReplaceInDirtyNodesCache(Hash256? address, NodeCommitInfo
Metrics.LoadedFromCacheNodesCount++;
if (!ReferenceEquals(cachedNodeCopy, node))
{
if (_logger.IsTrace) _logger.Trace($"Replacing {node} with its cached copy {cachedNodeCopy}.");
if (_logger.IsTrace) Trace(node, cachedNodeCopy);
TreePath path = nodeCommitInfo.Path;
cachedNodeCopy.ResolveKey(GetTrieStore(address), ref path, nodeCommitInfo.IsRoot);
if (node.Keccak != cachedNodeCopy.Keccak)
{
throw new InvalidOperationException($"The hash of replacement node {cachedNodeCopy} is not the same as the original {node}.");
ThrowNodeIsNotSame(node, cachedNodeCopy);
}

if (!nodeCommitInfo.IsRoot)
Expand All @@ -434,6 +483,20 @@ private TrieNode SaveOrReplaceInDirtyNodesCache(Hash256? address, NodeCommitInfo
}

return node;

[MethodImpl(MethodImplOptions.NoInlining)]
void Trace(TrieNode node, TrieNode cachedNodeCopy)
{
_logger.Trace($"Replacing {node} with its cached copy {cachedNodeCopy}.");
}

[DoesNotReturn]
[StackTraceHidden]
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 void FinishBlockCommit(TrieType trieType, long blockNumber, Hash256? address, TrieNode? root, WriteFlags writeFlags = WriteFlags.None)
Expand Down Expand Up @@ -512,10 +575,17 @@ public byte[] LoadRlp(Hash256? address, in TreePath path, Hash256 keccak, INodeS
byte[]? rlp = TryLoadRlp(address, path, keccak, nodeStorage, readFlags);
if (rlp is null)
{
throw new TrieNodeException($"Node {keccak} is missing from the DB", keccak);
ThrowMissingNode(keccak);
}

return rlp;

[DoesNotReturn]
[StackTraceHidden]
static void ThrowMissingNode(Hash256 keccak)
{
throw new TrieNodeException($"Node {keccak} is missing from the DB", keccak);
}
}

public virtual byte[]? LoadRlp(Hash256? address, in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) => LoadRlp(address, path, hash, null, flags);
Expand Down Expand Up @@ -710,7 +780,7 @@ bool CanRemove(Hash256? address, TinyTreePath path, in TreePath fullPath, ValueH
!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 (_persistedLastSeens.TryGetValue((address, path, keccak), out long commitBlock) &&
if (_persistedLastSeens.TryGetValue(new(address, in path, in keccak), out long commitBlock) &&
!IsNoLongerNeeded(commitBlock)) return false;

return true;
Expand All @@ -721,7 +791,7 @@ bool CanRemove(Hash256? address, TinyTreePath path, in TreePath fullPath, ValueH
void DoAct(KeyValuePair<HashAndTinyPath, Hash256> keyValuePair)
{
HashAndTinyPath key = keyValuePair.Key;
if (_pastPathHash.TryGet((key.addr, key.path), out ValueHash256 prevHash))
if (_pastPathHash.TryGet(new(key.addr, in key.path), out ValueHash256 prevHash))
{
TreePath fullPath = key.path.ToTreePath(); // Micro op to reduce double convert
if (CanRemove(key.addr, key.path, fullPath, prevHash, keyValuePair.Value))
Expand Down Expand Up @@ -843,21 +913,22 @@ private void PruneCache(bool skipRecalculateMemory = false)
private void TrackPrunedPersistedNodes(in DirtyNodesCache.Key key, TrieNode node)
{
if (key.Path.Length > TinyTreePath.MaxNibbleLength) return;
TinyTreePath treePath = new TinyTreePath(key.Path);
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 != null)
{
// Update _persistedLastSeen to later value.
if (!_persistedLastSeens.TryGetValue((key.Address, treePath, key.Keccak), out long currentLastSeen) || currentLastSeen < node.LastSeen.Value)
{
_persistedLastSeens[(key.Address, treePath, key.Keccak)] = node.LastSeen.Value;
}
_persistedLastSeens.AddOrUpdate(
new(key.Address, in treePath, key.Keccak),
(_, newValue) => newValue,
(_, newValue, currentLastSeen) => Math.Max(newValue, currentLastSeen),
node.LastSeen.Value);
}

// This persisted node is being removed from cache. Keep it in mind in case of an update to the same
// path.
_pastPathHash.Set((key.Address, treePath), key.Keccak);
_pastPathHash.Set(new(key.Address, in treePath), key.Keccak);
}

/// <summary>
Expand Down Expand Up @@ -947,16 +1018,17 @@ void PersistNode(TrieNode tn, Hash256? address2, TreePath path)
{
if (persistedHashes != null && path.Length <= TinyTreePath.MaxNibbleLength)
{
(Hash256 address2, TinyTreePath path) key = (address2, new TinyTreePath(path));
if (persistedHashes.ContainsKey(key))
HashAndTinyPath key = new(address2, new TinyTreePath(path));
ref Hash256? hash = ref CollectionsMarshal.GetValueRefOrAddDefault(persistedHashes, key, out bool exists);
if (exists)
{
// Null mark that there are multiple saved hash for this path. So we don't attempt to remove anything.
// Otherwise this would have to be a list, which is such a rare case that its not worth it to have a list.
persistedHashes[key] = null;
hash = null;
}
else
{
persistedHashes[key] = tn.Keccak;
hash = tn.Keccak;
}
}
this.PersistNode(address2, path, tn, commitSet.BlockNumber, writeFlags, writeBatch);
Expand Down Expand Up @@ -1247,36 +1319,42 @@ public bool HasRoot(Hash256 stateRoot)
return true;
}

private readonly struct HashAndTinyPath(Hash256? hash, TinyTreePath path) : IEquatable<HashAndTinyPath>
private readonly struct HashAndTinyPath(Hash256? hash, in TinyTreePath path) : IEquatable<HashAndTinyPath>
{
public readonly Hash256? addr = hash;
public readonly TinyTreePath path = path;

public static implicit operator HashAndTinyPath((Hash256? hash, TinyTreePath path) value) => new HashAndTinyPath(value.hash, value.path);

public bool Equals(HashAndTinyPath other) => addr == other.addr && path.Equals(other.path);
public override bool Equals(object? obj) => obj is HashAndTinyPath other && Equals(other);
public override int GetHashCode() => (int)BitOperations.Crc32C((uint)path.GetHashCode(), (uint)(addr?.GetHashCode() ?? 0));
public override int GetHashCode()
{
Hash256? address = addr;
var addressHash = 0;
if (address is not null)
{
addressHash = address.ValueHash256.GetHashCode();
}
return path.GetHashCode() ^ addressHash;
}
}

private readonly struct HashAndTinyPathAndHash(Hash256? hash, TinyTreePath path, ValueHash256 valueHash) : IEquatable<HashAndTinyPathAndHash>
private readonly struct HashAndTinyPathAndHash(Hash256? hash, in TinyTreePath path, in ValueHash256 valueHash) : IEquatable<HashAndTinyPathAndHash>
{
public readonly Hash256? hash = hash;
public readonly TinyTreePath path = path;
public readonly ValueHash256 valueHash = valueHash;

public static implicit operator HashAndTinyPathAndHash((Hash256? hash, TinyTreePath path, ValueHash256 valueHash) value) => new HashAndTinyPathAndHash(value.hash, value.path, value.valueHash);

public bool Equals(HashAndTinyPathAndHash other) => hash == other.hash && path.Equals(other.path) && valueHash.Equals(in other.valueHash);
public override bool Equals(object? obj) => obj is HashAndTinyPath other && Equals(other);
public override int GetHashCode()
{
uint hashCode = BitOperations.Crc32C((uint)valueHash.GetHashCode(), (uint)path.GetHashCode());
var hashHash = 0;
if (hash is not null)
{
hashCode = BitOperations.Crc32C(hashCode, (uint)hash.GetHashCode());
hashHash = hash.ValueHash256.GetHashCode();
}
return (int)hashCode;

return valueHash.GetChainedHashCode((uint)path.GetHashCode()) ^ hashHash;
}
}
}
Expand Down

0 comments on commit 50df67d

Please sign in to comment.