`map::deserialize_from` is called very often

[t-moe]

Hi @diondokter , Today I noticed the following behavior, which could be a bug, but also could be totally fine:

• I've implemented StorageItem for a type DummyConfig including deserialize_key_only.
• I've written multiple DummyConfig instances with the same key to the flash using store_item.
• My deserialize_from implementation contains a log statement.
• Now when I try to get an item from the flash using fetch_item I get a deserialize_from invocation for every matching item that was ever written (to the same page?). Even the outdated ones.
• Finally the correct (latest) item is returned from fetch_item

I'm using NoCache and I was a bit surprised by this behavior.

In the readme you mention:

An item is considered erased when its data CRC field is 0.

So I would assume that marked-as-erased items are not deserialized and simply skipped...

Again, this is probably totally fine and I can hardly imagine that you have a bug here (given the excellent quality of this crate). So it is more a question out of interest :).

Thank you

[diondokter]

Hey there!

It should only be deserialize_key_only that is called that often. By default it calls the full deserialize and just returns the key. That's why I added the key only function!

If you look at the trait bounds, you'll see that the map functions use the NorFlash trait, not the MultiWriteNorFlash trait. This means we can only write data once and thus can't erase anything (which would be a second write). So this means that when we search for a key, we need to deserialize all items to be able to check their keys.

[diondokter]

I guess an extra store function could be created that would first search for the existing key, then does a normal store and then erases the old item. This function would then require MultiWriteNorFlash.

Any suggestions for a name? :P

[t-moe]

Here is an example which shows the multiple calls to deserialize_from. I'm on a esp32c6 devkit, starting with an erased flash.

main.rs

#![no_std]
#![no_main]
#![feature(type_alias_impl_trait)]

use core::ops::Range;
use esp32c6_hal::{clock::ClockControl, peripherals::Peripherals, prelude::*};
use esp_backtrace as _;
use esp_storage::FlashStorage;
use sequential_storage::cache::NoCache;
use sequential_storage::map::{fetch_item, StorageItem, store_item};
use log::*;

#[derive(Debug)]
struct DummyConfig {
    key: u8,
    val: u8
}

impl StorageItem for DummyConfig {
    type Key = u8;
    type Error = ();
    fn serialize_into(&self, buffer: &mut [u8]) -> Result<usize, Self::Error> {
        debug!("serialize_into");
        buffer[0] = self.key;
        buffer[1] = self.val;
        Ok(2)
    }
    fn deserialize_from(buffer: &[u8]) -> Result<Self, Self::Error> where Self: Sized {
        debug!("deserialize_from");
        Ok(Self {
            key: buffer[0],
            val: buffer[1],
        })
    }
    fn deserialize_key_only(buffer: &[u8]) -> Result<Self::Key, Self::Error> where Self: Sized {
        debug!("deserialize_key_only");
        Ok(buffer[0])
    }
    fn key(&self) -> Self::Key {
        self.key
    }
}

const CONFIG_RANGE: Range<u32> = 0x9000..(0x9000+0x4000);

#[embassy_executor::main(entry = "riscv_rt_macros::entry")]
async fn main(_s: embassy_executor::Spawner) -> ! {
    let peripherals = Peripherals::take();
    let system = peripherals.SYSTEM.split();

    ClockControl::max(system.clock_control).freeze();
    esp_println::logger::init_logger_from_env();

    let mut flash = embassy_embedded_hal::adapter::BlockingAsync::new(FlashStorage::new());
    let mut buf = [0u8; FlashStorage::SECTOR_SIZE as usize];

    let mut config = DummyConfig{ key: 1, val: 0};

    for v in 0..5 {
        config.val = v;
        debug!("Storing item {:?}", config);
        store_item(&mut flash, CONFIG_RANGE, &mut NoCache::new(), &mut buf, &config)
            .await.unwrap();

    }

    debug!("Reading item");
    let restored =  fetch_item::<DummyConfig, _>(&mut flash, CONFIG_RANGE, &mut NoCache::new(), &mut buf, config.key).await.unwrap().unwrap();
    debug!("restored item {:?}", restored);

    loop{}
}

Output:

DEBUG - Storing item DummyConfig { key: 1, val: 0 }
DEBUG - serialize_into
DEBUG - Storing item DummyConfig { key: 1, val: 1 }
DEBUG - serialize_into
DEBUG - Storing item DummyConfig { key: 1, val: 2 }
DEBUG - serialize_into
DEBUG - Storing item DummyConfig { key: 1, val: 3 }
DEBUG - serialize_into
DEBUG - Storing item DummyConfig { key: 1, val: 4 }
DEBUG - serialize_into
DEBUG - Reading item
DEBUG - deserialize_key_only
DEBUG - deserialize_from
DEBUG - deserialize_key_only
DEBUG - deserialize_from
DEBUG - deserialize_key_only
DEBUG - deserialize_from
DEBUG - deserialize_key_only
DEBUG - deserialize_from
DEBUG - deserialize_key_only
DEBUG - deserialize_from
DEBUG - restored item DummyConfig { key: 1, val: 4 }

Dependencies

[dependencies]
esp32c6-hal = "0.8.0"
esp-backtrace = { version = "0.11.0", features = ["esp32c6", "panic-handler", "exception-handler", "println"] }
esp-println = { version = "0.9.0", features = ["esp32c6", "log", "colors", "jtag-serial"], default-features = false }
log = { version = "0.4.20" }
sequential-storage = "0.9.1"
embedded-storage = "0.3.1"
embassy-embedded-hal = { version = "0.1.0" , default-features = false}
esp-storage = { version = "0.3.0" , features = ["nor-flash", "esp32c6"]}
embassy-executor = { version = "0.5.0" , features = ["executor-thread", "nightly", "arch-riscv32"]}
riscv-rt-macros = "0.2.1"

[diondokter]

Ah, I think you're seeing this loop: https://github.com/tweedegolf/sequential-storage/blob/6612981f590e690df88f5c2d041e19129d1a323c/src/map.rs#L264-L272

If we found the same key, we deserialize the item fully to be able to keep on to it. It would be possible to only record the address and do the full deserialization at the end, but then we'd have to reread the flash memory. I guess that could be faster in some cases. Really depends on deserialization complexity, cpu speed and flash read speed.

So currently this is optimized for high cpu speed, low flash read speed. Could be nice to have a flag to indicated a high flash read speed... But that's difficult to pull off correctly and not very high prio IMO

[t-moe]

Ah, I see. Thanks for elaborating. I was somehow under the impression, that you would immediately erase outdated items (e.g. setting the crc of the old item to 0, after having successfully written the new item). But I see now, that you only erase them on demand or when migrating to a new page. With this implementation, it totally makes sense to keep the code as it is.

Also:

So currently this is optimized for high cpu speed, low flash read speed.

Seems very reasonable.

So no need to change anything ;). Thanks for taking the time to answer!

[diondokter]

So no need to change anything ;). Thanks for taking the time to answer!

No problem! Feel free to ask more in the future. Can only add to the quality of the crate