From 02f5e7fa120fd6997513e0808c357010f47697f9 Mon Sep 17 00:00:00 2001 From: Dion Dokter Date: Tue, 13 Oct 2026 14:17:40 +0200 Subject: [PATCH 1/3] Fix #145 --- src/item.rs | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/src/item.rs b/src/item.rs index 06b4690..5fbac6a 100644 --- a/src/item.rs +++ b/src/item.rs @@ -610,15 +610,17 @@ impl ItemIter { } pub(crate) struct ItemHeaderIter { - current_address: u32, + next_address: u32, end_address: u32, + item_corrupted: bool, } impl ItemHeaderIter { pub(crate) fn new(start_address: u32, end_address: u32) -> Self { Self { - current_address: start_address, + next_address: start_address, end_address, + item_corrupted: false, } } @@ -640,22 +642,38 @@ impl ItemHeaderIter { callback: impl Fn(&ItemHeader, u32) -> bool, ) -> Result<(Option, u32), Error> { loop { - match ItemHeader::read_new(flash, self.current_address, self.end_address).await { + let read_result = + ItemHeader::read_new(flash, self.next_address, self.end_address).await; + + let last_item_corrupted = core::mem::replace( + &mut self.item_corrupted, + matches!(read_result, Err(Error::Corrupted { .. })), + ); + + match read_result { Ok(Some(header)) => { - let next_address = header.next_item_address::(self.current_address); - if callback(&header, self.current_address) { - self.current_address = next_address; + let next_address = header.next_item_address::(self.next_address); + if callback(&header, self.next_address) { + self.next_address = next_address; } else { - let current_address = self.current_address; - self.current_address = next_address; + let current_address = self.next_address; + self.next_address = next_address; return Ok((Some(header), current_address)); } } Ok(None) => { - return Ok((None, self.current_address)); + return Ok((None, self.next_address)); } Err(Error::Corrupted { .. }) => { - self.current_address += S::WORD_SIZE as u32; + self.next_address += if last_item_corrupted { + // We skip a full header so we can't start writing into what could be a possible header. + // If we did write into a corrupted header, it might suddenly (wrongly) be deemed valid again. + round_up_to_alignment::(ItemHeader::LENGTH as u32) + } else { + // We've already skipped a full header, so now we have no information anymore except that + // a header must be word aligned, so look at every word + S::WORD_SIZE as u32 + }; } Err(e) => return Err(e), } From e38fcc4f8bceae8e0cf10634c5a5972885c6d581 Mon Sep 17 00:00:00 2001 From: Dion Dokter Date: Tue, 13 Oct 2026 15:14:57 +0200 Subject: [PATCH 2/3] Fix #138 --- README.md | 10 ++++----- src/cache/page_pointers.rs | 2 +- src/cache/tests.rs | 24 ++++++++++---------- src/item.rs | 6 ++--- src/lib.rs | 45 ++++++++++---------------------------- src/map.rs | 34 +++++++++++++--------------- src/mock_flash.rs | 5 ++--- src/queue/mod.rs | 33 ++++++++++++++-------------- 8 files changed, 66 insertions(+), 93 deletions(-) diff --git a/README.md b/README.md index 81d68a9..f588407 100644 --- a/README.md +++ b/README.md @@ -90,11 +90,11 @@ 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 | | --------------------------: | ---------------------------: | -------------------: | -------------------: | ------------------: | ---------------------: | | All `Uncached` | 0 | 100% | 100% | 100% | 100% | -| Only `CalculatedPageStates` | 24 | 78% | 97% | 44% | 85% | -| Only `ArrayPageStates` | 1 * num pages | 77% | 97% | 41% | 85% | -| Only `ArrayPagePointers` | 8 * num pages | 92% | 92% | 65% | 28% | -| Only `ArrayKeyPointers` | (sizeof(KEY) + 4) * num keys | half: 82%, full: 22% | half: 80%, full: 17% | - | - | -| All array caches together | | 6% | 8% | 6% | 14% | +| Only `CalculatedPageStates` | 24 | 87% | 97% | 58% | 85% | +| Only `ArrayPageStates` | 1 * num pages | 87% | 97% | 58% | 85% | +| Only `ArrayPagePointers` | 8 * num pages | 91% | 92% | 50% | 28% | +| Only `ArrayKeyPointers` | (sizeof(KEY) + 4) * num keys | half: 82%, full: 20% | half: 80%, full: 17% | - | - | +| All array caches together | | 7% | 8% | 9% | 14% | #### Takeaways diff --git a/src/cache/page_pointers.rs b/src/cache/page_pointers.rs index fdf23fd..248ba98 100644 --- a/src/cache/page_pointers.rs +++ b/src/cache/page_pointers.rs @@ -247,7 +247,7 @@ impl SealedPagePointersCache for TrackedPagePointers<'_> { // 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 + calculate_page_address::(flash_range, page_index) + S::WORD_SIZE as u32 * 2 }); if item_address == next_unerased_item { diff --git a/src/cache/tests.rs b/src/cache/tests.rs index 2947188..25d934f 100644 --- a/src/cache/tests.rs +++ b/src/cache/tests.rs @@ -22,7 +22,7 @@ mod queue_tests { run_test(Cache::new_uncached()).await, FlashStatsResult { erases: 149, - reads: 165009, + reads: 116519, writes: 6299, bytes_read: 651212, bytes_written: 53299 @@ -41,7 +41,7 @@ mod queue_tests { .await, FlashStatsResult { erases: 149, - reads: 68111, + reads: 68070, writes: 6299, bytes_read: 554314, bytes_written: 53299 @@ -60,7 +60,7 @@ mod queue_tests { .await, FlashStatsResult { erases: 149, - reads: 68037, + reads: 68033, writes: 6299, bytes_read: 554240, bytes_written: 53299 @@ -79,7 +79,7 @@ mod queue_tests { .await, FlashStatsResult { erases: 149, - reads: 106931, + reads: 58441, writes: 6299, bytes_read: 186588, bytes_written: 53299 @@ -98,7 +98,7 @@ mod queue_tests { .await, FlashStatsResult { erases: 149, - reads: 9959, + reads: 9955, writes: 6299, bytes_read: 89616, bytes_written: 53299 @@ -170,7 +170,7 @@ mod map_tests { run_test(Cache::new_uncached()).await, FlashStatsResult { erases: 198, - reads: 233786, + reads: 207470, writes: 5201, bytes_read: 1837101, bytes_written: 50401 @@ -189,7 +189,7 @@ mod map_tests { .await, FlashStatsResult { erases: 198, - reads: 181262, + reads: 181208, writes: 5201, bytes_read: 1784577, bytes_written: 50401 @@ -208,7 +208,7 @@ mod map_tests { .await, FlashStatsResult { erases: 198, - reads: 181162, + reads: 181158, writes: 5201, bytes_read: 1784477, bytes_written: 50401 @@ -227,7 +227,7 @@ mod map_tests { .await, FlashStatsResult { erases: 198, - reads: 215897, + reads: 189581, writes: 5201, bytes_read: 1693989, bytes_written: 50401 @@ -246,7 +246,7 @@ mod map_tests { .await, FlashStatsResult { erases: 198, - reads: 191876, + reads: 170630, writes: 5201, bytes_read: 1484871, bytes_written: 50401 @@ -265,7 +265,7 @@ mod map_tests { .await, FlashStatsResult { erases: 198, - reads: 52403, + reads: 42397, writes: 5201, bytes_read: 313708, bytes_written: 50401 @@ -284,7 +284,7 @@ mod map_tests { .await, FlashStatsResult { erases: 198, - reads: 14510, + reads: 14506, writes: 5201, bytes_read: 150592, bytes_written: 50401 diff --git a/src/item.rs b/src/item.rs index 5fbac6a..0041c6e 100644 --- a/src/item.rs +++ b/src/item.rs @@ -556,10 +556,10 @@ impl, KEY> GenericStorage { page_index: usize, ) -> Result> { let page_data_start_address = - calculate_page_address::(self.flash_range.clone(), page_index) + S::WORD_SIZE as u32; + calculate_page_address::(self.flash_range.clone(), page_index) + + S::WORD_SIZE as u32 * 2; let page_data_end_address = - calculate_page_end_address::(self.flash_range.clone(), page_index) - - S::WORD_SIZE as u32; + calculate_page_end_address::(self.flash_range.clone(), page_index); Ok(ItemHeaderIter::new( self.cache diff --git a/src/lib.rs b/src/lib.rs index 7584519..903c431 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -201,37 +201,18 @@ impl, KEY> GenericStorage { /// So only half of the byte needs to be zero. const HALF_MARKER_BITS: u32 = 4; - let mut buffer = [0; MAX_WORD_SIZE]; + let mut buffer = [0; MAX_WORD_SIZE * 2]; self.flash - .read(page_address, &mut buffer[..S::READ_SIZE]) + .read(page_address, &mut buffer[..S::WORD_SIZE * 2]) .await .map_err(|e| Error::Storage { value: e, #[cfg(feature = "_test")] backtrace: std::backtrace::Backtrace::capture(), })?; - let start_marked = buffer[..S::READ_SIZE] - .iter() - .map(|marker_byte| marker_byte.count_zeros()) - .sum::() - >= HALF_MARKER_BITS; - self.flash - .read( - page_address + (S::ERASE_SIZE - S::READ_SIZE) as u32, - &mut buffer[..S::READ_SIZE], - ) - .await - .map_err(|e| Error::Storage { - value: e, - #[cfg(feature = "_test")] - backtrace: std::backtrace::Backtrace::capture(), - })?; - let end_marked = buffer[..S::READ_SIZE] - .iter() - .map(|marker_byte| marker_byte.count_zeros()) - .sum::() - >= HALF_MARKER_BITS; + let start_marked = buffer[0].count_zeros() >= HALF_MARKER_BITS; + let end_marked = buffer[S::WORD_SIZE].count_zeros() >= HALF_MARKER_BITS; let discovered_state = match (start_marked, end_marked) { (true, true) => PageState::Closed, @@ -282,12 +263,11 @@ impl, KEY> GenericStorage { .notice_page_state(page_index, PageState::Closed, true); let buffer = AlignedBuf([MARKER; MAX_WORD_SIZE]); - let page_end_address = - calculate_page_end_address::(self.flash_range.clone(), page_index) - - S::WORD_SIZE as u32; + let second_word_address = + calculate_page_address::(self.flash_range.clone(), page_index) + S::WORD_SIZE as u32; // Close the end marker self.flash - .write(page_end_address, &buffer[..S::WORD_SIZE]) + .write(second_word_address, &buffer[..S::WORD_SIZE]) .await .map_err(|e| Error::Storage { value: e, @@ -355,10 +335,9 @@ impl, KEY> GenericStorage { .unwrap(); let page_data_start = crate::calculate_page_address::(self.flash_range.clone(), page_index) - + S::WORD_SIZE as u32; + + S::WORD_SIZE as u32 * 2; let page_data_end = - crate::calculate_page_end_address::(self.flash_range.clone(), page_index) - - S::WORD_SIZE as u32; + crate::calculate_page_end_address::(self.flash_range.clone(), page_index); let mut it = crate::item::ItemHeaderIter::new(page_data_start, page_data_end); while let (Some(header), item_address) = @@ -465,7 +444,7 @@ const MARKER: u8 = 0; /// The state of a page #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "defmt", derive(defmt::Format))] -pub enum PageState { +pub(crate) enum PageState { /// This page was fully written and has now been sealed Closed, /// This page has been written to, but may have some space left over @@ -689,14 +668,14 @@ mod tests { write_aligned(&mut flash, 0x000, &[MARKER, 0, 0, 0]) .await .unwrap(); - write_aligned(&mut flash, 0x100 - 4, &[0, 0, 0, MARKER]) + write_aligned(&mut flash, 0x000 + 4, &[MARKER, 0, 0, 0]) .await .unwrap(); // Page 1 markers write_aligned(&mut flash, 0x100, &[MARKER, 0, 0, 0]) .await .unwrap(); - write_aligned(&mut flash, 0x200 - 4, &[0, 0, 0, MARKER]) + write_aligned(&mut flash, 0x100 + 4, &[MARKER, 0, 0, 0]) .await .unwrap(); // Page 2 markers diff --git a/src/map.rs b/src/map.rs index 88a4bda..04633f0 100644 --- a/src/map.rs +++ b/src/map.rs @@ -225,8 +225,7 @@ impl, K: Key> MapStorage { if let Some(cached_location) = self.inner.cache.key_location(search_key) { let page_index = calculate_page_index::(self.flash_range(), cached_location); let page_data_end_address = - calculate_page_end_address::(self.flash_range(), page_index) - - S::WORD_SIZE as u32; + calculate_page_end_address::(self.flash_range(), page_index); let Some(header) = ItemHeader::read_new( &mut self.inner.flash, @@ -321,10 +320,9 @@ impl, K: Key> MapStorage { loop { let page_data_start_address = calculate_page_address::(self.flash_range(), current_page_to_check) - + S::WORD_SIZE as u32; + + S::WORD_SIZE as u32 * 2; let page_data_end_address = - calculate_page_end_address::(self.flash_range(), current_page_to_check) - - S::WORD_SIZE as u32; + calculate_page_end_address::(self.flash_range(), current_page_to_check); let mut it = ItemIter::new(page_data_start_address, page_data_end_address); while let Some((item, address)) = it.next(&mut self.inner.flash, data_buffer).await? { @@ -472,10 +470,9 @@ impl, K: Key> MapStorage { let page_data_start_address = calculate_page_address::(self.flash_range(), partial_open_page) - + S::WORD_SIZE as u32; + + S::WORD_SIZE as u32 * 2; let page_data_end_address = - calculate_page_end_address::(self.flash_range(), partial_open_page) - - S::WORD_SIZE as u32; + calculate_page_end_address::(self.flash_range(), partial_open_page); let key_len = key.serialize_into(data_buffer)?; let item_data_length = key_len @@ -670,10 +667,10 @@ impl, K: Key> MapStorage { } let page_data_start_address = - calculate_page_address::(self.flash_range(), page_index) + S::WORD_SIZE as u32; + calculate_page_address::(self.flash_range(), page_index) + + S::WORD_SIZE as u32 * 2; let page_data_end_address = - calculate_page_end_address::(self.flash_range(), page_index) - - S::WORD_SIZE as u32; + calculate_page_end_address::(self.flash_range(), page_index); // Go through all items on the page let mut item_headers = @@ -835,9 +832,8 @@ impl, K: Key> MapStorage { )?; let start_address = - calculate_page_address::(self.flash_range(), first_page) + S::WORD_SIZE as u32; - let end_address = - calculate_page_end_address::(self.flash_range(), first_page) - S::WORD_SIZE as u32; + calculate_page_address::(self.flash_range(), first_page) + S::WORD_SIZE as u32 * 2; + let end_address = calculate_page_end_address::(self.flash_range(), first_page); Ok(MapItemIter { storage: self, @@ -858,11 +854,11 @@ impl, K: Key> MapStorage { // doesn't have a newer value somewhere else. let mut next_page_write_address = - calculate_page_address::(self.flash_range(), target_page) + S::WORD_SIZE as u32; + calculate_page_address::(self.flash_range(), target_page) + S::WORD_SIZE as u32 * 2; let mut it = ItemIter::new( - calculate_page_address::(self.flash_range(), source_page) + S::WORD_SIZE as u32, - calculate_page_end_address::(self.flash_range(), source_page) - S::WORD_SIZE as u32, + calculate_page_address::(self.flash_range(), source_page) + S::WORD_SIZE as u32 * 2, + calculate_page_end_address::(self.flash_range(), source_page), ); while let Some((item, item_address)) = it.next(&mut self.inner.flash, data_buffer).await? { let (key, _) = K::deserialize_from(item.data())?; @@ -1101,11 +1097,11 @@ impl> MapItemIter<'_, K, S, C> { calculate_page_address::( self.storage.inner.flash_range.clone(), self.current_page_index, - ) + S::WORD_SIZE as u32, + ) + S::WORD_SIZE as u32 * 2, calculate_page_end_address::( self.storage.inner.flash_range.clone(), self.current_page_index, - ) - S::WORD_SIZE as u32, + ), ); break; } diff --git a/src/mock_flash.rs b/src/mock_flash.rs index 35d7a9e..2861631 100644 --- a/src/mock_flash.rs +++ b/src/mock_flash.rs @@ -139,10 +139,9 @@ impl let page_data_start = crate::calculate_page_address::(Self::FULL_FLASH_RANGE, page_index) - + Self::WORD_SIZE as u32; + + Self::WORD_SIZE as u32 * 2; let page_data_end = - crate::calculate_page_end_address::(Self::FULL_FLASH_RANGE, page_index) - - Self::WORD_SIZE as u32; + crate::calculate_page_end_address::(Self::FULL_FLASH_RANGE, page_index); let mut found_item = None; let mut it = crate::item::ItemHeaderIter::new(page_data_start, page_data_end); diff --git a/src/queue/mod.rs b/src/queue/mod.rs index c7433d1..65b540f 100644 --- a/src/queue/mod.rs +++ b/src/queue/mod.rs @@ -193,9 +193,9 @@ impl> QueueStorage { let current_page = self.find_youngest_page().await?; let page_data_start_address = - calculate_page_address::(self.flash_range(), current_page) + S::WORD_SIZE as u32; + calculate_page_address::(self.flash_range(), current_page) + S::WORD_SIZE as u32 * 2; let page_data_end_address = - calculate_page_end_address::(self.flash_range(), current_page) - S::WORD_SIZE as u32; + calculate_page_end_address::(self.flash_range(), current_page); self.inner.partial_close_page(current_page).await?; @@ -222,13 +222,13 @@ impl> QueueStorage { self.inner.partial_close_page(next_page).await?; next_address = Some( calculate_page_address::(self.flash_range(), next_page) - + S::WORD_SIZE as u32, + + S::WORD_SIZE as u32 * 2, ); } (PageState::Closed, _) | (PageState::PartialOpen, true) => { let next_page_data_start_address = calculate_page_address::(self.flash_range(), next_page) - + S::WORD_SIZE as u32; + + S::WORD_SIZE as u32 * 2; if !allow_overwrite_old_data && self.inner.page_contains_live_data(next_page).await? @@ -374,9 +374,9 @@ impl> QueueStorage { // See how much space we can find in the current page. let page_data_start_address = - calculate_page_address::(self.flash_range(), current_page) + S::WORD_SIZE as u32; + calculate_page_address::(self.flash_range(), current_page) + S::WORD_SIZE as u32 * 2; let page_data_end_address = - calculate_page_end_address::(self.flash_range(), current_page) - S::WORD_SIZE as u32; + calculate_page_end_address::(self.flash_range(), current_page); let next_item_address = match self.inner.cache.first_item_after_written(current_page) { Some(next_item_address) => next_item_address, @@ -433,9 +433,8 @@ impl> QueueStorage { // See how much space we can find in the current page. let page_data_start_address = - calculate_page_address::(self.flash_range(), page) + S::WORD_SIZE as u32; - let page_data_end_address = - calculate_page_end_address::(self.flash_range(), page) - S::WORD_SIZE as u32; + calculate_page_address::(self.flash_range(), page) + S::WORD_SIZE as u32 * 2; + let page_data_end_address = calculate_page_end_address::(self.flash_range(), page); if page_empty { total_free_space += page_data_end_address - page_data_start_address; @@ -560,7 +559,7 @@ impl> QueueStorage { Some(address) => address, None => { calculate_page_address::(self.inner.flash_range.clone(), oldest_page) - + S::WORD_SIZE as u32 + + S::WORD_SIZE as u32 * 2 } }; @@ -735,7 +734,7 @@ impl<'s, S: NorFlash, C: CacheImpl<()>> QueueIterator<'s, S, C> { let current_address = calculate_page_address::( self.storage.inner.flash_range.clone(), next_page, - ) + S::WORD_SIZE as u32; + ) + S::WORD_SIZE as u32 * 2; self.next_address = NextAddress::Address(current_address); @@ -750,7 +749,7 @@ impl<'s, S: NorFlash, C: CacheImpl<()>> QueueIterator<'s, S, C> { let page_data_end_address = calculate_page_end_address::( self.storage.inner.flash_range.clone(), current_page, - ) - S::WORD_SIZE as u32; + ); // Search for the first item with data let mut it = ItemHeaderIter::new(current_address, page_data_end_address); @@ -1194,7 +1193,7 @@ mod tests { push_stats.take_average(pushes), FlashAverageStatsResult { avg_erases: 0.0, - avg_reads: 16.864, + avg_reads: 13.1764, avg_writes: 3.1252, avg_bytes_read: 105.4112, avg_bytes_written: 60.5008 @@ -1204,7 +1203,7 @@ mod tests { peek_stats.take_average(peeks), FlashAverageStatsResult { avg_erases: 0.0052, - avg_reads: 3.8656, + avg_reads: 3.3032, avg_writes: 0.0, avg_bytes_read: 70.4256, avg_bytes_written: 0.0 @@ -1214,7 +1213,7 @@ mod tests { pop_stats.take_average(pops), FlashAverageStatsResult { avg_erases: 0.0572, - avg_reads: 3.7772, + avg_reads: 3.2148, avg_writes: 1.0, avg_bytes_read: 69.7184, avg_bytes_written: 8.0 @@ -1285,7 +1284,7 @@ mod tests { push_stats.take_average(pushes), FlashAverageStatsResult { avg_erases: 0.0, - avg_reads: 16.864, + avg_reads: 13.1764, avg_writes: 3.1252, avg_bytes_read: 105.4112, avg_bytes_written: 60.5008 @@ -1295,7 +1294,7 @@ mod tests { pop_stats.take_average(pops), FlashAverageStatsResult { avg_erases: 0.0624, - avg_reads: 23.5768, + avg_reads: 17.064, avg_writes: 1.0, avg_bytes_read: 180.512, avg_bytes_written: 8.0 From cf9c66407e28530631c85a27931028b0a85e1629 Mon Sep 17 00:00:00 2001 From: Dion Dokter Date: Tue, 13 Oct 2026 15:40:56 +0200 Subject: [PATCH 3/3] Be more strict about (partially) closing pages to avoid extra reads --- README.md | 6 +++--- src/cache/tests.rs | 8 ++++---- src/lib.rs | 29 ++++++++++------------------- src/queue/mod.rs | 39 +++++++++++++++++++++++++++------------ 4 files changed, 44 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index f588407..f0cbac4 100644 --- a/README.md +++ b/README.md @@ -90,9 +90,9 @@ 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 | | --------------------------: | ---------------------------: | -------------------: | -------------------: | ------------------: | ---------------------: | | All `Uncached` | 0 | 100% | 100% | 100% | 100% | -| Only `CalculatedPageStates` | 24 | 87% | 97% | 58% | 85% | -| Only `ArrayPageStates` | 1 * num pages | 87% | 97% | 58% | 85% | -| Only `ArrayPagePointers` | 8 * num pages | 91% | 92% | 50% | 28% | +| Only `CalculatedPageStates` | 24 | 87% | 97% | 59% | 86% | +| Only `ArrayPageStates` | 1 * num pages | 87% | 97% | 59% | 86% | +| Only `ArrayPagePointers` | 8 * num pages | 91% | 92% | 49% | 28% | | Only `ArrayKeyPointers` | (sizeof(KEY) + 4) * num keys | half: 82%, full: 20% | half: 80%, full: 17% | - | - | | All array caches together | | 7% | 8% | 9% | 14% | diff --git a/src/cache/tests.rs b/src/cache/tests.rs index 25d934f..2f19b04 100644 --- a/src/cache/tests.rs +++ b/src/cache/tests.rs @@ -22,9 +22,9 @@ mod queue_tests { run_test(Cache::new_uncached()).await, FlashStatsResult { erases: 149, - reads: 116519, + reads: 114520, writes: 6299, - bytes_read: 651212, + bytes_read: 647214, bytes_written: 53299 } ); @@ -79,9 +79,9 @@ mod queue_tests { .await, FlashStatsResult { erases: 149, - reads: 58441, + reads: 56442, writes: 6299, - bytes_read: 186588, + bytes_read: 182590, bytes_written: 53299 } ); diff --git a/src/lib.rs b/src/lib.rs index 903c431..a5b413b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -253,10 +253,9 @@ impl, KEY> GenericStorage { /// Fully closes a page by writing both the start and end marker async fn close_page(&mut self, page_index: usize) -> Result<(), Error> { - let current_state = self.partial_close_page(page_index).await?; - - if current_state != PageState::PartialOpen { - return Ok(()); + #[cfg(debug_assertions)] + if self.get_page_state_cached(page_index).await? != PageState::PartialOpen { + panic!("Not open"); } self.cache @@ -279,22 +278,14 @@ impl, KEY> GenericStorage { } /// Partially close a page by writing the start marker - async fn partial_close_page( - &mut self, - page_index: usize, - ) -> Result> { - let current_state = self.get_page_state_cached(page_index).await?; - - if current_state != PageState::Open { - return Ok(current_state); + async fn partial_close_page(&mut self, page_index: usize) -> Result<(), Error> { + #[cfg(debug_assertions)] + if self.get_page_state_cached(page_index).await? != PageState::Open { + panic!("Not open"); } - let new_state = match current_state { - PageState::Closed => PageState::Closed, - PageState::PartialOpen | PageState::Open => PageState::PartialOpen, - }; - - self.cache.notice_page_state(page_index, new_state, true); + self.cache + .notice_page_state(page_index, PageState::PartialOpen, true); let buffer = AlignedBuf([MARKER; MAX_WORD_SIZE]); let page_start_address = calculate_page_address::(self.flash_range.clone(), page_index); @@ -308,7 +299,7 @@ impl, KEY> GenericStorage { backtrace: std::backtrace::Backtrace::capture(), })?; - Ok(new_state) + Ok(()) } #[cfg(any(test, feature = "std", fuzzing))] diff --git a/src/queue/mod.rs b/src/queue/mod.rs index 65b540f..a46dcb9 100644 --- a/src/queue/mod.rs +++ b/src/queue/mod.rs @@ -190,14 +190,16 @@ impl> QueueStorage { return Err(Error::ItemTooBig); } - let current_page = self.find_youngest_page().await?; + let (current_page, current_page_state) = self.find_youngest_page().await?; let page_data_start_address = calculate_page_address::(self.flash_range(), current_page) + S::WORD_SIZE as u32 * 2; let page_data_end_address = calculate_page_end_address::(self.flash_range(), current_page); - self.inner.partial_close_page(current_page).await?; + if current_page_state != PageState::PartialOpen { + self.inner.partial_close_page(current_page).await?; + } // Find the last item on the page so we know where we need to write @@ -348,7 +350,7 @@ impl> QueueStorage { self.inner.cache.invalidate_cache_state(); } - let current_page = self.find_youngest_page().await?; + let (current_page, _) = self.find_youngest_page().await?; // Check if we have space on the next page let next_page = self.inner.next_page(current_page); @@ -481,14 +483,14 @@ impl> QueueStorage { Ok(total_free_space) } - async fn find_youngest_page(&mut self) -> Result> { + async fn find_youngest_page(&mut self) -> Result<(usize, PageState), Error> { let last_used_page = self .inner .find_first_page(0, PageState::PartialOpen) .await?; if let Some(last_used_page) = last_used_page { - return Ok(last_used_page); + return Ok((last_used_page, PageState::PartialOpen)); } // We have no partial open page. Search for a closed page to anker ourselves to @@ -508,7 +510,7 @@ impl> QueueStorage { }; if let Some(first_open_page) = first_open_page { - return Ok(first_open_page); + return Ok((first_open_page, PageState::Open)); } // All pages are closed... This is not correct. @@ -519,7 +521,7 @@ impl> QueueStorage { } async fn find_oldest_page(&mut self) -> Result> { - let youngest_page = self.find_youngest_page().await?; + let (youngest_page, _) = self.find_youngest_page().await?; // The oldest page is the first non-open page after the youngest page let oldest_closed_page = self @@ -1193,9 +1195,9 @@ mod tests { push_stats.take_average(pushes), FlashAverageStatsResult { avg_erases: 0.0, - avg_reads: 13.1764, + avg_reads: 12.1768, avg_writes: 3.1252, - avg_bytes_read: 105.4112, + avg_bytes_read: 97.4144, avg_bytes_written: 60.5008 } ); @@ -1284,9 +1286,9 @@ mod tests { push_stats.take_average(pushes), FlashAverageStatsResult { avg_erases: 0.0, - avg_reads: 13.1764, + avg_reads: 12.1768, avg_writes: 3.1252, - avg_bytes_read: 105.4112, + avg_bytes_read: 97.4144, avg_bytes_written: 60.5008 } ); @@ -1337,11 +1339,24 @@ mod tests { Cache::new_uncached(), ); + // Layout: + // 1. Closed + // 2. Closed + // 3. Partial open + // 4. Open + + storage.inner.partial_close_page(0).await.unwrap(); storage.inner.close_page(0).await.unwrap(); + + storage.inner.partial_close_page(1).await.unwrap(); storage.inner.close_page(1).await.unwrap(); + storage.inner.partial_close_page(2).await.unwrap(); - assert_eq!(storage.find_youngest_page().await.unwrap(), 2); + assert_eq!( + storage.find_youngest_page().await.unwrap(), + (2, PageState::PartialOpen) + ); assert_eq!(storage.find_oldest_page().await.unwrap(), 0); }