tweedegolf / sequential-storage

A crate for storing data in flash memory with minimal need for erasing pages
Apache License 2.0
99 stars 10 forks source link

map should take Key by reference #58

Closed jordens closed 2 months ago

jordens commented 3 months ago

Like #21.

If the Keys become reasonably large (heapless::String), there is a lot of stack thrashing for move and cloning going on.

There doesn't appear to be the need to move/own the key for either fetch_item or store_item, other than for caching. The key should be taken by reference.

On a similar note, there is no need to take range by value, just take it by reference and clone if absolutely necessary.

diondokter commented 3 months ago

Hi, I guess a reference would work. The key trait requires it to be clone, so when a copy is needed for storing in cache, the clone could still be made. This would be a breaking change though.

For range, I'm not gonna change it since edition 2024 will make the type Copy.