From 84ddef7251e9a1ea458a9ffa685621167d0e5729 Mon Sep 17 00:00:00 2001 From: Dion Dokter Date: Thu, 8 Feb 2024 17:23:53 +0100 Subject: [PATCH 1/5] Initial stab at key cache --- src/cache.rs | 750 ------------------------------------- src/cache/key_pointers.rs | 291 ++++++++++++++ src/cache/mod.rs | 294 +++++++++++++++ src/cache/page_pointers.rs | 171 +++++++++ src/cache/page_states.rs | 68 ++++ src/cache/tests.rs | 278 ++++++++++++++ src/item.rs | 14 +- src/lib.rs | 12 +- src/map.rs | 8 +- src/queue.rs | 4 +- 10 files changed, 1122 insertions(+), 768 deletions(-) delete mode 100644 src/cache.rs create mode 100644 src/cache/key_pointers.rs create mode 100644 src/cache/mod.rs create mode 100644 src/cache/page_pointers.rs create mode 100644 src/cache/page_states.rs create mode 100644 src/cache/tests.rs diff --git a/src/cache.rs b/src/cache.rs deleted file mode 100644 index 411a598..0000000 --- a/src/cache.rs +++ /dev/null @@ -1,750 +0,0 @@ -//! Module implementing all things cache related - -use core::{fmt::Debug, num::NonZeroU32, ops::Range}; - -use embedded_storage_async::nor_flash::NorFlash; - -use crate::{ - calculate_page_address, calculate_page_index, item::ItemHeader, NorFlashExt, PageState, -}; - -/// Trait implemented by all cache types -#[allow(private_bounds)] -pub trait CacheImpl: PrivateCacheImpl + Debug {} - -impl CacheImpl for &mut T {} - -impl CacheImpl for Cache {} - -pub(crate) trait PrivateCacheImpl { - type PSC: PageStatesCache; - type PPC: PagePointersCache; - - fn inner(&mut self) -> &mut Cache; -} - -impl PrivateCacheImpl for &mut T { - type PSC = T::PSC; - type PPC = T::PPC; - - fn inner(&mut self) -> &mut Cache { - T::inner(self) - } -} - -impl PrivateCacheImpl for Cache { - type PSC = PSC; - type PPC = PPC; - - fn inner(&mut self) -> &mut Cache { - self - } -} - -/// A cache object implementing no cache. -/// -/// This type of cache doesn't have to be kept around and may be constructed on every api call. -/// You could simply pass `&mut NoCache::new()` every time. -#[derive(Debug)] -pub struct NoCache(Cache); - -impl NoCache { - /// Construct a new instance - pub const fn new() -> Self { - Self(Cache::new(UncachedPageSates, UncachedPagePointers)) - } -} - -impl PrivateCacheImpl for NoCache { - type PSC = UncachedPageSates; - type PPC = UncachedPagePointers; - - fn inner(&mut self) -> &mut Cache { - &mut self.0 - } -} - -impl CacheImpl for NoCache {} - -/// A cache object that keeps track of the page states. -/// -/// This cache has to be kept around and passed to *every* api call to the same memory region until the cache gets discarded. -/// -/// Valid usecase: -/// `Create cache 1` -> `use 1` -> `use 1` -> `create cache 2` -> `use 2` -> `use 2` -/// -/// Invalid usecase: -/// `Create cache 1` -> `use 1` -> `create cache 2` -> `use 2` -> `❌ use 1 ❌` -/// -/// Make sure the page count is correct. If the number is lower than the actual amount, the code will panic at some point. -#[derive(Debug)] -pub struct PageStateCache( - Cache, UncachedPagePointers>, -); - -impl PageStateCache { - /// Construct a new instance - pub const fn new() -> Self { - Self(Cache::new(CachedPageStates::new(), UncachedPagePointers)) - } -} - -impl PrivateCacheImpl for PageStateCache { - type PSC = CachedPageStates; - type PPC = UncachedPagePointers; - - fn inner(&mut self) -> &mut Cache { - &mut self.0 - } -} - -impl CacheImpl for PageStateCache {} - -/// A cache object that keeps track of the page states and some pointers to the items in the page. -/// -/// This cache has to be kept around and passed to *every* api call to the same memory region until the cache gets discarded. -/// -/// Valid usecase: -/// `Create cache 1` -> `use 1` -> `use 1` -> `create cache 2` -> `use 2` -> `use 2` -/// -/// Invalid usecase: -/// `Create cache 1` -> `use 1` -> `create cache 2` -> `use 2` -> `❌ use 1 ❌` -/// -/// Make sure the page count is correct. If the number is lower than the actual amount, the code will panic at some point. -#[derive(Debug)] -pub struct PagePointerCache( - Cache, CachedPagePointers>, -); - -impl PagePointerCache { - /// Construct a new instance - pub const fn new() -> Self { - Self(Cache::new( - CachedPageStates::new(), - CachedPagePointers::new(), - )) - } -} - -impl PrivateCacheImpl for PagePointerCache { - type PSC = CachedPageStates; - type PPC = CachedPagePointers; - - fn inner(&mut self) -> &mut Cache { - &mut self.0 - } -} - -impl CacheImpl for PagePointerCache {} - -/// The cache struct that is behind it all. -/// -/// It manages the cache state in a generic way. For every field it can be chosen to have it be cached or not. -#[derive(Debug)] -pub(crate) struct Cache { - /// Managed from the library code. - /// - /// When true, the cache and/or flash has changed and things might not be fully - /// consistent if there's an early return due to error. - dirty: bool, - page_states: PSC, - page_pointers: PPC, -} - -impl Cache { - pub(crate) const fn new(page_states: PSC, page_pointers: PPC) -> Self { - Self { - dirty: false, - page_states, - page_pointers, - } - } - - /// True if the cache might be inconsistent - pub(crate) fn is_dirty(&self) -> bool { - self.dirty - } - - /// Mark the cache as potentially inconsistent with reality - pub(crate) fn mark_dirty(&mut self) { - self.dirty = true; - } - - /// Mark the cache as being consistent with reality - pub(crate) fn unmark_dirty(&mut self) { - self.dirty = false; - } - - /// Wipe the cache - pub(crate) fn invalidate_cache_state(&mut self) { - self.dirty = false; - self.page_states.invalidate_cache_state(); - self.page_pointers.invalidate_cache_state(); - } - - /// Get the cache state of the requested page - pub(crate) fn get_page_state(&self, page_index: usize) -> Option { - self.page_states.get_page_state(page_index) - } - - /// Let the cache know a page state changed - /// - /// The dirty flag should be true if the page state is actually going to be changed. - /// Keep it false if we're only discovering the state. - pub(crate) fn notice_page_state( - &mut self, - page_index: usize, - new_state: PageState, - dirty: bool, - ) { - if dirty { - self.mark_dirty(); - } - self.page_states.notice_page_state(page_index, new_state); - self.page_pointers.notice_page_state(page_index, new_state); - } - - /// Get the cached address of the first non-erased item in the requested page. - /// - /// This is purely for the items that get erased from start to end. - pub(crate) fn first_item_after_erased(&self, page_index: usize) -> Option { - self.page_pointers.first_item_after_erased(page_index) - } - - /// Get the cached address of the first free unwritten item in the requested page. - pub(crate) fn first_item_after_written(&self, page_index: usize) -> Option { - self.page_pointers.first_item_after_written(page_index) - } - - /// Let the cache know that an item has been written to flash - pub(crate) fn notice_item_written( - &mut self, - flash_range: Range, - item_address: u32, - item_header: &ItemHeader, - ) { - self.mark_dirty(); - self.page_pointers - .notice_item_written::(flash_range, item_address, item_header) - } - - /// Let the cache know that an item has been erased from flash - pub(crate) fn notice_item_erased( - &mut self, - flash_range: Range, - item_address: u32, - item_header: &ItemHeader, - ) { - self.mark_dirty(); - self.page_pointers - .notice_item_erased::(flash_range, item_address, item_header) - } -} - -pub(crate) trait PageStatesCache: Debug { - fn get_page_state(&self, page_index: usize) -> Option; - fn notice_page_state(&mut self, page_index: usize, new_state: PageState); - fn invalidate_cache_state(&mut self); -} - -pub(crate) struct CachedPageStates { - pages: [Option; PAGE_COUNT], -} - -impl Debug for CachedPageStates { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - write!(f, "[")?; - for (i, val) in self.pages.iter().enumerate() { - if i > 0 { - write!(f, ", ")?; - } - - if let Some(val) = val { - write!(f, "{val:?}")?; - } else { - write!(f, "?")?; - } - } - write!(f, "]")?; - - Ok(()) - } -} - -impl CachedPageStates { - pub const fn new() -> Self { - Self { - pages: [None; PAGE_COUNT], - } - } -} - -impl PageStatesCache for CachedPageStates { - fn get_page_state(&self, page_index: usize) -> Option { - self.pages[page_index] - } - - fn notice_page_state(&mut self, page_index: usize, new_state: PageState) { - self.pages[page_index] = Some(new_state); - } - - fn invalidate_cache_state(&mut self) { - *self = Self::new(); - } -} - -#[derive(Debug, Default)] -pub(crate) struct UncachedPageSates; - -impl PageStatesCache for UncachedPageSates { - fn get_page_state(&self, _page_index: usize) -> Option { - None - } - - fn notice_page_state(&mut self, _page_index: usize, _new_state: PageState) {} - - fn invalidate_cache_state(&mut self) {} -} - -pub(crate) trait PagePointersCache: Debug { - fn first_item_after_erased(&self, page_index: usize) -> Option; - fn first_item_after_written(&self, page_index: usize) -> Option; - - fn notice_item_written( - &mut self, - flash_range: Range, - item_address: u32, - item_header: &ItemHeader, - ); - fn notice_item_erased( - &mut self, - flash_range: Range, - item_address: u32, - item_header: &ItemHeader, - ); - - fn notice_page_state(&mut self, page_index: usize, new_state: PageState); - fn invalidate_cache_state(&mut self); -} - -// Use NoneZeroU32 because we never store 0's in here (because of the first page marker) -// and so Option can make use of the niche so we save bytes -pub(crate) struct CachedPagePointers { - after_erased_pointers: [Option; PAGE_COUNT], - after_written_pointers: [Option; PAGE_COUNT], -} - -impl Debug for CachedPagePointers { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - write!(f, "{{ after_erased_pointers: [")?; - for (i, val) in self.after_erased_pointers.iter().enumerate() { - if i > 0 { - write!(f, ", ")?; - } - - if let Some(val) = val { - write!(f, "{:?}", val.get())?; - } else { - write!(f, "?")?; - } - } - write!(f, "], after_written_pointers: [")?; - for (i, val) in self.after_written_pointers.iter().enumerate() { - if i > 0 { - write!(f, ", ")?; - } - - if let Some(val) = val { - write!(f, "{:?}", val.get())?; - } else { - write!(f, "?")?; - } - } - write!(f, "] }}")?; - - Ok(()) - } -} - -impl CachedPagePointers { - pub const fn new() -> Self { - Self { - after_erased_pointers: [None; PAGE_COUNT], - after_written_pointers: [None; PAGE_COUNT], - } - } -} - -impl PagePointersCache for CachedPagePointers { - fn first_item_after_erased(&self, page_index: usize) -> Option { - self.after_erased_pointers[page_index].map(|val| val.get()) - } - - fn first_item_after_written(&self, page_index: usize) -> Option { - self.after_written_pointers[page_index].map(|val| val.get()) - } - - fn notice_item_written( - &mut self, - flash_range: Range, - item_address: u32, - item_header: &ItemHeader, - ) { - let page_index = calculate_page_index::(flash_range, item_address); - - let next_item_address = item_header.next_item_address::(item_address); - - // We only care about the furthest written item, so discard if this is an earlier item - if let Some(first_item_after_written) = self.first_item_after_written(page_index) { - if next_item_address <= first_item_after_written { - return; - } - } - - self.after_written_pointers[page_index] = NonZeroU32::new(next_item_address); - } - - fn notice_item_erased( - &mut self, - flash_range: Range, - item_address: u32, - item_header: &ItemHeader, - ) { - let page_index = calculate_page_index::(flash_range.clone(), item_address); - - // Either the item we point to or the first item on the page - let next_unerased_item = self.first_item_after_erased(page_index).unwrap_or_else(|| { - calculate_page_address::(flash_range, page_index) + S::WORD_SIZE as u32 - }); - - if item_address == next_unerased_item { - self.after_erased_pointers[page_index] = - NonZeroU32::new(item_header.next_item_address::(item_address)); - } - } - - fn notice_page_state(&mut self, page_index: usize, new_state: PageState) { - if new_state.is_open() { - // This page was erased - self.after_erased_pointers[page_index] = None; - self.after_written_pointers[page_index] = None; - } - } - - fn invalidate_cache_state(&mut self) { - self.after_erased_pointers = [None; PAGE_COUNT]; - self.after_written_pointers = [None; PAGE_COUNT]; - } -} - -#[derive(Debug, Default)] -pub(crate) struct UncachedPagePointers; - -impl PagePointersCache for UncachedPagePointers { - fn first_item_after_erased(&self, _page_index: usize) -> Option { - None - } - - fn first_item_after_written(&self, _page_index: usize) -> Option { - None - } - - fn notice_item_written( - &mut self, - _flash_range: Range, - _item_address: u32, - _item_header: &ItemHeader, - ) { - } - - fn notice_item_erased( - &mut self, - _flash_range: Range, - _item_address: u32, - _item_header: &ItemHeader, - ) { - } - - fn notice_page_state(&mut self, _page_index: usize, _new_state: PageState) {} - - fn invalidate_cache_state(&mut self) {} -} - -#[cfg(test)] -mod queue_tests { - use core::ops::Range; - - use crate::{ - mock_flash::{self, FlashStatsResult, WriteCountCheck}, - queue::{peek, pop, push}, - AlignedBuf, - }; - - use super::*; - use futures_test::test; - - const NUM_PAGES: usize = 4; - const LOOP_COUNT: usize = 2000; - - #[test] - async fn no_cache() { - assert_eq!( - run_test(&mut NoCache::new()).await, - FlashStatsResult { - erases: 146, - reads: 594934, - writes: 6299, - bytes_read: 2766058, - bytes_written: 53299 - } - ); - } - - #[test] - async fn page_state_cache() { - assert_eq!( - run_test(&mut PageStateCache::::new()).await, - FlashStatsResult { - erases: 146, - reads: 308740, - writes: 6299, - bytes_read: 2479864, - bytes_written: 53299 - } - ); - } - - #[test] - async fn page_pointer_cache() { - assert_eq!( - run_test(&mut PagePointerCache::::new()).await, - FlashStatsResult { - erases: 146, - reads: 211172, - writes: 6299, - bytes_read: 1699320, - bytes_written: 53299 - } - ); - } - - async fn run_test(mut cache: impl CacheImpl) -> FlashStatsResult { - let mut flash = - mock_flash::MockFlashBase::::new(WriteCountCheck::Twice, None, true); - const FLASH_RANGE: Range = 0x00..0x400; - let mut data_buffer = AlignedBuf([0; 1024]); - - let start_snapshot = flash.stats_snapshot(); - - for i in 0..LOOP_COUNT { - println!("{i}"); - let data = vec![i as u8; i % 20 + 1]; - - println!("PUSH"); - push(&mut flash, FLASH_RANGE, &mut cache, &data, true) - .await - .unwrap(); - assert_eq!( - &peek(&mut flash, FLASH_RANGE, &mut cache, &mut data_buffer) - .await - .unwrap() - .unwrap()[..], - &data, - "At {i}" - ); - println!("POP"); - assert_eq!( - &pop(&mut flash, FLASH_RANGE, &mut cache, &mut data_buffer) - .await - .unwrap() - .unwrap()[..], - &data, - "At {i}" - ); - println!("PEEK"); - assert_eq!( - peek(&mut flash, FLASH_RANGE, &mut cache, &mut data_buffer) - .await - .unwrap(), - None, - "At {i}" - ); - println!("DONE"); - } - - start_snapshot.compare_to(flash.stats_snapshot()) - } -} - -#[cfg(test)] -mod map_tests { - use core::ops::Range; - - use crate::{ - map::{fetch_item, store_item, StorageItem}, - mock_flash::{self, FlashStatsResult, WriteCountCheck}, - AlignedBuf, - }; - - use super::*; - use futures_test::test; - - const NUM_PAGES: usize = 4; - - #[test] - async fn no_cache() { - assert_eq!( - run_test(&mut NoCache::new()).await, - FlashStatsResult { - erases: 198, - reads: 224161, - writes: 5201, - bytes_read: 1770974, - bytes_written: 50401 - } - ); - } - - #[test] - async fn page_state_cache() { - assert_eq!( - run_test(&mut PageStateCache::::new()).await, - FlashStatsResult { - erases: 198, - reads: 171543, - writes: 5201, - bytes_read: 1718356, - bytes_written: 50401 - } - ); - } - - #[test] - async fn page_pointer_cache() { - assert_eq!( - run_test(&mut PagePointerCache::::new()).await, - FlashStatsResult { - erases: 198, - reads: 153667, - writes: 5201, - bytes_read: 1575348, - bytes_written: 50401 - } - ); - } - - #[derive(Debug, PartialEq, Eq)] - struct MockStorageItem { - key: u8, - value: Vec, - } - - #[derive(Debug, PartialEq, Eq)] - enum MockStorageItemError { - BufferTooSmall, - InvalidKey, - BufferTooBig, - } - - impl StorageItem for MockStorageItem { - type Key = u8; - - type Error = MockStorageItemError; - - fn serialize_into(&self, buffer: &mut [u8]) -> Result { - if buffer.len() < 2 + self.value.len() { - return Err(MockStorageItemError::BufferTooSmall); - } - - if self.value.len() > 255 { - return Err(MockStorageItemError::BufferTooBig); - } - - // The serialized value must not be all 0xFF - if self.key == 0xFF { - return Err(MockStorageItemError::InvalidKey); - } - - buffer[0] = self.key; - buffer[1] = self.value.len() as u8; - buffer[2..][..self.value.len()].copy_from_slice(&self.value); - - Ok(2 + self.value.len()) - } - - fn deserialize_from(buffer: &[u8]) -> Result - where - Self: Sized, - { - if buffer.len() < 2 { - return Err(MockStorageItemError::BufferTooSmall); - } - - if buffer[0] == 0xFF { - return Err(MockStorageItemError::InvalidKey); - } - - let len = buffer[1]; - - if buffer.len() < 2 + len as usize { - return Err(MockStorageItemError::BufferTooSmall); - } - - Ok(Self { - key: buffer[0], - value: buffer[2..][..len as usize].to_vec(), - }) - } - - fn key(&self) -> Self::Key { - self.key - } - } - - async fn run_test(mut cache: impl CacheImpl) -> FlashStatsResult { - let mut cache = cache.inner(); - - let mut flash = - mock_flash::MockFlashBase::::new(WriteCountCheck::Twice, None, true); - const FLASH_RANGE: Range = 0x00..0x400; - let mut data_buffer = AlignedBuf([0; 128]); - - const LENGHT_PER_KEY: [usize; 24] = [ - 11, 13, 6, 13, 13, 10, 2, 3, 5, 36, 1, 65, 4, 6, 1, 15, 10, 7, 3, 15, 9, 3, 4, 5, - ]; - - let start_snapshot = flash.stats_snapshot(); - - for _ in 0..100 { - for i in 0..24 { - let item = MockStorageItem { - key: i as u8, - value: vec![i as u8; LENGHT_PER_KEY[i]], - }; - - store_item::<_, _>(&mut flash, FLASH_RANGE, &mut cache, &mut data_buffer, &item) - .await - .unwrap(); - } - - for i in 0..24 { - let item = fetch_item::( - &mut flash, - FLASH_RANGE, - &mut cache, - &mut data_buffer, - i as u8, - ) - .await - .unwrap() - .unwrap(); - - println!("Fetched {item:?}"); - - assert_eq!(item.value, vec![i as u8; LENGHT_PER_KEY[i]]); - } - } - - start_snapshot.compare_to(flash.stats_snapshot()) - } -} diff --git a/src/cache/key_pointers.rs b/src/cache/key_pointers.rs new file mode 100644 index 0000000..0f5a0a5 --- /dev/null +++ b/src/cache/key_pointers.rs @@ -0,0 +1,291 @@ +use core::{fmt::Debug, marker::PhantomData, num::NonZeroU32}; + +pub(crate) trait KeyPointersCache { + type Key: Eq; + + fn key_location(&self, key: &Self::Key) -> Option; + + fn notice_key_location(&mut self, key: Self::Key, item_address: u32); + fn notice_key_erased(&mut self, key: &Self::Key); + + fn invalidate_cache_state(&mut self); +} + +#[derive(Debug)] +pub(crate) struct CachedKeyPointers { + key_pointers: [Option<(KEY, NonZeroU32)>; KEYS], +} + +impl CachedKeyPointers { + const ARRAY_REPEAT_VALUE: Option<(KEY, NonZeroU32)> = None; + + pub(crate) const fn new() -> Self { + Self { + key_pointers: [Self::ARRAY_REPEAT_VALUE; KEYS], + } + } + + fn key_index(&self, key: &KEY) -> Option { + self.key_pointers + .iter() + .enumerate() + .filter_map(|(index, val)| val.as_ref().map(|val| (index, val))) + .find_map( + |(index, (known_key, _))| { + if key == known_key { + Some(index) + } else { + None + } + }, + ) + } + + fn insert_front(&mut self, value: (KEY, NonZeroU32)) { + self.key_pointers[KEYS - 1] = Some(value); + move_to_front(&mut self.key_pointers, KEYS - 1); + } +} + +impl KeyPointersCache for CachedKeyPointers { + type Key = KEY; + + fn key_location(&self, key: &Self::Key) -> Option { + self.key_index(key) + .map(|index| self.key_pointers[index].as_ref().unwrap().1.get()) + } + + fn notice_key_location(&mut self, key: Self::Key, item_address: u32) { + match self.key_index(&key) { + Some(existing_index) => { + self.key_pointers[existing_index] = + Some((key, NonZeroU32::new(item_address).unwrap())); + move_to_front(&mut self.key_pointers, existing_index); + } + None => self.insert_front((key, NonZeroU32::new(item_address).unwrap())), + } + } + + fn notice_key_erased(&mut self, key: &Self::Key) { + match self.key_index(key) { + Some(existing_index) => { + self.key_pointers[existing_index] = None; + move_to_back(&mut self.key_pointers, existing_index); + } + None => {} + } + } + + fn invalidate_cache_state(&mut self) { + *self = Self::new(); + } +} + +#[derive(Debug)] +pub(crate) struct UncachedKeyPointers; + +impl KeyPointersCache for UncachedKeyPointers { + type Key = (); + + fn key_location(&self, _key: &Self::Key) -> Option { + None + } + + fn notice_key_location(&mut self, _key: Self::Key, _item_address: u32) {} + + fn notice_key_erased(&mut self, _key: &Self::Key) {} + + fn invalidate_cache_state(&mut self) {} +} + +fn move_to_front(data: &mut [Option], index: usize) { + assert!(index < data.len()); + + // Swap the item we're moving into this temporary + let mut item = None; + core::mem::swap(&mut item, &mut data[index]); + + unsafe { + // Move the items until the index back one. + // This overwrites the None we just put in. + // This is fine because it's none and no drop has to occur + let ptr = data.as_mut_ptr(); + ptr.copy_to(ptr.add(1), index); + + // The item in front is now duplicated. + // Swap back our item into the front. + core::mem::swap(&mut item, &mut data[0]); + // The duplicated item must not drop, so just forget it + core::mem::forget(item); + } +} + +fn move_to_back(data: &mut [Option], index: usize) { + assert!(index < data.len()); + + // Swap the item we're moving into this temporary + let mut item = None; + core::mem::swap(&mut item, &mut data[index]); + + unsafe { + // Move the items until the index back one. + // This overwrites the None we just put in. + // This is fine because it's none and no drop has to occur + let ptr = data.as_mut_ptr(); + ptr.add(index + 1) + .copy_to(ptr.add(index), data.len() - 1 - index); + + // The item in front is now duplicated. + // Swap back our item into the back. + core::mem::swap(&mut item, &mut data[data.len() - 1]); + // The duplicated item must not drop, so just forget it + core::mem::forget(item); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_move_to_front() { + let mut array = [ + Some("0".into()), + Some("1".into()), + Some("2".into()), + Some("3".into()), + ]; + move_to_front::(&mut array, 0); + assert_eq!( + array, + [ + Some("0".into()), + Some("1".into()), + Some("2".into()), + Some("3".into()), + ] + ); + + let mut array = [ + Some("0".into()), + Some("1".into()), + Some("2".into()), + Some("3".into()), + ]; + move_to_front::(&mut array, 1); + assert_eq!( + array, + [ + Some("1".into()), + Some("0".into()), + Some("2".into()), + Some("3".into()), + ] + ); + + let mut array = [ + Some("0".into()), + Some("1".into()), + Some("2".into()), + Some("3".into()), + ]; + move_to_front::(&mut array, 2); + assert_eq!( + array, + [ + Some("2".into()), + Some("0".into()), + Some("1".into()), + Some("3".into()), + ] + ); + + let mut array = [ + Some("0".into()), + Some("1".into()), + Some("2".into()), + Some("3".into()), + ]; + move_to_front::(&mut array, 3); + assert_eq!( + array, + [ + Some("3".into()), + Some("0".into()), + Some("1".into()), + Some("2".into()), + ] + ); + } + + #[test] + fn test_move_to_back() { + let mut array = [ + Some("0".into()), + Some("1".into()), + Some("2".into()), + Some("3".into()), + ]; + move_to_back::(&mut array, 0); + assert_eq!( + array, + [ + Some("1".into()), + Some("2".into()), + Some("3".into()), + Some("0".into()), + ] + ); + + let mut array = [ + Some("0".into()), + Some("1".into()), + Some("2".into()), + Some("3".into()), + ]; + move_to_back::(&mut array, 1); + assert_eq!( + array, + [ + Some("0".into()), + Some("2".into()), + Some("3".into()), + Some("1".into()), + ] + ); + + let mut array = [ + Some("0".into()), + Some("1".into()), + Some("2".into()), + Some("3".into()), + ]; + move_to_back::(&mut array, 2); + assert_eq!( + array, + [ + Some("0".into()), + Some("1".into()), + Some("3".into()), + Some("2".into()), + ] + ); + + let mut array = [ + Some("0".into()), + Some("1".into()), + Some("2".into()), + Some("3".into()), + ]; + move_to_back::(&mut array, 3); + assert_eq!( + array, + [ + Some("0".into()), + Some("1".into()), + Some("2".into()), + Some("3".into()), + ] + ); + } +} diff --git a/src/cache/mod.rs b/src/cache/mod.rs new file mode 100644 index 0000000..1a933a2 --- /dev/null +++ b/src/cache/mod.rs @@ -0,0 +1,294 @@ +//! Module implementing all things cache related + +use core::{fmt::Debug, ops::Range}; + +use embedded_storage_async::nor_flash::NorFlash; + +use crate::{item::ItemHeader, PageState}; + +use self::{ + key_pointers::{KeyPointersCache, UncachedKeyPointers}, + page_pointers::{CachedPagePointers, UncachedPagePointers}, + page_states::{CachedPageStates, UncachedPageSates}, +}; + +pub(crate) mod key_pointers; +pub(crate) mod page_pointers; +pub(crate) mod page_states; +mod tests; + +pub(crate) use page_pointers::PagePointersCache; +pub(crate) use page_states::PageStatesCache; + +/// Trait implemented by all cache types +#[allow(private_bounds)] +pub trait CacheImpl: PrivateCacheImpl {} + +impl CacheImpl for &mut T {} + +impl CacheImpl + for Cache +{ +} + +pub(crate) trait PrivateCacheImpl { + type PSC: PageStatesCache; + type PPC: PagePointersCache; + type KEY: Eq; + type KPC: KeyPointersCache; + + fn inner(&mut self) -> &mut Cache; +} + +impl PrivateCacheImpl for &mut T { + type PSC = T::PSC; + type PPC = T::PPC; + type KEY = T::KEY; + type KPC = T::KPC; + + fn inner(&mut self) -> &mut Cache { + T::inner(self) + } +} + +impl PrivateCacheImpl + for Cache +{ + type PSC = PSC; + type PPC = PPC; + type KEY = KPC::Key; + type KPC = KPC; + + fn inner(&mut self) -> &mut Cache { + self + } +} + +/// A cache object implementing no cache. +/// +/// This type of cache doesn't have to be kept around and may be constructed on every api call. +/// You could simply pass `&mut NoCache::new()` every time. +#[derive(Debug)] +pub struct NoCache(Cache); + +impl NoCache { + /// Construct a new instance + pub const fn new() -> Self { + Self(Cache::new( + UncachedPageSates, + UncachedPagePointers, + UncachedKeyPointers, + )) + } +} + +impl PrivateCacheImpl for NoCache { + type PSC = UncachedPageSates; + type PPC = UncachedPagePointers; + type KEY = (); + type KPC = UncachedKeyPointers; + + fn inner(&mut self) -> &mut Cache { + &mut self.0 + } +} + +impl CacheImpl for NoCache {} + +/// A cache object that keeps track of the page states. +/// +/// This cache has to be kept around and passed to *every* api call to the same memory region until the cache gets discarded. +/// +/// Valid usecase: +/// `Create cache 1` -> `use 1` -> `use 1` -> `create cache 2` -> `use 2` -> `use 2` +/// +/// Invalid usecase: +/// `Create cache 1` -> `use 1` -> `create cache 2` -> `use 2` -> `❌ use 1 ❌` +/// +/// Make sure the page count is correct. If the number is lower than the actual amount, the code will panic at some point. +#[derive(Debug)] +pub struct PageStateCache( + Cache, UncachedPagePointers, UncachedKeyPointers>, +); + +impl PageStateCache { + /// Construct a new instance + pub const fn new() -> Self { + Self(Cache::new( + CachedPageStates::new(), + UncachedPagePointers, + UncachedKeyPointers, + )) + } +} + +impl PrivateCacheImpl for PageStateCache { + type PSC = CachedPageStates; + type PPC = UncachedPagePointers; + type KEY = (); + type KPC = UncachedKeyPointers; + + fn inner(&mut self) -> &mut Cache { + &mut self.0 + } +} + +impl CacheImpl for PageStateCache {} + +/// A cache object that keeps track of the page states and some pointers to the items in the page. +/// +/// This cache has to be kept around and passed to *every* api call to the same memory region until the cache gets discarded. +/// +/// Valid usecase: +/// `Create cache 1` -> `use 1` -> `use 1` -> `create cache 2` -> `use 2` -> `use 2` +/// +/// Invalid usecase: +/// `Create cache 1` -> `use 1` -> `create cache 2` -> `use 2` -> `❌ use 1 ❌` +/// +/// Make sure the page count is correct. If the number is lower than the actual amount, the code will panic at some point. +#[derive(Debug)] +pub struct PagePointerCache( + Cache, CachedPagePointers, UncachedKeyPointers>, +); + +impl PagePointerCache { + /// Construct a new instance + pub const fn new() -> Self { + Self(Cache::new( + CachedPageStates::new(), + CachedPagePointers::new(), + UncachedKeyPointers, + )) + } +} + +impl PrivateCacheImpl for PagePointerCache { + type PSC = CachedPageStates; + type PPC = CachedPagePointers; + type KEY = (); + type KPC = UncachedKeyPointers; + + fn inner(&mut self) -> &mut Cache { + &mut self.0 + } +} + +impl CacheImpl for PagePointerCache {} + +/// The cache struct that is behind it all. +/// +/// It manages the cache state in a generic way. For every field it can be chosen to have it be cached or not. +#[derive(Debug)] +pub(crate) struct Cache { + /// Managed from the library code. + /// + /// When true, the cache and/or flash has changed and things might not be fully + /// consistent if there's an early return due to error. + dirty: bool, + page_states: PSC, + page_pointers: PPC, + key_pointers: KPC, +} + +impl Cache { + pub(crate) const fn new(page_states: PSC, page_pointers: PPC, key_pointers: KPC) -> Self { + Self { + dirty: false, + page_states, + page_pointers, + key_pointers, + } + } + + /// True if the cache might be inconsistent + pub(crate) fn is_dirty(&self) -> bool { + self.dirty + } + + /// Mark the cache as potentially inconsistent with reality + pub(crate) fn mark_dirty(&mut self) { + self.dirty = true; + } + + /// Mark the cache as being consistent with reality + pub(crate) fn unmark_dirty(&mut self) { + self.dirty = false; + } + + /// Wipe the cache + pub(crate) fn invalidate_cache_state(&mut self) { + self.dirty = false; + self.page_states.invalidate_cache_state(); + self.page_pointers.invalidate_cache_state(); + self.key_pointers.invalidate_cache_state(); + } + + /// Get the cache state of the requested page + pub(crate) fn get_page_state(&self, page_index: usize) -> Option { + self.page_states.get_page_state(page_index) + } + + /// Let the cache know a page state changed + /// + /// The dirty flag should be true if the page state is actually going to be changed. + /// Keep it false if we're only discovering the state. + pub(crate) fn notice_page_state( + &mut self, + page_index: usize, + new_state: PageState, + dirty: bool, + ) { + if dirty { + self.mark_dirty(); + } + self.page_states.notice_page_state(page_index, new_state); + self.page_pointers.notice_page_state(page_index, new_state); + } + + /// Get the cached address of the first non-erased item in the requested page. + /// + /// This is purely for the items that get erased from start to end. + pub(crate) fn first_item_after_erased(&self, page_index: usize) -> Option { + self.page_pointers.first_item_after_erased(page_index) + } + + /// Get the cached address of the first free unwritten item in the requested page. + pub(crate) fn first_item_after_written(&self, page_index: usize) -> Option { + self.page_pointers.first_item_after_written(page_index) + } + + /// Let the cache know that an item has been written to flash + pub(crate) fn notice_item_written( + &mut self, + flash_range: Range, + item_address: u32, + item_header: &ItemHeader, + ) { + self.mark_dirty(); + self.page_pointers + .notice_item_written::(flash_range, item_address, item_header) + } + + /// Let the cache know that an item has been erased from flash + pub(crate) fn notice_item_erased( + &mut self, + flash_range: Range, + item_address: u32, + item_header: &ItemHeader, + ) { + self.mark_dirty(); + self.page_pointers + .notice_item_erased::(flash_range, item_address, item_header) + } + + pub(crate) fn key_location(&self, key: &KPC::Key) -> Option { + self.key_pointers.key_location(key) + } + + pub(crate) fn notice_key_location(&mut self, key: KPC::Key, item_address: u32) { + self.key_pointers.notice_key_location(key, item_address) + } + pub(crate) fn notice_key_erased(&mut self, key: &KPC::Key) { + self.key_pointers.notice_key_erased(key) + } +} diff --git a/src/cache/page_pointers.rs b/src/cache/page_pointers.rs new file mode 100644 index 0000000..ee1b6dc --- /dev/null +++ b/src/cache/page_pointers.rs @@ -0,0 +1,171 @@ +use core::{fmt::Debug, num::NonZeroU32, ops::Range}; + +use embedded_storage_async::nor_flash::NorFlash; + +use crate::{ + calculate_page_address, calculate_page_index, item::ItemHeader, NorFlashExt, PageState, +}; + +pub(crate) trait PagePointersCache: Debug { + fn first_item_after_erased(&self, page_index: usize) -> Option; + fn first_item_after_written(&self, page_index: usize) -> Option; + + fn notice_item_written( + &mut self, + flash_range: Range, + item_address: u32, + item_header: &ItemHeader, + ); + fn notice_item_erased( + &mut self, + flash_range: Range, + item_address: u32, + item_header: &ItemHeader, + ); + + fn notice_page_state(&mut self, page_index: usize, new_state: PageState); + fn invalidate_cache_state(&mut self); +} + +// Use NoneZeroU32 because we never store 0's in here (because of the first page marker) +// and so Option can make use of the niche so we save bytes +pub(crate) struct CachedPagePointers { + after_erased_pointers: [Option; PAGE_COUNT], + after_written_pointers: [Option; PAGE_COUNT], +} + +impl Debug for CachedPagePointers { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "{{ after_erased_pointers: [")?; + for (i, val) in self.after_erased_pointers.iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + + if let Some(val) = val { + write!(f, "{:?}", val.get())?; + } else { + write!(f, "?")?; + } + } + write!(f, "], after_written_pointers: [")?; + for (i, val) in self.after_written_pointers.iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + + if let Some(val) = val { + write!(f, "{:?}", val.get())?; + } else { + write!(f, "?")?; + } + } + write!(f, "] }}")?; + + Ok(()) + } +} + +impl CachedPagePointers { + pub const fn new() -> Self { + Self { + after_erased_pointers: [None; PAGE_COUNT], + after_written_pointers: [None; PAGE_COUNT], + } + } +} + +impl PagePointersCache for CachedPagePointers { + fn first_item_after_erased(&self, page_index: usize) -> Option { + self.after_erased_pointers[page_index].map(|val| val.get()) + } + + fn first_item_after_written(&self, page_index: usize) -> Option { + self.after_written_pointers[page_index].map(|val| val.get()) + } + + fn notice_item_written( + &mut self, + flash_range: Range, + item_address: u32, + item_header: &ItemHeader, + ) { + let page_index = calculate_page_index::(flash_range, item_address); + + let next_item_address = item_header.next_item_address::(item_address); + + // We only care about the furthest written item, so discard if this is an earlier item + if let Some(first_item_after_written) = self.first_item_after_written(page_index) { + if next_item_address <= first_item_after_written { + return; + } + } + + self.after_written_pointers[page_index] = NonZeroU32::new(next_item_address); + } + + fn notice_item_erased( + &mut self, + flash_range: Range, + item_address: u32, + item_header: &ItemHeader, + ) { + let page_index = calculate_page_index::(flash_range.clone(), item_address); + + // Either the item we point to or the first item on the page + let next_unerased_item = self.first_item_after_erased(page_index).unwrap_or_else(|| { + calculate_page_address::(flash_range, page_index) + S::WORD_SIZE as u32 + }); + + if item_address == next_unerased_item { + self.after_erased_pointers[page_index] = + NonZeroU32::new(item_header.next_item_address::(item_address)); + } + } + + fn notice_page_state(&mut self, page_index: usize, new_state: PageState) { + if new_state.is_open() { + // This page was erased + self.after_erased_pointers[page_index] = None; + self.after_written_pointers[page_index] = None; + } + } + + fn invalidate_cache_state(&mut self) { + self.after_erased_pointers = [None; PAGE_COUNT]; + self.after_written_pointers = [None; PAGE_COUNT]; + } +} + +#[derive(Debug, Default)] +pub(crate) struct UncachedPagePointers; + +impl PagePointersCache for UncachedPagePointers { + fn first_item_after_erased(&self, _page_index: usize) -> Option { + None + } + + fn first_item_after_written(&self, _page_index: usize) -> Option { + None + } + + fn notice_item_written( + &mut self, + _flash_range: Range, + _item_address: u32, + _item_header: &ItemHeader, + ) { + } + + fn notice_item_erased( + &mut self, + _flash_range: Range, + _item_address: u32, + _item_header: &ItemHeader, + ) { + } + + fn notice_page_state(&mut self, _page_index: usize, _new_state: PageState) {} + + fn invalidate_cache_state(&mut self) {} +} diff --git a/src/cache/page_states.rs b/src/cache/page_states.rs new file mode 100644 index 0000000..64235bc --- /dev/null +++ b/src/cache/page_states.rs @@ -0,0 +1,68 @@ +use core::fmt::Debug; + +use crate::PageState; + +pub(crate) trait PageStatesCache: Debug { + fn get_page_state(&self, page_index: usize) -> Option; + fn notice_page_state(&mut self, page_index: usize, new_state: PageState); + fn invalidate_cache_state(&mut self); +} + +pub(crate) struct CachedPageStates { + pages: [Option; PAGE_COUNT], +} + +impl Debug for CachedPageStates { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "[")?; + for (i, val) in self.pages.iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + + if let Some(val) = val { + write!(f, "{val:?}")?; + } else { + write!(f, "?")?; + } + } + write!(f, "]")?; + + Ok(()) + } +} + +impl CachedPageStates { + pub const fn new() -> Self { + Self { + pages: [None; PAGE_COUNT], + } + } +} + +impl PageStatesCache for CachedPageStates { + fn get_page_state(&self, page_index: usize) -> Option { + self.pages[page_index] + } + + fn notice_page_state(&mut self, page_index: usize, new_state: PageState) { + self.pages[page_index] = Some(new_state); + } + + fn invalidate_cache_state(&mut self) { + *self = Self::new(); + } +} + +#[derive(Debug, Default)] +pub(crate) struct UncachedPageSates; + +impl PageStatesCache for UncachedPageSates { + fn get_page_state(&self, _page_index: usize) -> Option { + None + } + + fn notice_page_state(&mut self, _page_index: usize, _new_state: PageState) {} + + fn invalidate_cache_state(&mut self) {} +} diff --git a/src/cache/tests.rs b/src/cache/tests.rs new file mode 100644 index 0000000..0af4762 --- /dev/null +++ b/src/cache/tests.rs @@ -0,0 +1,278 @@ +#[cfg(test)] +mod queue_tests { + use core::ops::Range; + + use crate::{ + cache::{CacheImpl, NoCache, PagePointerCache, PageStateCache}, + mock_flash::{self, FlashStatsResult, WriteCountCheck}, + queue::{peek, pop, push}, + AlignedBuf, + }; + + use futures_test::test; + + const NUM_PAGES: usize = 4; + const LOOP_COUNT: usize = 2000; + + #[test] + async fn no_cache() { + assert_eq!( + run_test(&mut NoCache::new()).await, + FlashStatsResult { + erases: 146, + reads: 594934, + writes: 6299, + bytes_read: 2766058, + bytes_written: 53299 + } + ); + } + + #[test] + async fn page_state_cache() { + assert_eq!( + run_test(&mut PageStateCache::::new()).await, + FlashStatsResult { + erases: 146, + reads: 308740, + writes: 6299, + bytes_read: 2479864, + bytes_written: 53299 + } + ); + } + + #[test] + async fn page_pointer_cache() { + assert_eq!( + run_test(&mut PagePointerCache::::new()).await, + FlashStatsResult { + erases: 146, + reads: 211172, + writes: 6299, + bytes_read: 1699320, + bytes_written: 53299 + } + ); + } + + async fn run_test(mut cache: impl CacheImpl) -> FlashStatsResult { + let mut flash = + mock_flash::MockFlashBase::::new(WriteCountCheck::Twice, None, true); + const FLASH_RANGE: Range = 0x00..0x400; + let mut data_buffer = AlignedBuf([0; 1024]); + + let start_snapshot = flash.stats_snapshot(); + + for i in 0..LOOP_COUNT { + println!("{i}"); + let data = vec![i as u8; i % 20 + 1]; + + println!("PUSH"); + push(&mut flash, FLASH_RANGE, &mut cache, &data, true) + .await + .unwrap(); + assert_eq!( + &peek(&mut flash, FLASH_RANGE, &mut cache, &mut data_buffer) + .await + .unwrap() + .unwrap()[..], + &data, + "At {i}" + ); + println!("POP"); + assert_eq!( + &pop(&mut flash, FLASH_RANGE, &mut cache, &mut data_buffer) + .await + .unwrap() + .unwrap()[..], + &data, + "At {i}" + ); + println!("PEEK"); + assert_eq!( + peek(&mut flash, FLASH_RANGE, &mut cache, &mut data_buffer) + .await + .unwrap(), + None, + "At {i}" + ); + println!("DONE"); + } + + start_snapshot.compare_to(flash.stats_snapshot()) + } +} + +#[cfg(test)] +mod map_tests { + use core::ops::Range; + + use crate::{ + cache::{CacheImpl, NoCache, PagePointerCache, PageStateCache}, + map::{fetch_item, store_item, StorageItem}, + mock_flash::{self, FlashStatsResult, WriteCountCheck}, + AlignedBuf, + }; + + use futures_test::test; + + const NUM_PAGES: usize = 4; + + #[test] + async fn no_cache() { + assert_eq!( + run_test(&mut NoCache::new()).await, + FlashStatsResult { + erases: 198, + reads: 224161, + writes: 5201, + bytes_read: 1770974, + bytes_written: 50401 + } + ); + } + + #[test] + async fn page_state_cache() { + assert_eq!( + run_test(&mut PageStateCache::::new()).await, + FlashStatsResult { + erases: 198, + reads: 171543, + writes: 5201, + bytes_read: 1718356, + bytes_written: 50401 + } + ); + } + + #[test] + async fn page_pointer_cache() { + assert_eq!( + run_test(&mut PagePointerCache::::new()).await, + FlashStatsResult { + erases: 198, + reads: 153667, + writes: 5201, + bytes_read: 1575348, + bytes_written: 50401 + } + ); + } + + #[derive(Debug, PartialEq, Eq)] + struct MockStorageItem { + key: u8, + value: Vec, + } + + #[derive(Debug, PartialEq, Eq)] + enum MockStorageItemError { + BufferTooSmall, + InvalidKey, + BufferTooBig, + } + + impl StorageItem for MockStorageItem { + type Key = u8; + + type Error = MockStorageItemError; + + fn serialize_into(&self, buffer: &mut [u8]) -> Result { + if buffer.len() < 2 + self.value.len() { + return Err(MockStorageItemError::BufferTooSmall); + } + + if self.value.len() > 255 { + return Err(MockStorageItemError::BufferTooBig); + } + + // The serialized value must not be all 0xFF + if self.key == 0xFF { + return Err(MockStorageItemError::InvalidKey); + } + + buffer[0] = self.key; + buffer[1] = self.value.len() as u8; + buffer[2..][..self.value.len()].copy_from_slice(&self.value); + + Ok(2 + self.value.len()) + } + + fn deserialize_from(buffer: &[u8]) -> Result + where + Self: Sized, + { + if buffer.len() < 2 { + return Err(MockStorageItemError::BufferTooSmall); + } + + if buffer[0] == 0xFF { + return Err(MockStorageItemError::InvalidKey); + } + + let len = buffer[1]; + + if buffer.len() < 2 + len as usize { + return Err(MockStorageItemError::BufferTooSmall); + } + + Ok(Self { + key: buffer[0], + value: buffer[2..][..len as usize].to_vec(), + }) + } + + fn key(&self) -> Self::Key { + self.key + } + } + + async fn run_test(mut cache: impl CacheImpl) -> FlashStatsResult { + let mut cache = cache.inner(); + + let mut flash = + mock_flash::MockFlashBase::::new(WriteCountCheck::Twice, None, true); + const FLASH_RANGE: Range = 0x00..0x400; + let mut data_buffer = AlignedBuf([0; 128]); + + const LENGHT_PER_KEY: [usize; 24] = [ + 11, 13, 6, 13, 13, 10, 2, 3, 5, 36, 1, 65, 4, 6, 1, 15, 10, 7, 3, 15, 9, 3, 4, 5, + ]; + + let start_snapshot = flash.stats_snapshot(); + + for _ in 0..100 { + for i in 0..24 { + let item = MockStorageItem { + key: i as u8, + value: vec![i as u8; LENGHT_PER_KEY[i]], + }; + + store_item::<_, _>(&mut flash, FLASH_RANGE, &mut cache, &mut data_buffer, &item) + .await + .unwrap(); + } + + for i in 0..24 { + let item = fetch_item::( + &mut flash, + FLASH_RANGE, + &mut cache, + &mut data_buffer, + i as u8, + ) + .await + .unwrap() + .unwrap(); + + println!("Fetched {item:?}"); + + assert_eq!(item.value, vec![i as u8; LENGHT_PER_KEY[i]]); + } + } + + start_snapshot.compare_to(flash.stats_snapshot()) + } +} diff --git a/src/item.rs b/src/item.rs index 2a8410f..72bfece 100644 --- a/src/item.rs +++ b/src/item.rs @@ -27,7 +27,7 @@ use core::ops::Range; use embedded_storage_async::nor_flash::{MultiwriteNorFlash, NorFlash}; use crate::{ - cache::{Cache, PagePointersCache, PageStatesCache}, + cache::{key_pointers::KeyPointersCache, Cache, PagePointersCache, PageStatesCache}, calculate_page_address, calculate_page_end_address, calculate_page_index, get_page_state, round_down_to_alignment, round_down_to_alignment_usize, round_up_to_alignment, round_up_to_alignment_usize, AlignedBuf, Error, NorFlashExt, PageState, MAX_WORD_SIZE, @@ -164,7 +164,7 @@ impl ItemHeader { mut self, flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache, address: u32, ) -> Result> { self.crc = None; @@ -210,7 +210,7 @@ impl<'d> Item<'d> { pub async fn write_new( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache, address: u32, data: &'d [u8], ) -> Result> { @@ -227,7 +227,7 @@ impl<'d> Item<'d> { async fn write_raw( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache, header: &ItemHeader, data: &[u8], address: u32, @@ -270,7 +270,7 @@ impl<'d> Item<'d> { &self, flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache, address: u32, ) -> Result<(), Error> { Self::write_raw( @@ -303,7 +303,7 @@ impl<'d> core::fmt::Debug for Item<'d> { pub async fn find_next_free_item_spot( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache, start_address: u32, end_address: u32, data_length: u32, @@ -440,7 +440,7 @@ fn crc32_with_initial(data: &[u8], initial: u32) -> u32 { pub async fn is_page_empty( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache, page_index: usize, page_state: Option, ) -> Result> { diff --git a/src/lib.rs b/src/lib.rs index 1a706d5..cf07d6f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,7 +7,7 @@ // - flash erase size is quite big, aka, this is a paged flash // - flash write size is quite small, so it writes words and not full pages -use cache::{Cache, PagePointersCache, PageStatesCache}; +use cache::{key_pointers::KeyPointersCache, Cache, PagePointersCache, PageStatesCache}; use core::{ fmt::Debug, ops::{Deref, DerefMut, Range}, @@ -85,7 +85,7 @@ async fn try_general_repair( async fn find_first_page( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache, starting_page_index: usize, page_state: PageState, ) -> Result, Error> { @@ -150,7 +150,7 @@ const MARKER: u8 = 0; async fn get_page_state( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache, page_index: usize, ) -> Result> { if let Some(cached_page_state) = cache.get_page_state(page_index) { @@ -218,7 +218,7 @@ async fn get_page_state( async fn open_page( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache, page_index: usize, ) -> Result<(), Error> { cache.notice_page_state(page_index, PageState::Open, true); @@ -242,7 +242,7 @@ async fn open_page( async fn close_page( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache, page_index: usize, ) -> Result<(), Error> { let current_state = @@ -275,7 +275,7 @@ async fn close_page( async fn partial_close_page( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache, page_index: usize, ) -> Result> { let current_state = get_page_state::(flash, flash_range.clone(), cache, page_index).await?; diff --git a/src/map.rs b/src/map.rs index a747389..bdda8fe 100644 --- a/src/map.rs +++ b/src/map.rs @@ -137,7 +137,7 @@ use super::*; pub async fn fetch_item( flash: &mut S, flash_range: Range, - mut cache: impl CacheImpl, + mut cache: impl CacheImpl, data_buffer: &mut [u8], search_key: I::Key, ) -> Result, MapError> { @@ -157,7 +157,7 @@ pub async fn fetch_item( async fn fetch_item_with_location( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache>, data_buffer: &mut [u8], search_key: I::Key, ) -> Result, MapError> { @@ -172,6 +172,8 @@ async fn fetch_item_with_location( let mut last_used_page = find_first_page(flash, flash_range.clone(), cache, 0, PageState::PartialOpen).await?; + cache.key_location(&search_key); + if last_used_page.is_none() { // In the event that all pages are still open or the last used page was just closed, we search for the first open page. // If the page one before that is closed, then that's the last used page. @@ -529,7 +531,7 @@ impl PartialEq for MapError { async fn migrate_items( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache>, data_buffer: &mut [u8], source_page: usize, target_page: usize, diff --git a/src/queue.rs b/src/queue.rs index 552d87f..4d1b8e2 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -575,7 +575,7 @@ pub async fn find_max_fit( async fn find_youngest_page( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache, ) -> Result> { let last_used_page = find_first_page(flash, flash_range.clone(), cache, 0, PageState::PartialOpen).await?; @@ -601,7 +601,7 @@ async fn find_youngest_page( async fn find_oldest_page( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut Cache, ) -> Result> { let youngest_page = find_youngest_page(flash, flash_range.clone(), cache).await?; From bc378d2239160ada8e38bc8bab5dd9683ee6e8e7 Mon Sep 17 00:00:00 2001 From: Dion Dokter Date: Fri, 9 Feb 2024 16:34:43 +0100 Subject: [PATCH 2/5] Moar traits! Awesome results! Bad fuzz output :( --- CHANGELOG.md | 5 + README.md | 14 +- fuzz/fuzz_targets/map.rs | 6 +- src/cache/key_pointers.rs | 41 ++-- src/cache/mod.rs | 474 ++++++++++++++++++++++++++------------ src/cache/page_states.rs | 4 +- src/cache/tests.rs | 34 ++- src/item.rs | 20 +- src/lib.rs | 53 ++--- src/map.rs | 160 ++++++++----- src/mock_flash.rs | 4 +- src/queue.rs | 106 +++++---- 12 files changed, 581 insertions(+), 340 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed764fa..2c0cd81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ (DD-MM-YY) +## Unreleased + +- *Breaking:* Storage item key must now also be clone +- Added KeyPointerCache which significantly helps out the map + ## 0.8.1 07-02-24 - Added new PagePointerCache that caches more than the PageStateCache. See the readme for more details. diff --git a/README.md b/README.md index 5e7defc..7e4ad07 100644 --- a/README.md +++ b/README.md @@ -73,11 +73,15 @@ Instead, we can optionally store some state in ram. These numbers are taken from the test cases in the cache module: -| Name | RAM bytes | Map # flash reads | Map flash bytes read | Queue # flash reads | Queue flash bytes read | -| ---------------: | ------------: | ----------------: | -------------------: | ------------------: | ---------------------: | -| NoCache | 0 | 100% | 100% | 100% | 100% | -| PageStateCache | 1 * num pages | 77% | 97% | 51% | 90% | -| PagePointerCache | 9 * num pages | 69% | 89% | 35% | 61% | +| Name | RAM bytes | Map # flash reads | Map flash bytes read | Queue # flash reads | Queue flash bytes read | +| ----------------------: | -------------------------------------------: | ----------------: | -------------------: | ------------------: | ---------------------: | +| NoCache | 0 | 100% | 100% | 100% | 100% | +| PageStateCache | 1 * num pages | 77% | 97% | 51% | 90% | +| PagePointerCache | 9 * num pages | 69% | 89% | 35% | 61% | +| KeyPointerCache (half*) | 9 * num pages + (sizeof(KEY) + 4) * num keys | 58% | 74% | - | - | +| KeyPointerCache | 9 * num pages + (sizeof(KEY) + 4) * num keys | 6.5% | 8.5% | - | - | + +(* With only half the slots for the keys. Performance can be better or worse depending on the order of reading. This is on the bad side for this situation) #### Takeaways diff --git a/fuzz/fuzz_targets/map.rs b/fuzz/fuzz_targets/map.rs index e7c9386..79609bd 100644 --- a/fuzz/fuzz_targets/map.rs +++ b/fuzz/fuzz_targets/map.rs @@ -5,7 +5,7 @@ use libfuzzer_sys::arbitrary::Arbitrary; use libfuzzer_sys::fuzz_target; use rand::SeedableRng; use sequential_storage::{ - cache::{CacheImpl, NoCache, PagePointerCache, PageStateCache}, + cache::{KeyCacheImpl, KeyPointerCache, NoCache, PagePointerCache, PageStateCache}, map::{MapError, StorageItem}, mock_flash::{MockFlashBase, MockFlashError, WriteCountCheck}, }; @@ -19,6 +19,7 @@ fuzz_target!(|data: Input| match data.cache_type { CacheType::NoCache => fuzz(data, NoCache::new()), CacheType::PageStateCache => fuzz(data, PageStateCache::::new()), CacheType::PagePointerCache => fuzz(data, PagePointerCache::::new()), + CacheType::KeyPointerCache => fuzz(data, KeyPointerCache::::new()), }); #[derive(Arbitrary, Debug, Clone)] @@ -101,9 +102,10 @@ enum CacheType { NoCache, PageStateCache, PagePointerCache, + KeyPointerCache, } -fn fuzz(ops: Input, mut cache: impl CacheImpl) { +fn fuzz(ops: Input, mut cache: impl KeyCacheImpl) { let mut flash = MockFlashBase::::new( WriteCountCheck::OnceOnly, Some(ops.fuel as u32), diff --git a/src/cache/key_pointers.rs b/src/cache/key_pointers.rs index 0f5a0a5..2a76248 100644 --- a/src/cache/key_pointers.rs +++ b/src/cache/key_pointers.rs @@ -1,12 +1,10 @@ -use core::{fmt::Debug, marker::PhantomData, num::NonZeroU32}; +use core::{fmt::Debug, num::NonZeroU32}; -pub(crate) trait KeyPointersCache { - type Key: Eq; +pub(crate) trait KeyPointersCache { + fn key_location(&self, key: &KEY) -> Option; - fn key_location(&self, key: &Self::Key) -> Option; - - fn notice_key_location(&mut self, key: Self::Key, item_address: u32); - fn notice_key_erased(&mut self, key: &Self::Key); + 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); } @@ -47,15 +45,13 @@ impl CachedKeyPointers { } } -impl KeyPointersCache for CachedKeyPointers { - type Key = KEY; - - fn key_location(&self, key: &Self::Key) -> Option { +impl KeyPointersCache for CachedKeyPointers { + fn key_location(&self, key: &KEY) -> Option { self.key_index(key) .map(|index| self.key_pointers[index].as_ref().unwrap().1.get()) } - fn notice_key_location(&mut self, key: Self::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] = @@ -66,13 +62,10 @@ impl KeyPointersCache for CachedKeyPointers { - self.key_pointers[existing_index] = None; - move_to_back(&mut self.key_pointers, existing_index); - } - None => {} + fn notice_key_erased(&mut self, key: &KEY) { + if let Some(existing_index) = self.key_index(key) { + self.key_pointers[existing_index] = None; + move_to_back(&mut self.key_pointers, existing_index); } } @@ -84,16 +77,14 @@ impl KeyPointersCache for CachedKeyPointers Option { +impl KeyPointersCache for UncachedKeyPointers { + fn key_location(&self, _key: &KEY) -> Option { None } - fn notice_key_location(&mut self, _key: Self::Key, _item_address: u32) {} + fn notice_key_location(&mut self, _key: KEY, _item_address: u32) {} - fn notice_key_erased(&mut self, _key: &Self::Key) {} + fn notice_key_erased(&mut self, _key: &KEY) {} fn invalidate_cache_state(&mut self) {} } diff --git a/src/cache/mod.rs b/src/cache/mod.rs index 1a933a2..54d2d47 100644 --- a/src/cache/mod.rs +++ b/src/cache/mod.rs @@ -7,9 +7,9 @@ use embedded_storage_async::nor_flash::NorFlash; use crate::{item::ItemHeader, PageState}; use self::{ - key_pointers::{KeyPointersCache, UncachedKeyPointers}, + key_pointers::{CachedKeyPointers, KeyPointersCache, UncachedKeyPointers}, page_pointers::{CachedPagePointers, UncachedPagePointers}, - page_states::{CachedPageStates, UncachedPageSates}, + page_states::{CachedPageStates, UncachedPageStates}, }; pub(crate) mod key_pointers; @@ -23,44 +23,174 @@ pub(crate) use page_states::PageStatesCache; /// Trait implemented by all cache types #[allow(private_bounds)] pub trait CacheImpl: PrivateCacheImpl {} - impl CacheImpl for &mut T {} +/// Trait implemented by all cache types that know about keys +#[allow(private_bounds)] +pub trait KeyCacheImpl: CacheImpl + PrivateKeyCacheImpl {} +impl> KeyCacheImpl for &mut T {} -impl CacheImpl - for Cache -{ +pub(crate) trait Invalidate { + fn invalidate_cache_state(&mut self); +} + +impl Invalidate for &mut T { + fn invalidate_cache_state(&mut self) { + T::invalidate_cache_state(self) + } } -pub(crate) trait PrivateCacheImpl { +pub(crate) trait PrivateCacheImpl: Invalidate { type PSC: PageStatesCache; type PPC: PagePointersCache; - type KEY: Eq; - type KPC: KeyPointersCache; - fn inner(&mut self) -> &mut Cache; + fn dirt_tracker(&mut self, f: impl FnOnce(&mut DirtTracker) -> R) -> Option; + fn page_states(&mut self) -> &mut Self::PSC; + fn page_pointers(&mut self) -> &mut Self::PPC; + + /// True if the cache might be inconsistent + fn is_dirty(&mut self) -> bool { + self.dirt_tracker(|d| d.is_dirty()).unwrap_or_default() + } + + /// Mark the cache as potentially inconsistent with reality + fn mark_dirty(&mut self) { + self.dirt_tracker(|d| d.mark_dirty()); + } + + /// Mark the cache as being consistent with reality + fn unmark_dirty(&mut self) { + self.dirt_tracker(|d| d.unmark_dirty()); + } + + /// Get the cache state of the requested page + fn get_page_state(&mut self, page_index: usize) -> Option { + self.page_states().get_page_state(page_index) + } + + /// Let the cache know a page state changed + /// + /// The dirty flag should be true if the page state is actually going to be changed. + /// Keep it false if we're only discovering the state. + fn notice_page_state(&mut self, page_index: usize, new_state: PageState, dirty: bool) { + if dirty { + self.mark_dirty(); + } + self.page_states().notice_page_state(page_index, new_state); + self.page_pointers() + .notice_page_state(page_index, new_state); + } + + /// Get the cached address of the first non-erased item in the requested page. + /// + /// This is purely for the items that get erased from start to end. + fn first_item_after_erased(&mut self, page_index: usize) -> Option { + self.page_pointers().first_item_after_erased(page_index) + } + + /// Get the cached address of the first free unwritten item in the requested page. + fn first_item_after_written(&mut self, page_index: usize) -> Option { + self.page_pointers().first_item_after_written(page_index) + } + + /// Let the cache know that an item has been written to flash + fn notice_item_written( + &mut self, + flash_range: Range, + item_address: u32, + item_header: &ItemHeader, + ) { + self.mark_dirty(); + self.page_pointers() + .notice_item_written::(flash_range, item_address, item_header) + } + + /// Let the cache know that an item has been erased from flash + fn notice_item_erased( + &mut self, + flash_range: Range, + item_address: u32, + item_header: &ItemHeader, + ) { + self.mark_dirty(); + self.page_pointers() + .notice_item_erased::(flash_range, item_address, item_header) + } } impl PrivateCacheImpl for &mut T { type PSC = T::PSC; type PPC = T::PPC; - type KEY = T::KEY; + + fn dirt_tracker(&mut self, f: impl FnOnce(&mut DirtTracker) -> R) -> Option { + T::dirt_tracker(self, f) + } + + fn page_states(&mut self) -> &mut Self::PSC { + T::page_states(self) + } + + fn page_pointers(&mut self) -> &mut Self::PPC { + T::page_pointers(self) + } +} + +pub(crate) trait PrivateKeyCacheImpl: PrivateCacheImpl { + type KPC: KeyPointersCache; + + fn key_pointers(&mut self) -> &mut Self::KPC; + + fn key_location(&mut self, key: &KEY) -> Option { + self.key_pointers().key_location(key) + } + + fn notice_key_location(&mut self, key: KEY, item_address: u32, dirty: bool) { + if dirty { + self.mark_dirty(); + } + self.key_pointers().notice_key_location(key, item_address) + } + #[allow(unused)] + fn notice_key_erased(&mut self, key: &KEY) { + self.mark_dirty(); + self.key_pointers().notice_key_erased(key) + } +} + +impl> PrivateKeyCacheImpl for &mut T { type KPC = T::KPC; - fn inner(&mut self) -> &mut Cache { - T::inner(self) + fn key_pointers(&mut self) -> &mut Self::KPC { + T::key_pointers(self) } } -impl PrivateCacheImpl - for Cache -{ - type PSC = PSC; - type PPC = PPC; - type KEY = KPC::Key; - type KPC = KPC; +#[derive(Debug)] +pub(crate) struct DirtTracker { + /// Managed from the library code. + /// + /// When true, the cache and/or flash has changed and things might not be fully + /// consistent if there's an early return due to error. + dirty: bool, +} - fn inner(&mut self) -> &mut Cache { - self +impl DirtTracker { + pub const fn new() -> Self { + DirtTracker { dirty: false } + } + + /// True if the cache might be inconsistent + pub fn is_dirty(&self) -> bool { + self.dirty + } + + /// Mark the cache as potentially inconsistent with reality + pub fn mark_dirty(&mut self) { + self.dirty = true; + } + + /// Mark the cache as being consistent with reality + pub fn unmark_dirty(&mut self) { + self.dirty = false; } } @@ -69,31 +199,55 @@ impl Privat /// This type of cache doesn't have to be kept around and may be constructed on every api call. /// You could simply pass `&mut NoCache::new()` every time. #[derive(Debug)] -pub struct NoCache(Cache); +pub struct NoCache { + page_states: UncachedPageStates, + page_pointers: UncachedPagePointers, + key_pointers: UncachedKeyPointers, +} impl NoCache { /// Construct a new instance pub const fn new() -> Self { - Self(Cache::new( - UncachedPageSates, - UncachedPagePointers, - UncachedKeyPointers, - )) + Self { + page_states: UncachedPageStates, + page_pointers: UncachedPagePointers, + key_pointers: UncachedKeyPointers, + } } } impl PrivateCacheImpl for NoCache { - type PSC = UncachedPageSates; + type PSC = UncachedPageStates; type PPC = UncachedPagePointers; - type KEY = (); - type KPC = UncachedKeyPointers; - fn inner(&mut self) -> &mut Cache { - &mut self.0 + fn dirt_tracker(&mut self, _f: impl FnOnce(&mut DirtTracker) -> R) -> Option { + // We have no state, so no need to track dirtyness + None + } + + fn page_states(&mut self) -> &mut Self::PSC { + &mut self.page_states + } + + fn page_pointers(&mut self) -> &mut Self::PPC { + &mut self.page_pointers } } impl CacheImpl for NoCache {} +impl KeyCacheImpl for NoCache {} + +impl Invalidate for NoCache { + fn invalidate_cache_state(&mut self) {} +} + +impl PrivateKeyCacheImpl for NoCache { + type KPC = UncachedKeyPointers; + + fn key_pointers(&mut self) -> &mut Self::KPC { + &mut self.key_pointers + } +} /// A cache object that keeps track of the page states. /// @@ -107,33 +261,60 @@ impl CacheImpl for NoCache {} /// /// Make sure the page count is correct. If the number is lower than the actual amount, the code will panic at some point. #[derive(Debug)] -pub struct PageStateCache( - Cache, UncachedPagePointers, UncachedKeyPointers>, -); +pub struct PageStateCache { + dirt_tracker: DirtTracker, + page_states: CachedPageStates, + page_pointers: UncachedPagePointers, + key_pointers: UncachedKeyPointers, +} impl PageStateCache { /// Construct a new instance pub const fn new() -> Self { - Self(Cache::new( - CachedPageStates::new(), - UncachedPagePointers, - UncachedKeyPointers, - )) + Self { + dirt_tracker: DirtTracker::new(), + page_states: CachedPageStates::new(), + page_pointers: UncachedPagePointers, + key_pointers: UncachedKeyPointers, + } } } impl PrivateCacheImpl for PageStateCache { type PSC = CachedPageStates; type PPC = UncachedPagePointers; - type KEY = (); - type KPC = UncachedKeyPointers; - fn inner(&mut self) -> &mut Cache { - &mut self.0 + fn dirt_tracker(&mut self, f: impl FnOnce(&mut DirtTracker) -> R) -> Option { + Some(f(&mut self.dirt_tracker)) + } + + fn page_states(&mut self) -> &mut Self::PSC { + &mut self.page_states + } + + fn page_pointers(&mut self) -> &mut Self::PPC { + &mut self.page_pointers } } impl CacheImpl for PageStateCache {} +impl KeyCacheImpl for PageStateCache {} + +impl Invalidate for PageStateCache { + fn invalidate_cache_state(&mut self) { + self.dirt_tracker.unmark_dirty(); + self.page_states.invalidate_cache_state(); + self.page_pointers.invalidate_cache_state(); + } +} + +impl PrivateKeyCacheImpl for PageStateCache { + type KPC = UncachedKeyPointers; + + fn key_pointers(&mut self) -> &mut Self::KPC { + &mut self.key_pointers + } +} /// A cache object that keeps track of the page states and some pointers to the items in the page. /// @@ -147,148 +328,137 @@ impl CacheImpl for PageStateCache {} /// /// Make sure the page count is correct. If the number is lower than the actual amount, the code will panic at some point. #[derive(Debug)] -pub struct PagePointerCache( - Cache, CachedPagePointers, UncachedKeyPointers>, -); +pub struct PagePointerCache { + dirt_tracker: DirtTracker, + page_states: CachedPageStates, + page_pointers: CachedPagePointers, + key_pointers: UncachedKeyPointers, +} impl PagePointerCache { /// Construct a new instance pub const fn new() -> Self { - Self(Cache::new( - CachedPageStates::new(), - CachedPagePointers::new(), - UncachedKeyPointers, - )) + Self { + dirt_tracker: DirtTracker::new(), + page_states: CachedPageStates::new(), + page_pointers: CachedPagePointers::new(), + key_pointers: UncachedKeyPointers, + } } } impl PrivateCacheImpl for PagePointerCache { type PSC = CachedPageStates; type PPC = CachedPagePointers; - type KEY = (); - type KPC = UncachedKeyPointers; - fn inner(&mut self) -> &mut Cache { - &mut self.0 + fn dirt_tracker(&mut self, f: impl FnOnce(&mut DirtTracker) -> R) -> Option { + Some(f(&mut self.dirt_tracker)) } -} -impl CacheImpl for PagePointerCache {} - -/// The cache struct that is behind it all. -/// -/// It manages the cache state in a generic way. For every field it can be chosen to have it be cached or not. -#[derive(Debug)] -pub(crate) struct Cache { - /// Managed from the library code. - /// - /// When true, the cache and/or flash has changed and things might not be fully - /// consistent if there's an early return due to error. - dirty: bool, - page_states: PSC, - page_pointers: PPC, - key_pointers: KPC, -} - -impl Cache { - pub(crate) const fn new(page_states: PSC, page_pointers: PPC, key_pointers: KPC) -> Self { - Self { - dirty: false, - page_states, - page_pointers, - key_pointers, - } - } - - /// True if the cache might be inconsistent - pub(crate) fn is_dirty(&self) -> bool { - self.dirty + fn page_states(&mut self) -> &mut Self::PSC { + &mut self.page_states } - /// Mark the cache as potentially inconsistent with reality - pub(crate) fn mark_dirty(&mut self) { - self.dirty = true; + fn page_pointers(&mut self) -> &mut Self::PPC { + &mut self.page_pointers } +} - /// Mark the cache as being consistent with reality - pub(crate) fn unmark_dirty(&mut self) { - self.dirty = false; - } +impl CacheImpl for PagePointerCache {} +impl KeyCacheImpl for PagePointerCache {} - /// Wipe the cache - pub(crate) fn invalidate_cache_state(&mut self) { - self.dirty = false; +impl Invalidate for PagePointerCache { + fn invalidate_cache_state(&mut self) { + self.dirt_tracker.unmark_dirty(); self.page_states.invalidate_cache_state(); self.page_pointers.invalidate_cache_state(); - self.key_pointers.invalidate_cache_state(); } +} - /// Get the cache state of the requested page - pub(crate) fn get_page_state(&self, page_index: usize) -> Option { - self.page_states.get_page_state(page_index) +impl PrivateKeyCacheImpl for PagePointerCache { + type KPC = UncachedKeyPointers; + + fn key_pointers(&mut self) -> &mut Self::KPC { + &mut self.key_pointers } +} - /// Let the cache know a page state changed - /// - /// The dirty flag should be true if the page state is actually going to be changed. - /// Keep it false if we're only discovering the state. - pub(crate) fn notice_page_state( - &mut self, - page_index: usize, - new_state: PageState, - dirty: bool, - ) { - if dirty { - self.mark_dirty(); +/// A cache object that keeps track of the page states and some pointers to the items in the page. +/// +/// This cache has to be kept around and passed to *every* api call to the same memory region until the cache gets discarded. +/// +/// Valid usecase: +/// `Create cache 1` -> `use 1` -> `use 1` -> `create cache 2` -> `use 2` -> `use 2` +/// +/// Invalid usecase: +/// `Create cache 1` -> `use 1` -> `create cache 2` -> `use 2` -> `❌ use 1 ❌` +/// +/// Make sure the page count is correct. If the number is lower than the actual amount, the code will panic at some point. +#[derive(Debug)] +pub struct KeyPointerCache { + dirt_tracker: DirtTracker, + page_states: CachedPageStates, + page_pointers: CachedPagePointers, + key_pointers: CachedKeyPointers, +} + +impl KeyPointerCache { + /// Construct a new instance + pub const fn new() -> Self { + Self { + dirt_tracker: DirtTracker::new(), + page_states: CachedPageStates::new(), + page_pointers: CachedPagePointers::new(), + key_pointers: CachedKeyPointers::new(), } - self.page_states.notice_page_state(page_index, new_state); - self.page_pointers.notice_page_state(page_index, new_state); } +} - /// Get the cached address of the first non-erased item in the requested page. - /// - /// This is purely for the items that get erased from start to end. - pub(crate) fn first_item_after_erased(&self, page_index: usize) -> Option { - self.page_pointers.first_item_after_erased(page_index) - } +impl PrivateCacheImpl + for KeyPointerCache +{ + type PSC = CachedPageStates; + type PPC = CachedPagePointers; - /// Get the cached address of the first free unwritten item in the requested page. - pub(crate) fn first_item_after_written(&self, page_index: usize) -> Option { - self.page_pointers.first_item_after_written(page_index) + fn dirt_tracker(&mut self, f: impl FnOnce(&mut DirtTracker) -> R) -> Option { + Some(f(&mut self.dirt_tracker)) } - /// Let the cache know that an item has been written to flash - pub(crate) fn notice_item_written( - &mut self, - flash_range: Range, - item_address: u32, - item_header: &ItemHeader, - ) { - self.mark_dirty(); - self.page_pointers - .notice_item_written::(flash_range, item_address, item_header) + fn page_states(&mut self) -> &mut Self::PSC { + &mut self.page_states } - /// Let the cache know that an item has been erased from flash - pub(crate) fn notice_item_erased( - &mut self, - flash_range: Range, - item_address: u32, - item_header: &ItemHeader, - ) { - self.mark_dirty(); - self.page_pointers - .notice_item_erased::(flash_range, item_address, item_header) + fn page_pointers(&mut self) -> &mut Self::PPC { + &mut self.page_pointers } +} - pub(crate) fn key_location(&self, key: &KPC::Key) -> Option { - self.key_pointers.key_location(key) - } +impl CacheImpl + for KeyPointerCache +{ +} +impl KeyCacheImpl + for KeyPointerCache +{ +} - pub(crate) fn notice_key_location(&mut self, key: KPC::Key, item_address: u32) { - self.key_pointers.notice_key_location(key, item_address) +impl Invalidate + for KeyPointerCache +{ + fn invalidate_cache_state(&mut self) { + self.dirt_tracker.unmark_dirty(); + self.page_states.invalidate_cache_state(); + self.page_pointers.invalidate_cache_state(); + self.key_pointers.invalidate_cache_state(); } - pub(crate) fn notice_key_erased(&mut self, key: &KPC::Key) { - self.key_pointers.notice_key_erased(key) +} + +impl PrivateKeyCacheImpl + for KeyPointerCache +{ + type KPC = CachedKeyPointers; + + fn key_pointers(&mut self) -> &mut Self::KPC { + &mut self.key_pointers } } diff --git a/src/cache/page_states.rs b/src/cache/page_states.rs index 64235bc..eaddddc 100644 --- a/src/cache/page_states.rs +++ b/src/cache/page_states.rs @@ -55,9 +55,9 @@ impl PageStatesCache for CachedPageStates { } #[derive(Debug, Default)] -pub(crate) struct UncachedPageSates; +pub(crate) struct UncachedPageStates; -impl PageStatesCache for UncachedPageSates { +impl PageStatesCache for UncachedPageStates { fn get_page_state(&self, _page_index: usize) -> Option { None } diff --git a/src/cache/tests.rs b/src/cache/tests.rs index 0af4762..b455d24 100644 --- a/src/cache/tests.rs +++ b/src/cache/tests.rs @@ -109,7 +109,7 @@ mod map_tests { use core::ops::Range; use crate::{ - cache::{CacheImpl, NoCache, PagePointerCache, PageStateCache}, + cache::{KeyCacheImpl, KeyPointerCache, NoCache, PagePointerCache, PageStateCache}, map::{fetch_item, store_item, StorageItem}, mock_flash::{self, FlashStatsResult, WriteCountCheck}, AlignedBuf, @@ -161,6 +161,34 @@ mod map_tests { ); } + #[test] + async fn key_pointer_cache_half() { + assert_eq!( + run_test(&mut KeyPointerCache::::new()).await, + FlashStatsResult { + erases: 198, + reads: 130523, + writes: 5201, + bytes_read: 1318479, + bytes_written: 50401 + } + ); + } + + #[test] + async fn key_pointer_cache_full() { + assert_eq!( + run_test(&mut KeyPointerCache::::new()).await, + FlashStatsResult { + erases: 198, + reads: 14506, + writes: 5201, + bytes_read: 150566, + bytes_written: 50401 + } + ); + } + #[derive(Debug, PartialEq, Eq)] struct MockStorageItem { key: u8, @@ -229,9 +257,7 @@ mod map_tests { } } - async fn run_test(mut cache: impl CacheImpl) -> FlashStatsResult { - let mut cache = cache.inner(); - + async fn run_test(mut cache: impl KeyCacheImpl) -> FlashStatsResult { let mut flash = mock_flash::MockFlashBase::::new(WriteCountCheck::Twice, None, true); const FLASH_RANGE: Range = 0x00..0x400; diff --git a/src/item.rs b/src/item.rs index 72bfece..6b45374 100644 --- a/src/item.rs +++ b/src/item.rs @@ -27,10 +27,10 @@ use core::ops::Range; use embedded_storage_async::nor_flash::{MultiwriteNorFlash, NorFlash}; use crate::{ - cache::{key_pointers::KeyPointersCache, Cache, PagePointersCache, PageStatesCache}, - calculate_page_address, calculate_page_end_address, calculate_page_index, get_page_state, - round_down_to_alignment, round_down_to_alignment_usize, round_up_to_alignment, - round_up_to_alignment_usize, AlignedBuf, Error, NorFlashExt, PageState, MAX_WORD_SIZE, + cache::PrivateCacheImpl, calculate_page_address, calculate_page_end_address, + calculate_page_index, get_page_state, round_down_to_alignment, round_down_to_alignment_usize, + round_up_to_alignment, round_up_to_alignment_usize, AlignedBuf, Error, NorFlashExt, PageState, + MAX_WORD_SIZE, }; #[derive(Debug)] @@ -164,7 +164,7 @@ impl ItemHeader { mut self, flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut impl PrivateCacheImpl, address: u32, ) -> Result> { self.crc = None; @@ -210,7 +210,7 @@ impl<'d> Item<'d> { pub async fn write_new( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut impl PrivateCacheImpl, address: u32, data: &'d [u8], ) -> Result> { @@ -227,7 +227,7 @@ impl<'d> Item<'d> { async fn write_raw( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut impl PrivateCacheImpl, header: &ItemHeader, data: &[u8], address: u32, @@ -270,7 +270,7 @@ impl<'d> Item<'d> { &self, flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut impl PrivateCacheImpl, address: u32, ) -> Result<(), Error> { Self::write_raw( @@ -303,7 +303,7 @@ impl<'d> core::fmt::Debug for Item<'d> { pub async fn find_next_free_item_spot( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut impl PrivateCacheImpl, start_address: u32, end_address: u32, data_length: u32, @@ -440,7 +440,7 @@ fn crc32_with_initial(data: &[u8], initial: u32) -> u32 { pub async fn is_page_empty( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut impl PrivateCacheImpl, page_index: usize, page_state: Option, ) -> Result> { diff --git a/src/lib.rs b/src/lib.rs index cf07d6f..76102d8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,7 +7,7 @@ // - flash erase size is quite big, aka, this is a paged flash // - flash write size is quite small, so it writes words and not full pages -use cache::{key_pointers::KeyPointersCache, Cache, PagePointersCache, PageStatesCache}; +use cache::PrivateCacheImpl; use core::{ fmt::Debug, ops::{Deref, DerefMut, Range}, @@ -51,28 +51,14 @@ async fn try_general_repair( flash: &mut S, flash_range: Range, ) -> Result<(), Error> { - use crate::cache::PrivateCacheImpl; - // Loop through the pages and get their state. If one returns the corrupted error, // the page is likely half-erased. Fix for that is to re-erase again to hopefully finish the job. for page_index in get_pages::(flash_range.clone(), 0) { if matches!( - get_page_state( - flash, - flash_range.clone(), - NoCache::new().inner(), - page_index - ) - .await, + get_page_state(flash, flash_range.clone(), &mut NoCache::new(), page_index).await, Err(Error::Corrupted { .. }) ) { - open_page( - flash, - flash_range.clone(), - NoCache::new().inner(), - page_index, - ) - .await?; + open_page(flash, flash_range.clone(), &mut NoCache::new(), page_index).await?; } } @@ -85,7 +71,7 @@ async fn try_general_repair( async fn find_first_page( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut impl PrivateCacheImpl, starting_page_index: usize, page_state: PageState, ) -> Result, Error> { @@ -150,7 +136,7 @@ const MARKER: u8 = 0; async fn get_page_state( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut impl PrivateCacheImpl, page_index: usize, ) -> Result> { if let Some(cached_page_state) = cache.get_page_state(page_index) { @@ -218,7 +204,7 @@ async fn get_page_state( async fn open_page( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut impl PrivateCacheImpl, page_index: usize, ) -> Result<(), Error> { cache.notice_page_state(page_index, PageState::Open, true); @@ -242,7 +228,7 @@ async fn open_page( async fn close_page( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut impl PrivateCacheImpl, page_index: usize, ) -> Result<(), Error> { let current_state = @@ -275,7 +261,7 @@ async fn close_page( async fn partial_close_page( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut impl PrivateCacheImpl, page_index: usize, ) -> Result> { let current_state = get_page_state::(flash, flash_range.clone(), cache, page_index).await?; @@ -434,7 +420,6 @@ impl NorFlashExt for S { #[cfg(test)] mod tests { use super::*; - use crate::cache::PrivateCacheImpl; use futures_test::test; type MockFlash = mock_flash::MockFlashBase<4, 4, 64>; @@ -481,7 +466,7 @@ mod tests { find_first_page( &mut flash, 0x000..0x400, - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), 0, PageState::Open ) @@ -493,7 +478,7 @@ mod tests { find_first_page( &mut flash, 0x000..0x400, - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), 0, PageState::PartialOpen ) @@ -505,7 +490,7 @@ mod tests { find_first_page( &mut flash, 0x000..0x400, - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), 1, PageState::PartialOpen ) @@ -517,7 +502,7 @@ mod tests { find_first_page( &mut flash, 0x000..0x400, - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), 2, PageState::PartialOpen ) @@ -529,7 +514,7 @@ mod tests { find_first_page( &mut flash, 0x000..0x400, - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), 3, PageState::Open ) @@ -541,7 +526,7 @@ mod tests { find_first_page( &mut flash, 0x000..0x200, - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), 0, PageState::PartialOpen ) @@ -554,7 +539,7 @@ mod tests { find_first_page( &mut flash, 0x000..0x400, - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), 0, PageState::Closed ) @@ -566,7 +551,7 @@ mod tests { find_first_page( &mut flash, 0x000..0x400, - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), 1, PageState::Closed ) @@ -578,7 +563,7 @@ mod tests { find_first_page( &mut flash, 0x000..0x400, - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), 2, PageState::Closed ) @@ -590,7 +575,7 @@ mod tests { find_first_page( &mut flash, 0x000..0x400, - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), 3, PageState::Closed ) @@ -602,7 +587,7 @@ mod tests { find_first_page( &mut flash, 0x200..0x400, - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), 0, PageState::Closed ) diff --git a/src/map.rs b/src/map.rs index bdda8fe..5eba9ac 100644 --- a/src/map.rs +++ b/src/map.rs @@ -115,12 +115,9 @@ //! # }); //! ``` -use crate::{ - cache::Cache, - item::{find_next_free_item_spot, Item, ItemHeader, ItemIter}, -}; +use crate::item::{find_next_free_item_spot, Item, ItemHeader, ItemIter}; -use self::cache::{CacheImpl, PrivateCacheImpl}; +use self::cache::{KeyCacheImpl, PrivateKeyCacheImpl}; use super::*; @@ -137,16 +134,16 @@ use super::*; pub async fn fetch_item( flash: &mut S, flash_range: Range, - mut cache: impl CacheImpl, + mut cache: impl KeyCacheImpl, data_buffer: &mut [u8], search_key: I::Key, ) -> Result, MapError> { - if cache.inner().is_dirty() { - cache.inner().invalidate_cache_state(); + if cache.is_dirty() { + cache.invalidate_cache_state(); } Ok( - fetch_item_with_location(flash, flash_range, cache.inner(), data_buffer, search_key) + fetch_item_with_location(flash, flash_range, &mut cache, data_buffer, search_key) .await? .map(|(item, _, _)| item), ) @@ -157,7 +154,7 @@ pub async fn fetch_item( async fn fetch_item_with_location( flash: &mut S, flash_range: Range, - cache: &mut Cache>, + cache: &mut impl PrivateKeyCacheImpl, data_buffer: &mut [u8], search_key: I::Key, ) -> Result, MapError> { @@ -168,12 +165,53 @@ async fn fetch_item_with_location( assert!(S::ERASE_SIZE >= S::WORD_SIZE * 3); assert!(S::WORD_SIZE <= MAX_WORD_SIZE); + 'cache: { + if let Some(cached_location) = cache.key_location(&search_key) { + let page_index = calculate_page_index::(flash_range.clone(), cached_location); + let page_data_end_address = + calculate_page_end_address::(flash_range.clone(), page_index) + - S::WORD_SIZE as u32; + + let Some(header) = + ItemHeader::read_new(flash, cached_location, page_data_end_address).await? + else { + // The cache points to a non-existing item? + if cfg!(feature = "_test") { + panic!("Wrong cache value"); + } + cache.invalidate_cache_state(); + break 'cache; + }; + + let item = header + .read_item(flash, data_buffer, cached_location, page_data_end_address) + .await?; + + match item { + item::MaybeItem::Corrupted(_, _) | item::MaybeItem::Erased(_, _) => { + if cfg!(feature = "_test") { + panic!("Wrong cache value"); + } + + // The cache points to a corrupted or erased item? + cache.invalidate_cache_state(); + break 'cache; + } + item::MaybeItem::Present(item) => { + return Ok(Some(( + I::deserialize_from(item.data()).map_err(MapError::Item)?, + cached_location, + item.header, + ))); + } + } + } + } + // We need to find the page we were last using. This should be the only partial open page. let mut last_used_page = find_first_page(flash, flash_range.clone(), cache, 0, PageState::PartialOpen).await?; - cache.key_location(&search_key); - if last_used_page.is_none() { // In the event that all pages are still open or the last used page was just closed, we search for the first open page. // If the page one before that is closed, then that's the last used page. @@ -229,7 +267,9 @@ async fn fetch_item_with_location( } // We've found the item! We can stop searching - if newest_found_item.is_some() { + if let Some(newest_found_item) = newest_found_item.as_ref() { + cache.notice_key_location(search_key, newest_found_item.1, false); + break; } @@ -262,7 +302,7 @@ async fn fetch_item_with_location( pub async fn store_item( flash: &mut S, flash_range: Range, - mut cache: impl CacheImpl, + mut cache: impl KeyCacheImpl, data_buffer: &mut [u8], item: &I, ) -> Result<(), MapError> { @@ -274,8 +314,6 @@ pub async fn store_item( assert!(S::ERASE_SIZE >= S::WORD_SIZE * 3); assert!(S::WORD_SIZE <= MAX_WORD_SIZE); - let cache = cache.inner(); - if cache.is_dirty() { cache.invalidate_cache_state(); } @@ -289,14 +327,20 @@ pub async fn store_item( } // If there is a partial open page, we try to write in that first if there is enough space - let next_page_to_use = if let Some(partial_open_page) = - find_first_page(flash, flash_range.clone(), cache, 0, PageState::PartialOpen).await? + let next_page_to_use = if let Some(partial_open_page) = find_first_page( + flash, + flash_range.clone(), + &mut cache, + 0, + PageState::PartialOpen, + ) + .await? { // We found a partial open page, but at this point it's relatively cheap to do a consistency check if !get_page_state( flash, flash_range.clone(), - cache, + &mut cache, next_page::(flash_range.clone(), partial_open_page), ) .await? @@ -326,7 +370,7 @@ pub async fn store_item( let free_spot_address = find_next_free_item_spot( flash, flash_range.clone(), - cache, + &mut cache, page_data_start_address, page_data_end_address, item_data_length as u32, @@ -335,10 +379,11 @@ pub async fn store_item( match free_spot_address { Some(free_spot_address) => { + cache.notice_key_location(item.key(), free_spot_address, true); Item::write_new( flash, flash_range.clone(), - cache, + &mut cache, free_spot_address, &data_buffer[..item_data_length], ) @@ -349,7 +394,7 @@ pub async fn store_item( } None => { // The item doesn't fit here, so we need to close this page and move to the next - close_page(flash, flash_range.clone(), cache, partial_open_page).await?; + close_page(flash, flash_range.clone(), &mut cache, partial_open_page).await?; Some(next_page::(flash_range.clone(), partial_open_page)) } } @@ -365,7 +410,8 @@ pub async fn store_item( match next_page_to_use { Some(next_page_to_use) => { let next_page_state = - get_page_state(flash, flash_range.clone(), cache, next_page_to_use).await?; + get_page_state(flash, flash_range.clone(), &mut cache, next_page_to_use) + .await?; if !next_page_state.is_open() { // What was the previous buffer page was not open... @@ -378,17 +424,19 @@ pub async fn store_item( // Since we're gonna write data here, let's already partially close the page // This could be done after moving the data, but this is more robust in the // face of shutdowns and cancellations - partial_close_page(flash, flash_range.clone(), cache, next_page_to_use).await?; + partial_close_page(flash, flash_range.clone(), &mut cache, next_page_to_use) + .await?; let next_buffer_page = next_page::(flash_range.clone(), next_page_to_use); let next_buffer_page_state = - get_page_state(flash, flash_range.clone(), cache, next_buffer_page).await?; + get_page_state(flash, flash_range.clone(), &mut cache, next_buffer_page) + .await?; if !next_buffer_page_state.is_open() { migrate_items::( flash, flash_range.clone(), - cache, + &mut cache, data_buffer, next_buffer_page, next_page_to_use, @@ -398,23 +446,28 @@ pub async fn store_item( } None => { // There's no partial open page, so we just gotta turn the first open page into a partial open one - let first_open_page = - match find_first_page(flash, flash_range.clone(), cache, 0, PageState::Open) - .await? - { - Some(first_open_page) => first_open_page, - None => { - // Uh oh, no open pages. - // Something has gone wrong. - // We should never get here. - return Err(MapError::Corrupted { - #[cfg(feature = "_test")] - backtrace: std::backtrace::Backtrace::capture(), - }); - } - }; - - partial_close_page(flash, flash_range.clone(), cache, first_open_page).await?; + let first_open_page = match find_first_page( + flash, + flash_range.clone(), + &mut cache, + 0, + PageState::Open, + ) + .await? + { + Some(first_open_page) => first_open_page, + None => { + // Uh oh, no open pages. + // Something has gone wrong. + // We should never get here. + return Err(MapError::Corrupted { + #[cfg(feature = "_test")] + backtrace: std::backtrace::Backtrace::capture(), + }); + } + }; + + partial_close_page(flash, flash_range.clone(), &mut cache, first_open_page).await?; } } @@ -429,7 +482,7 @@ pub async fn store_item( /// Items must also fit within a page (together with the bits of overhead added in the storage process). pub trait StorageItem { /// The key type of the key-value pair - type Key: Eq; + type Key: Eq + Clone; /// The error type for serialization and deserialization type Error; @@ -531,7 +584,7 @@ impl PartialEq for MapError { async fn migrate_items( flash: &mut S, flash_range: Range, - cache: &mut Cache>, + cache: &mut impl PrivateKeyCacheImpl, data_buffer: &mut [u8], source_page: usize, target_page: usize, @@ -552,7 +605,7 @@ async fn migrate_items( // Search for the newest item with the key we found let Some((_, found_address, _)) = - fetch_item_with_location::(flash, flash_range.clone(), cache, data_buffer, key) + fetch_item_with_location::(flash, flash_range.clone(), cache, data_buffer, key.clone()) .await? else { // We couldn't even find our own item? @@ -569,6 +622,7 @@ async fn migrate_items( .read_item(flash, data_buffer, item_address, u32::MAX) .await? .unwrap()?; + cache.notice_key_location(key, item_address, true); item.write(flash, flash_range.clone(), cache, next_page_write_address) .await?; next_page_write_address = item.header.next_item_address::(next_page_write_address); @@ -593,11 +647,9 @@ async fn migrate_items( pub async fn try_repair( flash: &mut S, flash_range: Range, - mut cache: impl CacheImpl, + mut cache: impl KeyCacheImpl, data_buffer: &mut [u8], ) -> Result<(), MapError> { - let cache = cache.inner(); - cache.invalidate_cache_state(); #[allow(dropping_references)] drop(cache); @@ -608,7 +660,7 @@ pub async fn try_repair( if let Some(partial_open_page) = find_first_page( flash, flash_range.clone(), - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), 0, PageState::PartialOpen, ) @@ -618,7 +670,7 @@ pub async fn try_repair( if !get_page_state( flash, flash_range.clone(), - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), buffer_page, ) .await? @@ -629,7 +681,7 @@ pub async fn try_repair( open_page( flash, flash_range.clone(), - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), partial_open_page, ) .await?; @@ -638,7 +690,7 @@ pub async fn try_repair( partial_close_page( flash, flash_range.clone(), - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), partial_open_page, ) .await?; @@ -646,7 +698,7 @@ pub async fn try_repair( migrate_items::( flash, flash_range.clone(), - cache::NoCache::new().inner(), + &mut cache::NoCache::new(), data_buffer, buffer_page, partial_open_page, diff --git a/src/mock_flash.rs b/src/mock_flash.rs index d036548..7aefcc7 100644 --- a/src/mock_flash.rs +++ b/src/mock_flash.rs @@ -116,7 +116,7 @@ impl #[cfg(feature = "_test")] /// Print all items in flash to the returned string pub fn print_items(&mut self) -> String { - use crate::cache::{NoCache, PrivateCacheImpl}; + use crate::cache::NoCache; use crate::NorFlashExt; use futures::executor::block_on; use std::fmt::Write; @@ -135,7 +135,7 @@ impl match block_on(crate::get_page_state( self, Self::FULL_FLASH_RANGE, - NoCache::new().inner(), + &mut NoCache::new(), page_index )) { Ok(value) => format!("{value:?}"), diff --git a/src/queue.rs b/src/queue.rs index 4d1b8e2..8cd0ac6 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -55,12 +55,9 @@ //! # }); //! ``` -use crate::{ - cache::Cache, - item::{find_next_free_item_spot, is_page_empty, Item, ItemHeader, ItemHeaderIter}, -}; +use crate::item::{find_next_free_item_spot, is_page_empty, Item, ItemHeader, ItemHeaderIter}; -use self::cache::{CacheImpl, PageStatesCache}; +use self::cache::CacheImpl; use super::*; use embedded_storage_async::nor_flash::MultiwriteNorFlash; @@ -86,8 +83,6 @@ pub async fn push( assert!(S::ERASE_SIZE >= S::WORD_SIZE * 4); assert!(S::WORD_SIZE <= MAX_WORD_SIZE); - let cache = cache.inner(); - if cache.is_dirty() { cache.invalidate_cache_state(); } @@ -101,21 +96,21 @@ pub async fn push( return Err(Error::BufferTooBig); } - let current_page = find_youngest_page(flash, flash_range.clone(), cache).await?; + let current_page = find_youngest_page(flash, flash_range.clone(), &mut cache).await?; let page_data_start_address = calculate_page_address::(flash_range.clone(), current_page) + S::WORD_SIZE as u32; let page_data_end_address = calculate_page_end_address::(flash_range.clone(), current_page) - S::WORD_SIZE as u32; - partial_close_page(flash, flash_range.clone(), cache, current_page).await?; + partial_close_page(flash, flash_range.clone(), &mut cache, current_page).await?; // Find the last item on the page so we know where we need to write let mut next_address = find_next_free_item_spot( flash, flash_range.clone(), - cache, + &mut cache, page_data_start_address, page_data_end_address, data.len() as u32, @@ -125,10 +120,10 @@ pub async fn push( if next_address.is_none() { // No cap left on this page, move to the next page let next_page = next_page::(flash_range.clone(), current_page); - match get_page_state(flash, flash_range.clone(), cache, next_page).await? { + match get_page_state(flash, flash_range.clone(), &mut cache, next_page).await? { PageState::Open => { - close_page(flash, flash_range.clone(), cache, current_page).await?; - partial_close_page(flash, flash_range.clone(), cache, next_page).await?; + close_page(flash, flash_range.clone(), &mut cache, current_page).await?; + partial_close_page(flash, flash_range.clone(), &mut cache, next_page).await?; next_address = Some( calculate_page_address::(flash_range.clone(), next_page) + S::WORD_SIZE as u32, @@ -140,16 +135,22 @@ pub async fn push( + S::WORD_SIZE as u32; if !allow_overwrite_old_data - && !is_page_empty(flash, flash_range.clone(), cache, next_page, Some(state)) - .await? + && !is_page_empty( + flash, + flash_range.clone(), + &mut cache, + next_page, + Some(state), + ) + .await? { cache.unmark_dirty(); return Err(Error::FullStorage); } - open_page(flash, flash_range.clone(), cache, next_page).await?; - close_page(flash, flash_range.clone(), cache, current_page).await?; - partial_close_page(flash, flash_range.clone(), cache, next_page).await?; + open_page(flash, flash_range.clone(), &mut cache, next_page).await?; + close_page(flash, flash_range.clone(), &mut cache, current_page).await?; + partial_close_page(flash, flash_range.clone(), &mut cache, next_page).await?; next_address = Some(next_page_data_start_address); } PageState::PartialOpen => { @@ -165,7 +166,7 @@ pub async fn push( Item::write_new( flash, flash_range.clone(), - cache, + &mut cache, next_address.unwrap(), data, ) @@ -268,8 +269,8 @@ impl<'d, S: MultiwriteNorFlash, CI: CacheImpl> PopIterator<'d, S, CI> { &mut self, data_buffer: &'m mut [u8], ) -> Result, Error> { - if self.iter.cache.inner().is_dirty() { - self.iter.cache.inner().invalidate_cache_state(); + if self.iter.cache.is_dirty() { + self.iter.cache.invalidate_cache_state(); } let reset_point = self.iter.create_reset_point(); @@ -282,13 +283,13 @@ impl<'d, S: MultiwriteNorFlash, CI: CacheImpl> PopIterator<'d, S, CI> { .erase_data( self.iter.flash, self.iter.flash_range.clone(), - self.iter.cache.inner(), + &mut self.iter.cache, item_address, ) .await { Ok(_) => { - self.iter.cache.inner().unmark_dirty(); + self.iter.cache.unmark_dirty(); Ok(Some(ret)) } Err(e) => { @@ -297,7 +298,7 @@ impl<'d, S: MultiwriteNorFlash, CI: CacheImpl> PopIterator<'d, S, CI> { } } } else { - self.iter.cache.inner().unmark_dirty(); + self.iter.cache.unmark_dirty(); Ok(None) } } @@ -322,8 +323,8 @@ impl<'d, S: NorFlash, CI: CacheImpl> PeekIterator<'d, S, CI> { &mut self, data_buffer: &'m mut [u8], ) -> Result, Error> { - if self.iter.cache.inner().is_dirty() { - self.iter.cache.inner().invalidate_cache_state(); + if self.iter.cache.is_dirty() { + self.iter.cache.invalidate_cache_state(); } Ok(self.iter.next(data_buffer).await?.map(|(item, _)| { @@ -367,14 +368,14 @@ impl<'d, S: NorFlash, CI: CacheImpl> QueueIterator<'d, S, CI> { assert!(S::ERASE_SIZE >= S::WORD_SIZE * 4); assert!(S::WORD_SIZE <= MAX_WORD_SIZE); - if cache.inner().is_dirty() { - cache.inner().invalidate_cache_state(); + if cache.is_dirty() { + cache.invalidate_cache_state(); } - let oldest_page = find_oldest_page(flash, flash_range.clone(), cache.inner()).await?; + let oldest_page = find_oldest_page(flash, flash_range.clone(), &mut cache).await?; // We start at the start of the oldest page - let current_address = match cache.inner().first_item_after_erased(oldest_page) { + let current_address = match cache.first_item_after_erased(oldest_page) { Some(address) => address, None => { calculate_page_address::(flash_range.clone(), oldest_page) + S::WORD_SIZE as u32 @@ -395,8 +396,8 @@ impl<'d, S: NorFlash, CI: CacheImpl> QueueIterator<'d, S, CI> { ) -> Result, u32)>, Error> { let mut data_buffer = Some(data_buffer); - if self.cache.inner().is_dirty() { - self.cache.inner().invalidate_cache_state(); + if self.cache.is_dirty() { + self.cache.invalidate_cache_state(); } loop { @@ -407,7 +408,7 @@ impl<'d, S: NorFlash, CI: CacheImpl> QueueIterator<'d, S, CI> { if get_page_state( self.flash, self.flash_range.clone(), - self.cache.inner(), + &mut self.cache, next_page, ) .await? @@ -416,11 +417,11 @@ impl<'d, S: NorFlash, CI: CacheImpl> QueueIterator<'d, S, CI> { == find_oldest_page( self.flash, self.flash_range.clone(), - self.cache.inner(), + &mut self.cache, ) .await? { - self.cache.inner().unmark_dirty(); + self.cache.unmark_dirty(); return Ok(None); } @@ -477,7 +478,7 @@ impl<'d, S: NorFlash, CI: CacheImpl> QueueIterator<'d, S, CI> { CurrentAddress::Address(next_address) }; // Return the item we found - self.cache.inner().unmark_dirty(); + self.cache.unmark_dirty(); return Ok(Some((item, found_item_address))); } } @@ -515,19 +516,25 @@ pub async fn find_max_fit( assert!(S::ERASE_SIZE >= S::WORD_SIZE * 4); assert!(S::WORD_SIZE <= MAX_WORD_SIZE); - let cache = cache.inner(); - if cache.is_dirty() { cache.invalidate_cache_state(); } - let current_page = find_youngest_page(flash, flash_range.clone(), cache).await?; + let current_page = find_youngest_page(flash, flash_range.clone(), &mut cache).await?; // Check if we have space on the next page let next_page = next_page::(flash_range.clone(), current_page); - match get_page_state(flash, flash_range.clone(), cache, next_page).await? { + match get_page_state(flash, flash_range.clone(), &mut cache, next_page).await? { state @ PageState::Closed => { - if is_page_empty(flash, flash_range.clone(), cache, next_page, Some(state)).await? { + if is_page_empty( + flash, + flash_range.clone(), + &mut cache, + next_page, + Some(state), + ) + .await? + { cache.unmark_dirty(); return Ok(Some((S::ERASE_SIZE - (2 * S::WORD_SIZE)) as u32)); } @@ -575,7 +582,7 @@ pub async fn find_max_fit( async fn find_youngest_page( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut impl PrivateCacheImpl, ) -> Result> { let last_used_page = find_first_page(flash, flash_range.clone(), cache, 0, PageState::PartialOpen).await?; @@ -601,7 +608,7 @@ async fn find_youngest_page( async fn find_oldest_page( flash: &mut S, flash_range: Range, - cache: &mut Cache, + cache: &mut impl PrivateCacheImpl, ) -> Result> { let youngest_page = find_youngest_page(flash, flash_range.clone(), cache).await?; @@ -627,7 +634,7 @@ pub async fn try_repair( flash_range: Range, mut cache: impl CacheImpl, ) -> Result<(), Error> { - cache.inner().invalidate_cache_state(); + cache.invalidate_cache_state(); drop(cache); crate::try_general_repair(flash, flash_range.clone()).await?; @@ -636,7 +643,6 @@ pub async fn try_repair( #[cfg(test)] mod tests { - use crate::cache::PrivateCacheImpl; use crate::mock_flash::{FlashAverageStatsResult, FlashStatsResult, WriteCountCheck}; use super::*; @@ -1243,24 +1249,24 @@ mod tests { const FLASH_RANGE: Range = 0x000..0x1000; - close_page(&mut flash, FLASH_RANGE, cache::NoCache::new().inner(), 0) + close_page(&mut flash, FLASH_RANGE, &mut cache::NoCache::new(), 0) .await .unwrap(); - close_page(&mut flash, FLASH_RANGE, cache::NoCache::new().inner(), 1) + close_page(&mut flash, FLASH_RANGE, &mut cache::NoCache::new(), 1) .await .unwrap(); - partial_close_page(&mut flash, FLASH_RANGE, cache::NoCache::new().inner(), 2) + partial_close_page(&mut flash, FLASH_RANGE, &mut cache::NoCache::new(), 2) .await .unwrap(); assert_eq!( - find_youngest_page(&mut flash, FLASH_RANGE, cache::NoCache::new().inner()) + find_youngest_page(&mut flash, FLASH_RANGE, &mut cache::NoCache::new()) .await .unwrap(), 2 ); assert_eq!( - find_oldest_page(&mut flash, FLASH_RANGE, cache::NoCache::new().inner()) + find_oldest_page(&mut flash, FLASH_RANGE, &mut cache::NoCache::new()) .await .unwrap(), 0 From 0377271084039ae99978f5be14e220dd37924cc6 Mon Sep 17 00:00:00 2001 From: Dion Dokter Date: Sun, 11 Feb 2024 00:01:19 +0100 Subject: [PATCH 3/5] Fix migration cache issue --- src/map.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/map.rs b/src/map.rs index 5eba9ac..a404f07 100644 --- a/src/map.rs +++ b/src/map.rs @@ -177,7 +177,7 @@ async fn fetch_item_with_location( else { // The cache points to a non-existing item? if cfg!(feature = "_test") { - panic!("Wrong cache value"); + panic!("Wrong cache value. Addr: {cached_location}"); } cache.invalidate_cache_state(); break 'cache; @@ -190,7 +190,7 @@ async fn fetch_item_with_location( match item { item::MaybeItem::Corrupted(_, _) | item::MaybeItem::Erased(_, _) => { if cfg!(feature = "_test") { - panic!("Wrong cache value"); + panic!("Wrong cache value. Addr: {cached_location}"); } // The cache points to a corrupted or erased item? @@ -604,9 +604,14 @@ async fn migrate_items( let (item_header, data_buffer) = item.destruct(); // Search for the newest item with the key we found - let Some((_, found_address, _)) = - fetch_item_with_location::(flash, flash_range.clone(), cache, data_buffer, key.clone()) - .await? + let Some((_, found_address, _)) = fetch_item_with_location::( + flash, + flash_range.clone(), + cache, + data_buffer, + key.clone(), + ) + .await? else { // We couldn't even find our own item? return Err(MapError::Corrupted { @@ -622,7 +627,7 @@ async fn migrate_items( .read_item(flash, data_buffer, item_address, u32::MAX) .await? .unwrap()?; - cache.notice_key_location(key, item_address, true); + cache.notice_key_location(key, next_page_write_address, true); item.write(flash, flash_range.clone(), cache, next_page_write_address) .await?; next_page_write_address = item.header.next_item_address::(next_page_write_address); From 991b93dffa1b6f5cf4e4e87e41b64a3df02521fb Mon Sep 17 00:00:00 2001 From: Dion Dokter Date: Sun, 11 Feb 2024 01:08:45 +0100 Subject: [PATCH 4/5] Little bit more docs --- README.md | 19 ++++++++++--------- src/cache/mod.rs | 9 +++++++-- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 7e4ad07..d21cd3a 100644 --- a/README.md +++ b/README.md @@ -73,15 +73,12 @@ Instead, we can optionally store some state in ram. These numbers are taken from the test cases in the cache module: -| Name | RAM bytes | Map # flash reads | Map flash bytes read | Queue # flash reads | Queue flash bytes read | -| ----------------------: | -------------------------------------------: | ----------------: | -------------------: | ------------------: | ---------------------: | -| NoCache | 0 | 100% | 100% | 100% | 100% | -| PageStateCache | 1 * num pages | 77% | 97% | 51% | 90% | -| PagePointerCache | 9 * num pages | 69% | 89% | 35% | 61% | -| KeyPointerCache (half*) | 9 * num pages + (sizeof(KEY) + 4) * num keys | 58% | 74% | - | - | -| KeyPointerCache | 9 * num pages + (sizeof(KEY) + 4) * num keys | 6.5% | 8.5% | - | - | - -(* With only half the slots for the keys. Performance can be better or worse depending on the order of reading. This is on the bad side for this situation) +| Name | RAM bytes | Map # flash reads | Map flash bytes read | Queue # flash reads | Queue flash bytes read | +| ---------------: | -------------------------------------------: | ----------------: | -------------------: | ------------------: | ---------------------: | +| 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% | - | - | #### Takeaways @@ -91,6 +88,10 @@ These numbers are taken from the test cases in the cache module: - PagePointerCache - Very efficient for the queue - Minimum cache level that makes a dent in the map +- KeyPointerCache + - Awesome savings! + - Numbers are less good if there are more keys than the cache can store + - Same as PagePointerCache when used for queue ## Inner workings diff --git a/src/cache/mod.rs b/src/cache/mod.rs index 54d2d47..1612bb5 100644 --- a/src/cache/mod.rs +++ b/src/cache/mod.rs @@ -383,8 +383,9 @@ impl PrivateKeyCacheImpl for PagePointerC } } -/// A cache object that keeps track of the page states and some pointers to the items in the page. -/// +/// An object that caches the location of the newest item with a given key. +/// This cache also caches pages states and page pointers. +/// /// This cache has to be kept around and passed to *every* api call to the same memory region until the cache gets discarded. /// /// Valid usecase: @@ -394,6 +395,10 @@ impl PrivateKeyCacheImpl for PagePointerC /// `Create cache 1` -> `use 1` -> `create cache 2` -> `use 2` -> `❌ use 1 ❌` /// /// Make sure the page count is correct. If the number is lower than the actual amount, the code will panic at some point. +/// +/// The number of key slots can be lower than the total amount of possible keys used, but this will lower +/// 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 { dirt_tracker: DirtTracker, From e5aa3cd4f52d5ee9a1ca21000785e6e9eaa3baed Mon Sep 17 00:00:00 2001 From: Dion Dokter Date: Sun, 11 Feb 2024 01:11:14 +0100 Subject: [PATCH 5/5] fmt --- src/cache/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cache/mod.rs b/src/cache/mod.rs index 1612bb5..05acf43 100644 --- a/src/cache/mod.rs +++ b/src/cache/mod.rs @@ -385,7 +385,7 @@ impl PrivateKeyCacheImpl for PagePointerC /// An object that caches the location of the newest item with a given key. /// This cache also caches pages states and page pointers. -/// +/// /// This cache has to be kept around and passed to *every* api call to the same memory region until the cache gets discarded. /// /// Valid usecase: @@ -395,7 +395,7 @@ impl PrivateKeyCacheImpl for PagePointerC /// `Create cache 1` -> `use 1` -> `create cache 2` -> `use 2` -> `❌ use 1 ❌` /// /// Make sure the page count is correct. If the number is lower than the actual amount, the code will panic at some point. -/// +/// /// The number of key slots can be lower than the total amount of possible keys used, but this will lower /// 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.