Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Somehow &[u8] as Key? #39

Closed
kaspar030 opened this issue Mar 29, 2024 · 5 comments · Fixed by #48
Closed

Somehow &[u8] as Key? #39

kaspar030 opened this issue Mar 29, 2024 · 5 comments · Fixed by #48

Comments

@kaspar030
Copy link

(Actually I want to use string literals, without going through a hasher.)

I don't mind using some more bytes for the Key, for my use case (some configuration values) a flash page is plenty.

I've looked into making Key support variable length types. It's doable (use a function for LEN, ...).
I stopped where the lifetimes show their head - in order to return slices, Key::deserialize_from() would need to carry a lifetime similar to Value.

What do you think? Would you be interested in a PR?

@diondokter
Copy link
Collaborator

diondokter commented Mar 29, 2024

Hi there!

I was meaning to keep the key simple. But having string support would be nice.
So if you think you can add support, that could be nice.
Lifetimes will be difficult. I think the biggest stumbling block will be that the key pointer cache needs to own a copy of the keys.
This means that the best realistic case is that only &'static [u8] and &'static str are supported (or other things with just a static lifetime).

If you see a way around this, I'd be interested to hear what your ideas are.

Something that could be done that could be good enough is to just make the key variable length. That way one could efficiently store array strings in there. You'd get 99% of the benefits but 0% of the lifetime issues.
We could also add an optional feature to implement the key trait for e.g. the types from the arrayvec crate.

@kaspar030
Copy link
Author

Thanks for the quick response!

Just aiming at array strings might be a good idea. Only &'static [u8]/ &static str` would also be fine. I'll look into it!

(I'll actually be on vacation next week, don't expect progress before mid April.)

@diondokter
Copy link
Collaborator

Hi @kaspar030! What's the status on this?
I want to release the 2.0 version at some point and this is the last thing that's open for that.

(Not trying to rush you, but at some point I'm gonna do it myself and it would be a waste of time if e.g. you were almost done already)

@kaspar030
Copy link
Author

I didn't get to work on this further, yet. Feel free to take over. :) Here is the branch where I hacked in, but I'd guess knowing the codebase would make you solve this differently.

@diondokter
Copy link
Collaborator

Thanks for the update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants