diff --git a/src/Nethermind/Nethermind.Core.Test/Caching/LruCacheLowObjectTests.cs b/src/Nethermind/Nethermind.Core.Test/Caching/LruCacheLowObjectTests.cs index a75a35abd40..de3864b84f8 100644 --- a/src/Nethermind/Nethermind.Core.Test/Caching/LruCacheLowObjectTests.cs +++ b/src/Nethermind/Nethermind.Core.Test/Caching/LruCacheLowObjectTests.cs @@ -41,7 +41,7 @@ public void At_capacity() Cache cache = Create(); for (int i = 0; i < Capacity; i++) { - cache.Set(_addresses[i], _accounts[i]); + cache.Set(_addresses[i], _accounts[i]).Should().BeTrue(); } Account? account = cache.Get(_addresses[Capacity - 1]); @@ -52,8 +52,8 @@ public void At_capacity() public void Can_reset() { Cache cache = Create(); - cache.Set(_addresses[0], _accounts[0]); - cache.Set(_addresses[0], _accounts[1]); + cache.Set(_addresses[0], _accounts[0]).Should().BeTrue(); + cache.Set(_addresses[0], _accounts[1]).Should().BeFalse(); cache.Get(_addresses[0]).Should().Be(_accounts[1]); } @@ -68,10 +68,10 @@ public void Can_ask_before_first_set() public void Can_clear() { Cache cache = Create(); - cache.Set(_addresses[0], _accounts[0]); + cache.Set(_addresses[0], _accounts[0]).Should().BeTrue(); cache.Clear(); cache.Get(_addresses[0]).Should().BeNull(); - cache.Set(_addresses[0], _accounts[1]); + cache.Set(_addresses[0], _accounts[1]).Should().BeTrue(); cache.Get(_addresses[0]).Should().Be(_accounts[1]); } @@ -81,13 +81,27 @@ public void Beyond_capacity() Cache cache = Create(); for (int i = 0; i < Capacity * 2; i++) { - cache.Set(_addresses[i], _accounts[i]); + cache.Set(_addresses[i], _accounts[i]).Should().BeTrue(); } Account? account = cache.Get(_addresses[Capacity]); account.Should().Be(_accounts[Capacity]); } + [Test] + public void Beyond_capacity_lru() + { + Cache cache = Create(); + for (int i = 0; i < Capacity * 2; i++) + { + for (int ii = 0; ii < Capacity / 2; ii++) + { + cache.Set(_addresses[i], _accounts[i]); + } + cache.Set(_addresses[i], _accounts[i]); + } + } + [Test] public void Can_set_and_then_set_null() { @@ -114,7 +128,7 @@ public void Clear_should_free_all_capacity() Cache cache = Create(); for (int i = 0; i < Capacity; i++) { - cache.Set(_addresses[i], _accounts[i]); + cache.Set(_addresses[i], _accounts[i]).Should().BeTrue(); } cache.Clear(); @@ -124,7 +138,7 @@ public void Clear_should_free_all_capacity() // fill again for (int i = 0; i < Capacity; i++) { - cache.Set(_addresses[i], _accounts[MapForRefill(i)]); + cache.Set(_addresses[i], _accounts[MapForRefill(i)]).Should().BeTrue(); } // validate @@ -145,8 +159,10 @@ public void Delete_keeps_internal_structure() for (int i = 0; i < iterations; i++) { - cache.Set(i, i); - cache.Delete(i - itemsToKeep); + cache.Set(i, i).Should().BeTrue(); + var remove = i - itemsToKeep; + if (remove >= 0) + cache.Delete(remove).Should().BeTrue(); } int count = 0; diff --git a/src/Nethermind/Nethermind.Core.Test/Caching/LruCacheTests.cs b/src/Nethermind/Nethermind.Core.Test/Caching/LruCacheTests.cs index 35399b9e56d..95e37aa7c91 100755 --- a/src/Nethermind/Nethermind.Core.Test/Caching/LruCacheTests.cs +++ b/src/Nethermind/Nethermind.Core.Test/Caching/LruCacheTests.cs @@ -39,7 +39,7 @@ public void At_capacity() ICache cache = Create(); for (int i = 0; i < Capacity; i++) { - cache.Set(_addresses[i], _accounts[i]); + cache.Set(_addresses[i], _accounts[i]).Should().BeTrue(); } Account? account = cache.Get(_addresses[Capacity - 1]); @@ -50,8 +50,8 @@ public void At_capacity() public void Can_reset() { ICache cache = Create(); - cache.Set(_addresses[0], _accounts[0]); - cache.Set(_addresses[0], _accounts[1]); + cache.Set(_addresses[0], _accounts[0]).Should().BeTrue(); + cache.Set(_addresses[0], _accounts[1]).Should().BeFalse(); cache.Get(_addresses[0]).Should().Be(_accounts[1]); } @@ -66,21 +66,35 @@ public void Can_ask_before_first_set() public void Can_clear() { ICache cache = Create(); - cache.Set(_addresses[0], _accounts[0]); + cache.Set(_addresses[0], _accounts[0]).Should().BeTrue(); cache.Clear(); cache.Get(_addresses[0]).Should().BeNull(); - cache.Set(_addresses[0], _accounts[1]); + cache.Set(_addresses[0], _accounts[1]).Should().BeTrue(); cache.Get(_addresses[0]).Should().Be(_accounts[1]); } [Test] - public void Beyond_capacity() + public void Beyond_capacity_lru() { ICache cache = Create(); for (int i = 0; i < Capacity * 2; i++) { + for (int ii = 0; ii < Capacity / 2; ii++) + { + cache.Set(_addresses[i], _accounts[i]); + } cache.Set(_addresses[i], _accounts[i]); } + } + + [Test] + public void Beyond_capacity() + { + ICache cache = Create(); + for (int i = 0; i < Capacity * 2; i++) + { + cache.Set(_addresses[i], _accounts[i]).Should().BeTrue(); + } Account? account = cache.Get(_addresses[Capacity]); account.Should().Be(_accounts[Capacity]); diff --git a/src/Nethermind/Nethermind.Core.Test/Caching/LruKeyCacheLowObjectTests.cs b/src/Nethermind/Nethermind.Core.Test/Caching/LruKeyCacheLowObjectTests.cs index 6b5217ec0e5..cb8b07dc324 100644 --- a/src/Nethermind/Nethermind.Core.Test/Caching/LruKeyCacheLowObjectTests.cs +++ b/src/Nethermind/Nethermind.Core.Test/Caching/LruKeyCacheLowObjectTests.cs @@ -78,6 +78,20 @@ public void Beyond_capacity() cache.Get(_addresses[Capacity]).Should().BeTrue(); } + [Test] + public void Beyond_capacity_lru() + { + LruKeyCacheLowObject cache = new(Capacity, "test"); + for (int i = 0; i < Capacity * 2; i++) + { + for (int ii = 0; ii < Capacity / 2; ii++) + { + cache.Set(_addresses[i]); + } + cache.Set(_addresses[i]); + } + } + [Test] public void Can_delete() { diff --git a/src/Nethermind/Nethermind.Core.Test/Caching/LruKeyCacheNonConcurrentTests.cs b/src/Nethermind/Nethermind.Core.Test/Caching/LruKeyCacheNonConcurrentTests.cs new file mode 100644 index 00000000000..614ba3cfa4e --- /dev/null +++ b/src/Nethermind/Nethermind.Core.Test/Caching/LruKeyCacheNonConcurrentTests.cs @@ -0,0 +1,104 @@ +// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using FluentAssertions; +using Nethermind.Core.Caching; +using Nethermind.Core.Test.Builders; +using Nethermind.Int256; +using NUnit.Framework; + +namespace Nethermind.Core.Test.Caching +{ + [TestFixture] + public class LruKeyCacheNonConcurrentTests + { + private const int Capacity = 16; + + private readonly Account[] _accounts = new Account[Capacity * 2]; + private readonly Address[] _addresses = new Address[Capacity * 2]; + + [SetUp] + public void Setup() + { + for (int i = 0; i < Capacity * 2; i++) + { + _accounts[i] = Build.An.Account.WithBalance((UInt256)i).TestObject; + _addresses[i] = Build.An.Address.FromNumber(i).TestObject; + } + } + + [Test] + public void At_capacity() + { + LruKeyCacheNonConcurrent
cache = new(Capacity, "test"); + for (int i = 0; i < Capacity; i++) + { + cache.Set(_addresses[i]).Should().BeTrue(); + } + + cache.Get(_addresses[Capacity - 1]).Should().BeTrue(); + } + + [Test] + public void Can_reset() + { + LruKeyCacheNonConcurrent
cache = new(Capacity, "test"); + cache.Set(_addresses[0]).Should().BeTrue(); + cache.Set(_addresses[0]).Should().BeFalse(); + cache.Get(_addresses[0]).Should().BeTrue(); + } + + [Test] + public void Can_ask_before_first_set() + { + LruKeyCacheNonConcurrent
cache = new(Capacity, "test"); + cache.Get(_addresses[0]).Should().BeFalse(); + } + + [Test] + public void Can_clear() + { + LruKeyCacheNonConcurrent
cache = new(Capacity, "test"); + cache.Set(_addresses[0]).Should().BeTrue(); + cache.Clear(); + cache.Get(_addresses[0]).Should().BeFalse(); + cache.Set(_addresses[0]).Should().BeTrue(); + cache.Get(_addresses[0]).Should().BeTrue(); + } + + [Test] + public void Beyond_capacity() + { + LruKeyCacheNonConcurrent
cache = new(Capacity, "test"); + for (int i = 0; i < Capacity * 2; i++) + { + cache.Set(_addresses[i]); + } + + cache.Get(_addresses[Capacity]).Should().BeTrue(); + } + + [Test] + public void Beyond_capacity_lru() + { + LruKeyCacheNonConcurrent cache = new(Capacity, "test"); + for (int i = 0; i < Capacity * 2; i++) + { + for (int ii = 0; ii < Capacity / 2; ii++) + { + cache.Set(_addresses[i]); + } + cache.Set(_addresses[i]); + } + } + + [Test] + public void Can_delete() + { + LruKeyCacheNonConcurrent
cache = new(Capacity, "test"); + cache.Set(_addresses[0]).Should().BeTrue(); + cache.Delete(_addresses[0]); + cache.Get(_addresses[0]).Should().BeFalse(); + } + } +} diff --git a/src/Nethermind/Nethermind.Core.Test/Caching/LruKeyCacheTests.cs b/src/Nethermind/Nethermind.Core.Test/Caching/LruKeyCacheTests.cs index 05185bfa433..5cf04410450 100644 --- a/src/Nethermind/Nethermind.Core.Test/Caching/LruKeyCacheTests.cs +++ b/src/Nethermind/Nethermind.Core.Test/Caching/LruKeyCacheTests.cs @@ -33,7 +33,7 @@ public void At_capacity() LruKeyCache
cache = new(Capacity, "test"); for (int i = 0; i < Capacity; i++) { - cache.Set(_addresses[i]); + cache.Set(_addresses[i]).Should().BeTrue(); } cache.Get(_addresses[Capacity - 1]).Should().BeTrue(); @@ -72,17 +72,31 @@ public void Beyond_capacity() LruKeyCache
cache = new(Capacity, "test"); for (int i = 0; i < Capacity * 2; i++) { - cache.Set(_addresses[i]); + cache.Set(_addresses[i]).Should().BeTrue(); } cache.Get(_addresses[Capacity]).Should().BeTrue(); } + [Test] + public void Beyond_capacity_lru() + { + LruKeyCache cache = new(Capacity, "test"); + for (int i = 0; i < Capacity * 2; i++) + { + for (int ii = 0; ii < Capacity / 2; ii++) + { + cache.Set(_addresses[i]); + } + cache.Set(_addresses[i]); + } + } + [Test] public void Can_delete() { LruKeyCache
cache = new(Capacity, "test"); - cache.Set(_addresses[0]); + cache.Set(_addresses[0]).Should().BeTrue(); cache.Delete(_addresses[0]); cache.Get(_addresses[0]).Should().BeFalse(); } diff --git a/src/Nethermind/Nethermind.Core.Test/Caching/SpanLruCacheTests.cs b/src/Nethermind/Nethermind.Core.Test/Caching/SpanLruCacheTests.cs index f30d930bf53..5cf255515f4 100755 --- a/src/Nethermind/Nethermind.Core.Test/Caching/SpanLruCacheTests.cs +++ b/src/Nethermind/Nethermind.Core.Test/Caching/SpanLruCacheTests.cs @@ -40,7 +40,7 @@ public void At_capacity() ISpanCache cache = Create(); for (int i = 0; i < Capacity; i++) { - cache.Set(_addresses[i].Bytes, _accounts[i]); + cache.Set(_addresses[i].Bytes, _accounts[i]).Should().BeTrue(); } Account? account = cache.Get(_addresses[Capacity - 1].Bytes); @@ -51,8 +51,8 @@ public void At_capacity() public void Can_reset() { ISpanCache cache = Create(); - cache.Set(_addresses[0].Bytes, _accounts[0]); - cache.Set(_addresses[0].Bytes, _accounts[1]); + cache.Set(_addresses[0].Bytes, _accounts[0]).Should().BeTrue(); + cache.Set(_addresses[0].Bytes, _accounts[1]).Should().BeFalse(); cache.Get(_addresses[0].Bytes).Should().Be(_accounts[1]); } @@ -67,10 +67,10 @@ public void Can_ask_before_first_set() public void Can_clear() { ISpanCache cache = Create(); - cache.Set(_addresses[0].Bytes, _accounts[0]); + cache.Set(_addresses[0].Bytes, _accounts[0]).Should().BeTrue(); cache.Clear(); cache.Get(_addresses[0].Bytes).Should().BeNull(); - cache.Set(_addresses[0].Bytes, _accounts[1]); + cache.Set(_addresses[0].Bytes, _accounts[1]).Should().BeTrue(); cache.Get(_addresses[0].Bytes).Should().Be(_accounts[1]); } @@ -80,13 +80,27 @@ public void Beyond_capacity() ISpanCache cache = Create(); for (int i = 0; i < Capacity * 2; i++) { - cache.Set(_addresses[i].Bytes, _accounts[i]); + cache.Set(_addresses[i].Bytes, _accounts[i]).Should().BeTrue(); } Account? account = cache.Get(_addresses[Capacity].Bytes); account.Should().Be(_accounts[Capacity]); } + [Test] + public void Beyond_capacity_lru() + { + ISpanCache cache = Create(); + for (int i = 0; i < Capacity * 2; i++) + { + for (int ii = 0; ii < Capacity / 2; ii++) + { + cache.Set(_addresses[i].Bytes, _accounts[i]); + } + cache.Set(_addresses[i].Bytes, _accounts[i]); + } + } + [Test] public void Can_set_and_then_set_null() { @@ -101,7 +115,7 @@ public void Can_set_and_then_set_null() public void Can_delete() { ISpanCache cache = Create(); - cache.Set(_addresses[0].Bytes, _accounts[0]); + cache.Set(_addresses[0].Bytes, _accounts[0]).Should().BeTrue(); cache.Delete(_addresses[0].Bytes).Should().BeTrue(); cache.Get(_addresses[0].Bytes).Should().Be(null); cache.Delete(_addresses[0].Bytes).Should().BeFalse(); diff --git a/src/Nethermind/Nethermind.Core/Caching/LinkedListNode.cs b/src/Nethermind/Nethermind.Core/Caching/LinkedListNode.cs index 93011a77bb6..7a882123e39 100644 --- a/src/Nethermind/Nethermind.Core/Caching/LinkedListNode.cs +++ b/src/Nethermind/Nethermind.Core/Caching/LinkedListNode.cs @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only +using System; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; @@ -21,7 +22,10 @@ public static void MoveToMostRecent([NotNull] ref LinkedListNode? leastRecent { if (node.Next == node) { - Debug.Assert(leastRecentlyUsed == node, "this should only be true for a list with only one node"); + if (leastRecentlyUsed != node) + { + InvalidNotSingleNodeList(); + } // Do nothing only one node } else @@ -33,10 +37,16 @@ public static void MoveToMostRecent([NotNull] ref LinkedListNode? leastRecent public static void Remove(ref LinkedListNode? leastRecentlyUsed, LinkedListNode node) { - Debug.Assert(leastRecentlyUsed is not null, "This method shouldn't be called on empty list!"); + if (leastRecentlyUsed is null) + { + InvalidRemoveFromEmptyList(); + } if (node.Next == node) { - Debug.Assert(leastRecentlyUsed == node, "this should only be true for a list with only one node"); + if (leastRecentlyUsed != node) + { + InvalidNotSingleNodeList(); + } leastRecentlyUsed = null; } else @@ -48,6 +58,20 @@ public static void Remove(ref LinkedListNode? leastRecentlyUsed, LinkedListNo leastRecentlyUsed = node.Next; } } + + [DoesNotReturn] + [StackTraceHidden] + static void InvalidRemoveFromEmptyList() + { + throw new InvalidOperationException("This method shouldn't be called on empty list"); + } + } + + [DoesNotReturn] + [StackTraceHidden] + static void InvalidNotSingleNodeList() + { + throw new InvalidOperationException("This should only be true for a list with only one node"); } public static void AddMostRecent([NotNull] ref LinkedListNode? leastRecentlyUsed, LinkedListNode node) diff --git a/src/Nethermind/Nethermind.Core/Caching/LruCacheLowObject.cs b/src/Nethermind/Nethermind.Core/Caching/LruCacheLowObject.cs index 6e27ae087ca..bbdceab425f 100644 --- a/src/Nethermind/Nethermind.Core/Caching/LruCacheLowObject.cs +++ b/src/Nethermind/Nethermind.Core/Caching/LruCacheLowObject.cs @@ -163,9 +163,13 @@ private void Replace(TKey key, TValue value) ref var node = ref _items[offset]; _cacheMap.Remove(node.Key); + node = new(key, value) + { + Next = node.Next, + Prev = node.Prev + }; MoveToMostRecent(ref node, offset); - node = new(key, value); _cacheMap.Add(key, offset); [DoesNotReturn] @@ -181,7 +185,10 @@ private void MoveToMostRecent(ref LruCacheItem node, int offset) { if (node.Next == offset) { - Debug.Assert(_leastRecentlyUsed == offset, "this should only be true for a list with only one node"); + if (_leastRecentlyUsed != offset) + { + InvalidNotSingleNodeList(); + } // Do nothing only one node } else @@ -193,10 +200,16 @@ private void MoveToMostRecent(ref LruCacheItem node, int offset) private void Remove(ref LruCacheItem node, int offset) { - Debug.Assert(_leastRecentlyUsed >= 0, "This method shouldn't be called on empty list!"); + if (_leastRecentlyUsed < 0) + { + InvalidRemoveFromEmptyList(); + } if (node.Next == offset) { - Debug.Assert(_leastRecentlyUsed == offset, "this should only be true for a list with only one node"); + if (_leastRecentlyUsed != offset) + { + InvalidNotSingleNodeList(); + } _leastRecentlyUsed = -1; } else @@ -208,6 +221,18 @@ private void Remove(ref LruCacheItem node, int offset) _leastRecentlyUsed = node.Next; } } + + static void InvalidRemoveFromEmptyList() + { + throw new InvalidOperationException("This method shouldn't be called on empty list"); + } + } + + [DoesNotReturn] + [StackTraceHidden] + static void InvalidNotSingleNodeList() + { + throw new InvalidOperationException("This should only be true for a list with only one node"); } private void AddMostRecent(ref LruCacheItem node, int offset) diff --git a/src/Nethermind/Nethermind.Core/Caching/LruKeyCacheLowObject.cs b/src/Nethermind/Nethermind.Core/Caching/LruKeyCacheLowObject.cs index 92318716948..dd8e3af89eb 100644 --- a/src/Nethermind/Nethermind.Core/Caching/LruKeyCacheLowObject.cs +++ b/src/Nethermind/Nethermind.Core/Caching/LruKeyCacheLowObject.cs @@ -140,9 +140,13 @@ private void Replace(TKey key) ref var node = ref _items[offset]; _cacheMap.Remove(node.Key); + node = new(key) + { + Next = node.Next, + Prev = node.Prev + }; MoveToMostRecent(ref node, offset); - node = new(key); _cacheMap.Add(key, offset); [DoesNotReturn] @@ -156,7 +160,10 @@ private void MoveToMostRecent(ref LruCacheItem node, int offset) { if (node.Next == offset) { - Debug.Assert(_leastRecentlyUsed == offset, "this should only be true for a list with only one node"); + if (_leastRecentlyUsed != offset) + { + InvalidNotSingleNodeList(); + } // Do nothing only one node } else @@ -168,10 +175,16 @@ private void MoveToMostRecent(ref LruCacheItem node, int offset) private void Remove(ref LruCacheItem node, int offset) { - Debug.Assert(_leastRecentlyUsed >= 0, "This method shouldn't be called on empty list!"); + if (_leastRecentlyUsed < 0) + { + InvalidRemoveFromEmptyList(); + } if (node.Next == offset) { - Debug.Assert(_leastRecentlyUsed == offset, "this should only be true for a list with only one node"); + if (_leastRecentlyUsed != offset) + { + InvalidNotSingleNodeList(); + } _leastRecentlyUsed = -1; } else @@ -183,6 +196,18 @@ private void Remove(ref LruCacheItem node, int offset) _leastRecentlyUsed = node.Next; } } + + static void InvalidRemoveFromEmptyList() + { + throw new InvalidOperationException("This method shouldn't be called on empty list"); + } + } + + [DoesNotReturn] + [StackTraceHidden] + static void InvalidNotSingleNodeList() + { + throw new InvalidOperationException("This should only be true for a list with only one node"); } private void AddMostRecent(ref LruCacheItem node, int offset) diff --git a/src/Nethermind/Nethermind.Synchronization/FastSync/TreeSync.cs b/src/Nethermind/Nethermind.Synchronization/FastSync/TreeSync.cs index 4cc449cb47b..449420b2cba 100644 --- a/src/Nethermind/Nethermind.Synchronization/FastSync/TreeSync.cs +++ b/src/Nethermind/Nethermind.Synchronization/FastSync/TreeSync.cs @@ -66,8 +66,8 @@ public class TreeSync private readonly ReaderWriterLockSlim _syncStateLock = new(); private readonly ConcurrentDictionary _pendingRequests = new(); private Dictionary> _dependencies = new(); - private readonly LruKeyCache _alreadySavedNode = new(AlreadySavedCapacity, "saved nodes"); - private readonly LruKeyCache _alreadySavedCode = new(AlreadySavedCapacity, "saved nodes"); + private readonly LruKeyCacheLowObject _alreadySavedNode = new(AlreadySavedCapacity, "saved nodes"); + private readonly LruKeyCacheLowObject _alreadySavedCode = new(AlreadySavedCapacity, "saved nodes"); private BranchProgress _branchProgress; private int _hintsToResetRoot;