Take keys by reference

tweedegolf · diondokter · Jul 17, 2024 · Jul 17, 2024 · Jul 17, 2024 · Jul 17, 2024
commit 838f4a6080f8d79473d692ce8c8f9899ca0bd8df
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,18 +4,21 @@
 
 ## Unreleased
 
+## 3.0.0 17-07-24
+
+- *Breaking:* Map keys are now always passed by reference. This avoids extra cloning and memory use for bigger keys.
 - Added `space_left` function for queue
 - Added a new `map::remove_all_items()` API to remove all stored items in flash.
 
-# 2.0.2 07-05-24
+## 2.0.2 07-05-24
 
 - Added check for too big items that won't ever fit in flash so it returns a good clear error.
 
-# 2.0.1 06-05-24
+## 2.0.1 06-05-24
 
 - Implemented the `get_len` function for all built-in key types
 
-# 2.0.0 06-05-24
+## 2.0.0 06-05-24
 
 - *Breaking:* Made the cache API a bit more strict. Caches now always have to be passed as a mutable reference.
   The API before would lead to a lot of extra unncesessary binary size.

diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "sequential-storage"
-version = "2.0.2"
+version = "3.0.0"
 edition = "2021"
 license = "MIT OR Apache-2.0"
 description = "A crate for storing data in flash with minimal erase cycles."

diff --git a/example/Cargo.lock b/example/Cargo.lock
diff --git a/example/src/main.rs b/example/src/main.rs
@@ -96,7 +96,7 @@ async fn run_map<E: defmt::Format>(
             flash_range.clone(),
             &mut NoCache::new(),
             &mut data_buffer,
-            0u8,
+            &0u8,
             &0u8,
         )
         .await
@@ -108,7 +108,7 @@ async fn run_map<E: defmt::Format>(
             flash_range.clone(),
             &mut NoCache::new(),
             &mut data_buffer,
-            1u8,
+            &1u8,
             &123u32,
         )
         .await
@@ -120,7 +120,7 @@ async fn run_map<E: defmt::Format>(
             flash_range.clone(),
             &mut NoCache::new(),
             &mut data_buffer,
-            2u8,
+            &2u8,
             &0.123f32,
         )
         .await
@@ -132,7 +132,7 @@ async fn run_map<E: defmt::Format>(
             flash_range.clone(),
             &mut NoCache::new(),
             &mut data_buffer,
-            3,
+            &3,
         )
         .await
     );
@@ -145,7 +145,7 @@ async fn run_map<E: defmt::Format>(
             flash_range.clone(),
             &mut NoCache::new(),
             &mut data_buffer,
-            1,
+            &1,
         )
         .await
     );

