Merge pull request #35 from tweedegolf/binsize-opts

Binary size optimizations and map interface changes

CHANGELOG.md

@@ -4,6 +4,13 @@
 
 ## Unreleased
 
+- *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.
+- *Breaking:* Removed the `StorageItem` trait in favor of two separate `Key` and `Value` traits. This helps cut
+ binary size and is closer to what users of the map APIs were expecting.
+- *Breaking:* The error type is no longer generic over the Item error. That error variant has been renamed `MapValueError`
+ and carries a predefined error subtype.
+
 ## 1.0.0 01-03-24
 
 - *Breaking:* Corruption repair is automatic now! The repair functions have been made private.

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "sequential-storage"
-version = "1.0.0"
+version = "2.0.0"
 edition = "2021"
 license = "MIT OR Apache-2.0"
 description = "A crate for storing data in flash with minimal erase cycles."

README.md

@@ -25,8 +25,8 @@ The in-flash representation is not (yet?) stable. This too follows semver.
 
 - A breaking change to the in-flash representation will lead to a major version bump
 - A feature addition will lead to a minor version bump
- - This is always backward-compatible. So data created by e.g. `0.8.0` can be used by `0.8.1`.
- - This may not be forward-compatible. So data created by e.g. `0.8.1` may not be usable by `0.8.0`.
+ - This is always backward-compatible. So data created by e.g. `1.0.0` can be used by `1.1.0`.
+ - This may not be forward-compatible. So data created by e.g. `1.0.1` may not be usable by `1.0.0`.
 - After 1.0, patch releases only fix bugs and don't change the in-flash representation
 
 For any update, consult the changelog to see what changed. Any externally noticable changes are recorded there.
@@ -88,8 +88,8 @@ These numbers are taken from the test cases in the cache module:
 | ---------------: | -------------------------------------------: | ----------------: | -------------------: | ------------------: | ---------------------: |
 | NoCache | 0 | 100% | 100% | 100% | 100% |
 | PageStateCache | 1 * num pages | 77% | 97% | 51% | 90% |
-| PagePointerCache | 9 * num pages | 69% | 89% | 35% | 61% |
-| KeyPointerCache | 9 * num pages + (sizeof(KEY) + 4) * num keys | 6.5% | 8.5% | - | - |
+| PagePointerCache | 9 * num pages | 70% | 89% | 35% | 61% |
+| KeyPointerCache | 9 * num pages + (sizeof(KEY) + 4) * num keys | 6.2% | 8.2% | - | - |
 
 #### Takeaways
 

fuzz/fuzz_targets/map.rs

@@ -6,7 +6,6 @@ use libfuzzer_sys::fuzz_target;
 use rand::SeedableRng;
 use sequential_storage::{
  cache::{KeyCacheImpl, KeyPointerCache, NoCache, PagePointerCache, PageStateCache},
- map::StorageItem,
  mock_flash::{MockFlashBase, MockFlashError, WriteCountCheck},
  Error,
 };
