Added Cell caching of length to improve efficiency

srdtrk · srdtrk · commit e0acd6fd326d · 2022-08-15T11:53:59.000+03:00
diff --git a/packages/storage/Readme.md b/packages/storage/Readme.md
@@ -98,7 +98,7 @@ The same conventions from `Item` also apply here, that is:
 To import and intialize this storage object as a constant in `state.rs`, do the following:
 
 ```rust
-use secret_toolkit_storage::{AppendStore}
+use secret_toolkit::storage::{AppendStore}
 ```
 
 ```rust
@@ -121,7 +121,7 @@ The main user facing methods to read/write to AppendStore are `pop`, `push`, `ge
 AppendStore also implements a readonly iterator feature. This feature is also used to create a paging wrapper method called `paging`. The way you create the iterator is:
 
 ```rust
-let iter = user_count_store.iter(&deps.storage)?
+let iter = user_count_store.iter(&deps.storage)?;
 ```
 
 More examples can be found in the unit tests. And the paging wrapper is used in the following manner:
@@ -133,6 +133,19 @@ let page_size: u32 = 5;
 let values = user_count_store.paging(&deps.storage, start_page, page_size)?;
 ```
 
+> ❗ When using any iterators in any of the storage objects, the following will result in a compiling error.
+
+```rust
+let iterator = COUNT_STORE.iter(&deps.storage)?;
+```
+
+However, the follwoing will not result in an error:
+
+```rust
+let append_store = COUNT_STORE
+let iterator = append_store.iter(&deps.storage)?;
+```
+
 ### **DequeStore**
 
 This is a storage wrapper based on AppendStore that replicates a double ended list. This storage object allows the user to efficiently pop/push items to either end of the list.
diff --git a/packages/storage/src/append_store.rs b/packages/storage/src/append_store.rs
@@ -4,6 +4,7 @@
 //! This is achieved by storing each item in a separate storage entry. A special key is reserved
 //! for storing the length of the collection so far.
 use std::any::type_name;
+use std::cell::Cell;
 use std::{convert::TryInto};
 use std::marker::PhantomData;
 
@@ -25,6 +26,7 @@ pub struct AppendStore<'a, T, Ser = Bincode2>
     /// needed if any suffixes were added to the original namespace.
     /// therefore it is not necessarily same as the namespace.
     prefix: Option<Vec<u8>>,
+    length: Cell<Option<u32>>,
     item_type: PhantomData<T>,
     serialization_type: PhantomData<Ser>,
 }
@@ -35,6 +37,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser>{
         Self {
             namespace: prefix,
             prefix: None,
+            length: Cell::new(None),
             item_type: PhantomData,
             serialization_type: PhantomData,
         }
@@ -50,6 +53,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser>{
         Self {
             namespace: self.namespace,
             prefix: Some(prefix),
+            length: Cell::new(None),
             item_type: self.item_type,
             serialization_type: self.serialization_type,
         }
@@ -59,13 +63,20 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser>{
 impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
     /// gets the length from storage, and otherwise sets it to 0
     pub fn get_len<S: ReadonlyStorage>(&self, storage: &S) -> StdResult<u32> {
-        let len_key = [self.as_slice(), LEN_KEY].concat();
-        if let Some(len_vec) = storage.get(&len_key) {
-            let len_bytes = len_vec.as_slice().try_into().map_err(|err| StdError::parse_err("u32", err))?;
-            let len = u32::from_be_bytes(len_bytes);
-            Ok(len)
-        } else {
-            Ok(0)
+        match self.length.get() {
+            Some(len) => { Ok(len) },
+            None => {
+                let len_key = [self.as_slice(), LEN_KEY].concat();
+                if let Some(len_vec) = storage.get(&len_key) {
+                    let len_bytes = len_vec.as_slice().try_into().map_err(|err| StdError::parse_err("u32", err))?;
+                    let len = u32::from_be_bytes(len_bytes);
+                    self.length.set(Some(len));
+                    Ok(len)
+                } else {
+                    self.length.set(Some(0));
+                    Ok(0)
+                }
+            },
         }
     }
     /// checks if the collection has any elements
@@ -87,6 +98,7 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
     /// Set the length of the collection
     fn set_len<S: Storage>(&self, storage: &mut S, len: u32) {
         let len_key = [self.as_slice(), LEN_KEY].concat();
+        self.length.set(Some(len));
         storage.set(&len_key, &len.to_be_bytes());
     }
     /// Clear the collection
@@ -194,25 +206,14 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> AppendStore<'a, T, Ser> {
     }
 }
 
-impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> Clone for AppendStore<'a, T, Ser> {
-    fn clone(&self) -> Self {
-        Self {
-            namespace: self.namespace.clone(),
-            prefix: self.prefix.clone(),
-            item_type: self.item_type.clone(),
-            serialization_type: self.serialization_type.clone()
-        }
-    }
-}
-
 /// An iterator over the contents of the append store.
 pub struct AppendStoreIter<'a, T, S, Ser>
 where
     T: Serialize + DeserializeOwned,
     S: ReadonlyStorage,
     Ser: Serde,
 {
-    append_store: AppendStore<'a, T, Ser>,
+    append_store: &'a AppendStore<'a, T, Ser>,
     storage: &'a S,
     start: u32,
     end: u32,
@@ -232,7 +233,7 @@ impl<'a, T, S, Ser> AppendStoreIter<'a, T, S, Ser>
         end: u32
     ) -> Self {
         Self {
-            append_store: append_store.clone(),
+            append_store,
             storage,
             start,
             end,
@@ -336,6 +337,39 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn test_length() -> StdResult<()> {
+        let mut storage = MockStorage::new();
+        let append_store: AppendStore<i32> = AppendStore::new(b"test");
+
+        assert_eq!(append_store.length, Cell::new(None));
+        assert_eq!(append_store.get_len(&mut storage)?, 0);
+        assert_eq!(append_store.length, Cell::new(Some(0)));
+
+        append_store.push(&mut storage, &1234)?;
+        append_store.push(&mut storage, &2143)?;
+        append_store.push(&mut storage, &3412)?;
+        append_store.push(&mut storage, &4321)?;
+        assert_eq!(append_store.length, Cell::new(Some(4)));
+        assert_eq!(append_store.get_len(&mut storage)?, 4);
+
+        assert_eq!(append_store.pop(&mut storage), Ok(4321));
+        assert_eq!(append_store.pop(&mut storage), Ok(3412));
+        assert_eq!(append_store.length, Cell::new(Some(2)));
+        assert_eq!(append_store.get_len(&mut storage)?, 2);
+
+        assert_eq!(append_store.pop(&mut storage), Ok(2143));
+        assert_eq!(append_store.pop(&mut storage), Ok(1234));
+        assert_eq!(append_store.length, Cell::new(Some(0)));
+        assert_eq!(append_store.get_len(&mut storage)?, 0);
+
+        assert!(append_store.pop(&mut storage).is_err());
+        assert_eq!(append_store.length, Cell::new(Some(0)));
+        assert_eq!(append_store.get_len(&mut storage)?, 0);
+
+        Ok(())
+    }
+
     #[test]
     fn test_iterator() -> StdResult<()> {
         let mut storage = MockStorage::new();
diff --git a/packages/storage/src/deque_store.rs b/packages/storage/src/deque_store.rs
@@ -5,6 +5,7 @@
 //! A special key is reserved for storing the length of the collection so far.
 //! Another special key is reserved for storing the offset of the collection.
 use std::any::type_name;
+use std::cell::Cell;
 use std::{convert::TryInto};
 use std::marker::PhantomData;
 
@@ -27,6 +28,8 @@ pub struct DequeStore<'a, T, Ser = Bincode2>
     /// needed if any suffixes were added to the original namespace.
     /// therefore it is not necessarily same as the namespace.
     prefix: Option<Vec<u8>>,
+    length: Cell<Option<u32>>,
+    offset: Cell<Option<u32>>,
     item_type: PhantomData<T>,
     serialization_type: PhantomData<Ser>,
 }
@@ -37,6 +40,8 @@ impl<'a, 'b, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser>
         Self {
             namespace: prefix,
             prefix: None,
+            length: Cell::new(None),
+            offset: Cell::new(None),
             item_type: PhantomData,
             serialization_type: PhantomData,
         }
@@ -52,6 +57,8 @@ impl<'a, 'b, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser>
         Self {
             namespace: self.namespace,
             prefix: Some(prefix),
+            length: Cell::new(None),
+            offset: Cell::new(None),
             item_type: self.item_type,
             serialization_type: self.serialization_type,
         }
@@ -61,11 +68,33 @@ impl<'a, 'b, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser>
 impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
     /// gets the length from storage, and otherwise sets it to 0
     pub fn get_len<S: ReadonlyStorage>(&self, storage: &S) -> StdResult<u32> {
-        self._get_u32(storage, LEN_KEY)
+        match self.length.get() {
+            Some(len) => { Ok(len) },
+            None => {
+                match self._get_u32(storage, LEN_KEY) {
+                    Ok(len) => {
+                        self.length.set(Some(len));
+                        Ok(len)
+                    },
+                    Err(e) => { Err(e) },
+                }
+            },
+        }
     }
     /// gets the offset from storage, and otherwise sets it to 0
     pub fn get_off<S: ReadonlyStorage>(&self, storage: &S) -> StdResult<u32> {
-        self._get_u32(storage, OFFSET_KEY)
+        match self.offset.get() {
+            Some(len) => { Ok(len) },
+            None => {
+                match self._get_u32(storage, OFFSET_KEY) {
+                    Ok(len) => {
+                        self.offset.set(Some(len));
+                        Ok(len)
+                    },
+                    Err(e) => { Err(e) },
+                }
+            },
+        }
     }
     /// gets offset or length
     fn _get_u32<S: ReadonlyStorage>(&self, storage: &S, key: &[u8]) -> StdResult<u32> {
@@ -99,10 +128,12 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
     }
     /// Set the length of the collection
     fn set_len<S: Storage>(&self, storage: &mut S, len: u32) {
+        self.length.set(Some(len));
         self._set_u32(storage, LEN_KEY, len)
     }
     /// Set the offset of the collection
     fn set_off<S: Storage>(&self, storage: &mut S, off: u32) {
+        self.offset.set(Some(off));
         self._set_u32(storage, OFFSET_KEY, off)
     }
     /// Set the length or offset of the collection
@@ -248,25 +279,14 @@ impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> DequeStore<'a, T, Ser> {
     }
 }
 
-impl<'a, T: Serialize + DeserializeOwned, Ser: Serde> Clone for DequeStore<'a, T, Ser> {
-    fn clone(&self) -> Self {
-        Self {
-            namespace: self.namespace.clone(),
-            prefix: self.prefix.clone(),
-            item_type: self.item_type.clone(),
-            serialization_type: self.serialization_type.clone()
-        }
-    }
-}
-
 /// An iterator over the contents of the deque store.
 pub struct DequeStoreIter<'a, T, S, Ser>
 where
     T: Serialize + DeserializeOwned,
     S: ReadonlyStorage,
     Ser: Serde,
 {
-    deque_store: DequeStore<'a, T, Ser>,
+    deque_store: &'a DequeStore<'a, T, Ser>,
     storage: &'a S,
     start: u32,
     end: u32,
@@ -286,7 +306,7 @@ impl<'a, T, S, Ser> DequeStoreIter<'a, T, S, Ser>
         end: u32
     ) -> Self {
         Self {
-            deque_store: deque_store.clone(),
+            deque_store,
             storage,
             start,
             end,
diff --git a/packages/storage/src/keymap.rs b/packages/storage/src/keymap.rs
@@ -366,19 +366,6 @@ impl<'a, K: Serialize + DeserializeOwned, T: Serialize + DeserializeOwned, Ser:
     }
 }
 
-impl<'a, K: Serialize + DeserializeOwned, T: Serialize + DeserializeOwned, Ser: Serde> Clone for Keymap<'a, K, T, Ser> {
-    fn clone(&self) -> Self {
-        Self {
-            namespace: self.namespace.clone(),
-            prefix: self.prefix.clone(),
-            length: Cell::new(None),
-            key_type: self.key_type.clone(),
-            item_type: self.item_type.clone(),
-            serialization_type: self.serialization_type.clone()
-        }
-    }
-}
-
 /// An iterator over the keys of the Keymap.
 pub struct KeyIter<'a, K, T, S, Ser>
 where
@@ -1275,29 +1262,36 @@ mod tests {
             number: 1111,
         };
 
+        assert_eq!(keymap.length, Cell::new(None));
         assert_eq!(keymap.get_len(&storage)?, 0);
+        assert_eq!(keymap.length, Cell::new(Some(0)));
 
         let key1 = "k1".to_string();
         let key2 = "k2".to_string();
 
         keymap.insert(&mut storage, &key1, foo1.clone())?;
         assert_eq!(keymap.get_len(&storage)?, 1);
+        assert_eq!(keymap.length, Cell::new(Some(1)));
 
         // add another item
         keymap.insert(&mut storage, &key2, foo2.clone())?;
         assert_eq!(keymap.get_len(&storage)?, 2);
+        assert_eq!(keymap.length, Cell::new(Some(2)));
 
         // remove item and check length
         keymap.remove(&mut storage, &key1)?;
         assert_eq!(keymap.get_len(&storage)?, 1);
+        assert_eq!(keymap.length, Cell::new(Some(1)));
 
         // override item (should not change length)
         keymap.insert(&mut storage, &key2, foo1)?;
         assert_eq!(keymap.get_len(&storage)?, 1);
+        assert_eq!(keymap.length, Cell::new(Some(1)));
 
         // remove item and check length
         keymap.remove(&mut storage, &key2)?;
         assert_eq!(keymap.get_len(&storage)?, 0);
+        assert_eq!(keymap.length, Cell::new(Some(0)));
 
         Ok(())
     }
