Remove ChangeSet from StoreView (#3739)

* Speed up Read only storage

* Add ut

* Fix ut

* Format

* Fix CloneCache

* Rename

* change error message

* Fix clone cache tests

* Fix merge

* Update UT_MemoryStore.cs

* Fix build

* Update src/Neo/Persistence/StoreCache.cs

---------

Co-authored-by: Jimmy <[email protected]>
Co-authored-by: Christopher Schuchardt <[email protected]>
Co-authored-by: NGD Admin <[email protected]>

Author: 4 people

src/Neo/NeoSystem.cs

@@ -84,7 +84,7 @@ public class NeoSystem : IDisposable
         /// <remarks>
         /// It doesn't need to be disposed because the <see cref="IStoreSnapshot"/> inside it is null.
         /// </remarks>
-        public DataCache StoreView => new StoreCache(store);
+        public StoreCache StoreView => new(store);
 
         /// <summary>
         /// The memory pool of the <see cref="NeoSystem"/>.

src/Neo/Persistence/ClonedCache.cs

@@ -20,7 +20,7 @@ class ClonedCache : DataCache
     {
         private readonly DataCache _innerCache;
 
-        public ClonedCache(DataCache innerCache)
+        public ClonedCache(DataCache innerCache) : base(false)
         {
             _innerCache = innerCache;
         }

src/Neo/Persistence/DataCache.cs

@@ -43,7 +43,12 @@ public class Trackable(StorageItem item, TrackState state)
         }
 
         private readonly Dictionary<StorageKey, Trackable> _dictionary = [];
-        private readonly HashSet<StorageKey> _changeSet = [];
+        private readonly HashSet<StorageKey>? _changeSet;
+
+        /// <summary>
+        /// True if DataCache is readOnly
+        /// </summary>
+        public bool IsReadOnly => _changeSet == null;
 
         /// <summary>
         /// Reads a specified entry from the cache. If the entry is not in the cache, it will be automatically loaded from the underlying storage.
@@ -72,6 +77,16 @@ public StorageItem this[StorageKey key]
             }
         }
 
+        /// <summary>
+        /// Data cache constructor
+        /// </summary>
+        /// <param name="readOnly">True if you don't want to allow writes</param>
+        protected DataCache(bool readOnly)
+        {
+            if (!readOnly)
+                _changeSet = [];
+        }
+
         /// <summary>
         /// Adds a new entry to the cache.
         /// </summary>
@@ -97,7 +112,7 @@ public void Add(StorageKey key, StorageItem value)
                 {
                     _dictionary[key] = new Trackable(value, TrackState.Added);
                 }
-                _changeSet.Add(key);
+                _changeSet?.Add(key);
             }
         }
 
@@ -113,6 +128,11 @@ public void Add(StorageKey key, StorageItem value)
         /// </summary>
         public virtual void Commit()
         {
+            if (_changeSet is null)
+            {
+                throw new InvalidOperationException("DataCache is read only");
+            }
+
             lock (_dictionary)
             {
                 foreach (var key in _changeSet)
@@ -138,6 +158,24 @@ public virtual void Commit()
             }
         }
 
+        /// <summary>
+        /// Gets the change set in the cache.
+        /// </summary>
+        /// <returns>The change set.</returns>
+        public IEnumerable<KeyValuePair<StorageKey, Trackable>> GetChangeSet()
+        {
+            if (_changeSet is null)
+            {
+                throw new InvalidOperationException("DataCache is read only");
+            }
+
+            lock (_dictionary)
+            {
+                foreach (var key in _changeSet)
+                    yield return new(key, _dictionary[key]);
+            }
+        }
+
         /// <summary>
         /// Creates a snapshot, which uses this instance as the underlying storage.
         /// </summary>