@@ -45,57 +44,13 @@ struct StoreOp {
 }
 
 impl StoreOp {
- fn into_test_item(self, rng: &mut impl rand::Rng) -> TestItem {
- TestItem {
- key: self.key,
- value: (0..(self.value_len % 8) as usize)
+ fn into_test_item(self, rng: &mut impl rand::Rng) -> (u8, Vec<u8>) {
+ (
+ self.key,
+ (0..(self.value_len % 8) as usize)
  .map(|_| rng.gen())
  .collect(),
- }
- }
-}
-
-#[derive(Debug, Clone, PartialEq, Eq)]
-struct TestItem {
- key: u8,
- value: Vec<u8>,
-}
-
-impl StorageItem for TestItem {
- type Key = u8;
-
- type Error = ();
-
- fn serialize_into(&self, buffer: &mut [u8]) -> Result<usize, Self::Error> {
- if buffer.len() < 1 + self.value.len() {
- return Err(());
- }
-
- buffer[0] = self.key;
- buffer[1..][..self.value.len()].copy_from_slice(&self.value);
-
- Ok(1 + self.value.len())
- }
-
- fn deserialize_from(buffer: &[u8]) -> Result<Self, Self::Error>
- where
- Self: Sized,
- {
- Ok(Self {
- key: buffer[0],
- value: buffer[1..].to_vec(),
- })
- }
-
- fn key(&self) -> Self::Key {
- self.key
- }
-
- fn deserialize_key_only(buffer: &[u8]) -> Result<Self::Key, Self::Error>
- where
- Self: Sized,
- {
- Ok(buffer[0])
+ )
  }
 }
 
@@ -139,36 +94,35 @@ fn fuzz(ops: Input, mut cache: impl KeyCacheImpl<u8> + Debug) {
 
  match op.clone() {
  Op::Store(op) => {
- let item = op.into_test_item(&mut rng);
+ let (key, value) = op.into_test_item(&mut rng);
  match block_on(sequential_storage::map::store_item(
  &mut flash,
  FLASH_RANGE,
  &mut cache,
  &mut buf.0,
- &item,
+ key,
+ &value.as_slice(),
  )) {
  Ok(_) => {
- map.insert(item.key, item.value);
+ map.insert(key, value);
  }
  Err(Error::FullStorage) => {}
  Err(Error::Storage {
  value: MockFlashError::EarlyShutoff(_),
  backtrace: _backtrace,
  }) => {
- match block_on(sequential_storage::map::fetch_item::<TestItem, _>(
+ match block_on(sequential_storage::map::fetch_item::<u8, &[u8], _>(
  &mut flash,
  FLASH_RANGE,
  &mut cache,
  &mut buf.0,
- item.key,
+ key,
  )) {
- Ok(Some(check_item))
- if check_item.key == item.key && check_item.value == item.value =>
- {
+ Ok(Some(check_item)) if check_item == value => {
  #[cfg(fuzzing_repro)]
  eprintln!("Early shutoff when storing {item:?}! (but it still stored fully). Originated from:\n{_backtrace:#}");
  // Even though we got a shutoff, it still managed to store well
- map.insert(item.key, item.value);
+ map.insert(key, value);
  }
  _ => {
  // Could not fetch the item we stored...
@@ -188,7 +142,7 @@ fn fuzz(ops: Input, mut cache: impl KeyCacheImpl<u8> + Debug) {
  }
  }
  Op::Fetch(key) => {
- match block_on(sequential_storage::map::fetch_item::<TestItem, _>(
+ match block_on(sequential_storage::map::fetch_item::<u8, &[u8], _>(
  &mut flash,
  FLASH_RANGE,
  &mut cache,
@@ -199,8 +153,7 @@ fn fuzz(ops: Input, mut cache: impl KeyCacheImpl<u8> + Debug) {
  let map_value = map
  .get(&key)
  .expect(&format!("Map doesn't contain: {fetch_result:?}"));
- assert_eq!(key, fetch_result.key, "Mismatching keys");
- assert_eq!(map_value, &fetch_result.value, "Mismatching values");
+ assert_eq!(map_value, &fetch_result, "Mismatching values");
  }
  Ok(None) => {
  assert_eq!(None, map.get(&key));
@@ -223,7 +176,7 @@ fn fuzz(ops: Input, mut cache: impl KeyCacheImpl<u8> + Debug) {
  }
  }
  Op::Remove(key) => {
- match block_on(sequential_storage::map::remove_item::<TestItem, _>(
+ match block_on(sequential_storage::map::remove_item::<u8, _>(
  &mut flash,
  FLASH_RANGE,
  &mut cache,
@@ -237,7 +190,7 @@ fn fuzz(ops: Input, mut cache: impl KeyCacheImpl<u8> + Debug) {
  value: MockFlashError::EarlyShutoff(_),
  backtrace: _backtrace,
  }) => {
- match block_on(sequential_storage::map::fetch_item::<TestItem, _>(
+ match block_on(sequential_storage::map::fetch_item::<u8, &[u8], _>(
  &mut flash,
  FLASH_RANGE,
  &mut cache,

src/cache/mod.rs

@@ -23,11 +23,10 @@ pub(crate) use page_states::PageStatesCache;
 /// Trait implemented by all cache types
 #[allow(private_bounds)]
 pub trait CacheImpl: PrivateCacheImpl {}
-impl<T: CacheImpl> CacheImpl for &mut T {}
+
 /// Trait implemented by all cache types that know about keys
 #[allow(private_bounds)]
 pub trait KeyCacheImpl<KEY: Eq>: CacheImpl + PrivateKeyCacheImpl<KEY> {}
-impl<KEY: Eq, T: KeyCacheImpl<KEY>> KeyCacheImpl<KEY> for &mut T {}
 
 pub(crate) trait Invalidate {
  fn invalidate_cache_state(&mut self);
@@ -216,6 +215,12 @@ impl NoCache {
  }
 }
 
+impl Default for NoCache {
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
 impl PrivateCacheImpl for NoCache {
  type PSC = UncachedPageStates;
  type PPC = UncachedPagePointers;
@@ -280,6 +285,12 @@ impl<const PAGE_COUNT: usize> PageStateCache<PAGE_COUNT> {
  }
 }
 
+impl<const PAGE_COUNT: usize> Default for PageStateCache<PAGE_COUNT> {
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
 impl<const PAGE_COUNT: usize> PrivateCacheImpl for PageStateCache<PAGE_COUNT> {
  type PSC = CachedPageStates<PAGE_COUNT>;
  type PPC = UncachedPagePointers;
@@ -347,6 +358,12 @@ impl<const PAGE_COUNT: usize> PagePointerCache<PAGE_COUNT> {
  }
 }
 
+impl<const PAGE_COUNT: usize> Default for PagePointerCache<PAGE_COUNT> {
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
 impl<const PAGE_COUNT: usize> PrivateCacheImpl for PagePointerCache<PAGE_COUNT> {
  type PSC = CachedPageStates<PAGE_COUNT>;
  type PPC = CachedPagePointers<PAGE_COUNT>;
@@ -419,6 +436,14 @@ impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> KeyPointerCache<PAGE_C
  }
 }
 
+impl<const PAGE_COUNT: usize, KEY: Eq, 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
  for KeyPointerCache<PAGE_COUNT, KEY, KEYS>
 {