diff --git a/fuzz/fuzz_targets/map.rs b/fuzz/fuzz_targets/map.rs
@@ -105,7 +105,7 @@ fn fuzz(ops: Input, mut cache: impl KeyCacheImpl<u8> + Debug) {
                     FLASH_RANGE,
                     &mut cache,
                     &mut buf.0,
-                    key,
+                    &key,
                     &value.as_slice(),
                 )) {
                     Ok(_) => {
@@ -121,7 +121,7 @@ fn fuzz(ops: Input, mut cache: impl KeyCacheImpl<u8> + Debug) {
                             FLASH_RANGE,
                             &mut cache,
                             &mut buf.0,
-                            key,
+                            &key,
                         )) {
                             Ok(Some(check_item)) if check_item == value => {
                                 #[cfg(fuzzing_repro)]
@@ -152,7 +152,7 @@ fn fuzz(ops: Input, mut cache: impl KeyCacheImpl<u8> + Debug) {
                     FLASH_RANGE,
                     &mut cache,
                     &mut buf.0,
-                    key,
+                    &key,
                 )) {
                     Ok(Some(fetch_result)) => {
                         let map_value = map
@@ -186,7 +186,7 @@ fn fuzz(ops: Input, mut cache: impl KeyCacheImpl<u8> + Debug) {
                     FLASH_RANGE,
                     &mut cache,
                     &mut buf.0,
-                    key,
+                    &key,
                 )) {
                     Ok(()) => {
                         map.remove(&key);
@@ -201,7 +201,7 @@ fn fuzz(ops: Input, mut cache: impl KeyCacheImpl<u8> + Debug) {
                             FLASH_RANGE,
                             &mut cache,
                             &mut buf.0,
-                            key,
+                            &key,
                         )) {
                             Ok(Some(_)) => {
                                 #[cfg(fuzzing_repro)]
@@ -247,7 +247,7 @@ fn fuzz(ops: Input, mut cache: impl KeyCacheImpl<u8> + Debug) {
                                 FLASH_RANGE,
                                 &mut cache,
                                 &mut buf.0,
-                                key,
+                                &key,
                             )) {
                                 Ok(Some(_)) => {
                                     #[cfg(fuzzing_repro)]

diff --git a/src/cache/key_pointers.rs b/src/cache/key_pointers.rs
@@ -1,9 +1,11 @@
 use core::{fmt::Debug, num::NonZeroU32};
 
-pub(crate) trait KeyPointersCache<KEY: Eq> {
+use crate::map::Key;
+
+pub(crate) trait KeyPointersCache<KEY: Key> {
     fn key_location(&self, key: &KEY) -> Option<u32>;
 
-    fn notice_key_location(&mut self, key: KEY, item_address: u32);
+    fn notice_key_location(&mut self, key: &KEY, item_address: u32);
     fn notice_key_erased(&mut self, key: &KEY);
 
     fn invalidate_cache_state(&mut self);
@@ -45,20 +47,20 @@ impl<KEY: Eq, const KEYS: usize> CachedKeyPointers<KEY, KEYS> {
     }
 }
 
-impl<KEY: Eq, const KEYS: usize> KeyPointersCache<KEY> for CachedKeyPointers<KEY, KEYS> {
+impl<KEY: Key, const KEYS: usize> KeyPointersCache<KEY> for CachedKeyPointers<KEY, KEYS> {
     fn key_location(&self, key: &KEY) -> Option<u32> {
         self.key_index(key)
             .map(|index| self.key_pointers[index].as_ref().unwrap().1.get())
     }
 
-    fn notice_key_location(&mut self, key: KEY, item_address: u32) {
+    fn notice_key_location(&mut self, key: &KEY, item_address: u32) {
         match self.key_index(&key) {
             Some(existing_index) => {
                 self.key_pointers[existing_index] =
-                    Some((key, NonZeroU32::new(item_address).unwrap()));
+                    Some((key.clone(), NonZeroU32::new(item_address).unwrap()));
                 move_to_front(&mut self.key_pointers, existing_index);
             }
-            None => self.insert_front((key, NonZeroU32::new(item_address).unwrap())),
+            None => self.insert_front((key.clone(), NonZeroU32::new(item_address).unwrap())),
         }
     }
 
@@ -77,12 +79,12 @@ impl<KEY: Eq, const KEYS: usize> KeyPointersCache<KEY> for CachedKeyPointers<KEY
 #[derive(Debug)]
 pub(crate) struct UncachedKeyPointers;
 
-impl<KEY: Eq> KeyPointersCache<KEY> for UncachedKeyPointers {
+impl<KEY: Key> KeyPointersCache<KEY> for UncachedKeyPointers {
     fn key_location(&self, _key: &KEY) -> Option<u32> {
         None
     }
 
-    fn notice_key_location(&mut self, _key: KEY, _item_address: u32) {}
+    fn notice_key_location(&mut self, _key: &KEY, _item_address: u32) {}
 
     fn notice_key_erased(&mut self, _key: &KEY) {}
 

diff --git a/src/cache/mod.rs b/src/cache/mod.rs
@@ -4,7 +4,7 @@ use core::{fmt::Debug, ops::Range};
 
 use embedded_storage_async::nor_flash::NorFlash;
 
-use crate::{item::ItemHeader, PageState};
+use crate::{item::ItemHeader, map::Key, PageState};
 
 use self::{
     key_pointers::{CachedKeyPointers, KeyPointersCache, UncachedKeyPointers},
@@ -26,7 +26,7 @@ pub trait CacheImpl: PrivateCacheImpl {}
 
 /// Trait implemented by all cache types that know about keys
 #[allow(private_bounds)]
-pub trait KeyCacheImpl<KEY: Eq>: CacheImpl + PrivateKeyCacheImpl<KEY> {}
+pub trait KeyCacheImpl<KEY: Key>: CacheImpl + PrivateKeyCacheImpl<KEY> {}
 
 pub(crate) trait Invalidate {
     fn invalidate_cache_state(&mut self);
@@ -133,7 +133,7 @@ impl<T: PrivateCacheImpl> PrivateCacheImpl for &mut T {
     }
 }
 
-pub(crate) trait PrivateKeyCacheImpl<KEY: Eq>: PrivateCacheImpl {
+pub(crate) trait PrivateKeyCacheImpl<KEY: Key>: PrivateCacheImpl {
     type KPC: KeyPointersCache<KEY>;
 
     fn key_pointers(&mut self) -> &mut Self::KPC;
@@ -142,7 +142,7 @@ pub(crate) trait PrivateKeyCacheImpl<KEY: Eq>: PrivateCacheImpl {
         self.key_pointers().key_location(key)
     }
 
-    fn notice_key_location(&mut self, key: KEY, item_address: u32, dirty: bool) {
+    fn notice_key_location(&mut self, key: &KEY, item_address: u32, dirty: bool) {
         if dirty {
             self.mark_dirty();
         }
@@ -155,7 +155,7 @@ pub(crate) trait PrivateKeyCacheImpl<KEY: Eq>: PrivateCacheImpl {
     }
 }
 
-impl<KEY: Eq, T: PrivateKeyCacheImpl<KEY>> PrivateKeyCacheImpl<KEY> for &mut T {
+impl<KEY: Key, T: PrivateKeyCacheImpl<KEY>> PrivateKeyCacheImpl<KEY> for &mut T {
     type KPC = T::KPC;
 
     fn key_pointers(&mut self) -> &mut Self::KPC {
@@ -240,13 +240,13 @@ impl PrivateCacheImpl for NoCache {
 }
 
 impl CacheImpl for NoCache {}
-impl<KEY: Eq> KeyCacheImpl<KEY> for NoCache {}
+impl<KEY: Key> KeyCacheImpl<KEY> for NoCache {}
 
 impl Invalidate for NoCache {
     fn invalidate_cache_state(&mut self) {}
 }
 
-impl<KEY: Eq> PrivateKeyCacheImpl<KEY> for NoCache {
+impl<KEY: Key> PrivateKeyCacheImpl<KEY> for NoCache {
     type KPC = UncachedKeyPointers;
 
     fn key_pointers(&mut self) -> &mut Self::KPC {
@@ -309,7 +309,7 @@ impl<const PAGE_COUNT: usize> PrivateCacheImpl for PageStateCache<PAGE_COUNT> {
 }
 
 impl<const PAGE_COUNT: usize> CacheImpl for PageStateCache<PAGE_COUNT> {}
-impl<KEY: Eq, const PAGE_COUNT: usize> KeyCacheImpl<KEY> for PageStateCache<PAGE_COUNT> {}
+impl<KEY: Key, const PAGE_COUNT: usize> KeyCacheImpl<KEY> for PageStateCache<PAGE_COUNT> {}
 
 impl<const PAGE_COUNT: usize> Invalidate for PageStateCache<PAGE_COUNT> {
     fn invalidate_cache_state(&mut self) {
@@ -319,7 +319,7 @@ impl<const PAGE_COUNT: usize> Invalidate for PageStateCache<PAGE_COUNT> {
     }
 }
 
-impl<KEY: Eq, const PAGE_COUNT: usize> PrivateKeyCacheImpl<KEY> for PageStateCache<PAGE_COUNT> {
+impl<KEY: Key, const PAGE_COUNT: usize> PrivateKeyCacheImpl<KEY> for PageStateCache<PAGE_COUNT> {
     type KPC = UncachedKeyPointers;
 
     fn key_pointers(&mut self) -> &mut Self::KPC {
@@ -382,7 +382,7 @@ impl<const PAGE_COUNT: usize> PrivateCacheImpl for PagePointerCache<PAGE_COUNT>
 }
 
 impl<const PAGE_COUNT: usize> CacheImpl for PagePointerCache<PAGE_COUNT> {}
-impl<KEY: Eq, const PAGE_COUNT: usize> KeyCacheImpl<KEY> for PagePointerCache<PAGE_COUNT> {}
+impl<KEY: Key, const PAGE_COUNT: usize> KeyCacheImpl<KEY> for PagePointerCache<PAGE_COUNT> {}
 
 impl<const PAGE_COUNT: usize> Invalidate for PagePointerCache<PAGE_COUNT> {
     fn invalidate_cache_state(&mut self) {
@@ -392,7 +392,7 @@ impl<const PAGE_COUNT: usize> Invalidate for PagePointerCache<PAGE_COUNT> {
     }
 }
 
-impl<KEY: Eq, const PAGE_COUNT: usize> PrivateKeyCacheImpl<KEY> for PagePointerCache<PAGE_COUNT> {
+impl<KEY: Key, const PAGE_COUNT: usize> PrivateKeyCacheImpl<KEY> for PagePointerCache<PAGE_COUNT> {
     type KPC = UncachedKeyPointers;
 
     fn key_pointers(&mut self) -> &mut Self::KPC {
@@ -417,14 +417,14 @@ impl<KEY: Eq, const PAGE_COUNT: usize> PrivateKeyCacheImpl<KEY> for PagePointerC
 /// the chance of a cache hit.
 /// The keys are cached in a fifo and any time its location is updated in cache it's added to the front.
 #[derive(Debug)]
-pub struct KeyPointerCache<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> {
+pub struct KeyPointerCache<const PAGE_COUNT: usize, KEY: Key, const KEYS: usize> {
     dirt_tracker: DirtTracker,
     page_states: CachedPageStates<PAGE_COUNT>,
     page_pointers: CachedPagePointers<PAGE_COUNT>,
     key_pointers: CachedKeyPointers<KEY, KEYS>,
 }
 
-impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> KeyPointerCache<PAGE_COUNT, KEY, KEYS> {
+impl<const PAGE_COUNT: usize, KEY: Key, const KEYS: usize> KeyPointerCache<PAGE_COUNT, KEY, KEYS> {
     /// Construct a new instance
     pub const fn new() -> Self {
         Self {
@@ -436,15 +436,15 @@ impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> KeyPointerCache<PAGE_C
     }
 }
 
-impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> Default
+impl<const PAGE_COUNT: usize, KEY: Key, const KEYS: usize> Default
     for KeyPointerCache<PAGE_COUNT, KEY, KEYS>
 {
     fn default() -> Self {
         Self::new()
     }
 }
 
-impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> PrivateCacheImpl
+impl<const PAGE_COUNT: usize, KEY: Key, const KEYS: usize> PrivateCacheImpl
     for KeyPointerCache<PAGE_COUNT, KEY, KEYS>
 {
     type PSC = CachedPageStates<PAGE_COUNT>;
@@ -463,16 +463,16 @@ impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> PrivateCacheImpl
     }
 }
 
-impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> CacheImpl
+impl<const PAGE_COUNT: usize, KEY: Key, const KEYS: usize> CacheImpl
     for KeyPointerCache<PAGE_COUNT, KEY, KEYS>
 {
 }
-impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> KeyCacheImpl<KEY>
+impl<const PAGE_COUNT: usize, KEY: Key, const KEYS: usize> KeyCacheImpl<KEY>
     for KeyPointerCache<PAGE_COUNT, KEY, KEYS>
 {
 }
 
-impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> Invalidate
+impl<const PAGE_COUNT: usize, KEY: Key, const KEYS: usize> Invalidate
     for KeyPointerCache<PAGE_COUNT, KEY, KEYS>
 {
     fn invalidate_cache_state(&mut self) {
@@ -483,7 +483,7 @@ impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> Invalidate
     }
 }
 
-impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> PrivateKeyCacheImpl<KEY>
+impl<const PAGE_COUNT: usize, KEY: Key, const KEYS: usize> PrivateKeyCacheImpl<KEY>
     for KeyPointerCache<PAGE_COUNT, KEY, KEYS>
 {
     type KPC = CachedKeyPointers<KEY, KEYS>;

diff --git a/src/cache/tests.rs b/src/cache/tests.rs
@@ -213,7 +213,7 @@ mod map_tests {
                     FLASH_RANGE,
                     cache,
                     &mut data_buffer,
-                    i as u16,
+                    &(i as u16),
                     &vec![i as u8; LENGHT_PER_KEY[i]].as_slice(),
                 )
                 .await
@@ -231,7 +231,7 @@ mod map_tests {
                     FLASH_RANGE,
                     cache,
                     &mut data_buffer,
-                    i as u16,
+                    &(i as u16),
                 )
                 .await
                 .unwrap()