@@ -170,20 +208,20 @@ public void Delete(StorageKey key)
                     if (trackable.State == TrackState.Added)
                     {
                         trackable.State = TrackState.NotFound;
-                        _changeSet.Remove(key);
+                        _changeSet?.Remove(key);
                     }
                     else if (trackable.State != TrackState.NotFound)
                     {
                         trackable.State = TrackState.Deleted;
-                        _changeSet.Add(key);
+                        _changeSet?.Add(key);
                     }
                 }
                 else
                 {
                     var item = TryGetInternal(key);
                     if (item == null) return;
                     _dictionary.Add(key, new Trackable(item, TrackState.Deleted));
-                    _changeSet.Add(key);
+                    _changeSet?.Add(key);
                 }
             }
         }
@@ -262,19 +300,6 @@ public void Delete(StorageKey key)
                     yield break;
         }
 
-        /// <summary>
-        /// Gets the change set in the cache.
-        /// </summary>
-        /// <returns>The change set.</returns>
-        public IEnumerable<KeyValuePair<StorageKey, Trackable>> GetChangeSet()
-        {
-            lock (_dictionary)
-            {
-                foreach (var key in _changeSet)
-                    yield return new(key, _dictionary[key]);
-            }
-        }
-
         /// <summary>
         /// Determines whether the cache contains the specified entry.
         /// </summary>
@@ -337,13 +362,13 @@ public bool Contains(StorageKey key)
                         else
                         {
                             trackable.State = TrackState.Added;
-                            _changeSet.Add(key);
+                            _changeSet?.Add(key);
                         }
                     }
                     else if (trackable.State == TrackState.None)
                     {
                         trackable.State = TrackState.Changed;
-                        _changeSet.Add(key);
+                        _changeSet?.Add(key);
                     }
                 }
                 else
@@ -359,7 +384,7 @@ public bool Contains(StorageKey key)
                         trackable = new Trackable(item, TrackState.Changed);
                     }
                     _dictionary.Add(key, trackable);
-                    _changeSet.Add(key);
+                    _changeSet?.Add(key);
                 }
                 return trackable.Item;
             }
@@ -392,7 +417,7 @@ public StorageItem GetOrAdd(StorageKey key, Func<StorageItem> factory)
                         else
                         {
                             trackable.State = TrackState.Added;
-                            _changeSet.Add(key);
+                            _changeSet?.Add(key);
                         }
                     }
                 }
@@ -402,7 +427,7 @@ public StorageItem GetOrAdd(StorageKey key, Func<StorageItem> factory)
                     if (item == null)
                     {
                         trackable = new Trackable(factory(), TrackState.Added);
-                        _changeSet.Add(key);
+                        _changeSet?.Add(key);
                     }
                     else
                     {

src/Neo/Persistence/StoreCache.cs

@@ -31,7 +31,8 @@ public class StoreCache : DataCache, IDisposable
         /// Initializes a new instance of the <see cref="StoreCache"/> class.
         /// </summary>
         /// <param name="store">An <see cref="IStore"/> to create a readonly cache.</param>
-        public StoreCache(IStore store)
+        /// <param name="readOnly">True if you don't want to track write changes</param>
+        public StoreCache(IStore store, bool readOnly = true) : base(readOnly)
         {
             _store = store;
         }
@@ -40,7 +41,7 @@ public StoreCache(IStore store)
         /// Initializes a new instance of the <see cref="StoreCache"/> class.
         /// </summary>
         /// <param name="snapshot">An <see cref="IStoreSnapshot"/> to create a snapshot cache.</param>
-        public StoreCache(IStoreSnapshot snapshot)
+        public StoreCache(IStoreSnapshot snapshot) : base(false)
         {
             _store = snapshot;
             _snapshot = snapshot;

tests/Neo.Plugins.RpcServer.Tests/UT_RpcServer.Wallet.cs

@@ -342,7 +342,7 @@ public void TestListAddress_WhenWalletNotOpen()
         public void TestCancelTransaction()
         {
             TestUtilOpenWallet();
-            var snapshot = _neoSystem.GetSnapshot();
+            var snapshot = _neoSystem.GetSnapshotCache();
             var tx = TestUtils.CreateValidTx(snapshot, _wallet, _walletAccount);
             snapshot.Commit();
             var paramsArray = new JArray(tx.Hash.ToString(), new JArray(_walletAccount.Address));