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

Add fallible APIs for StringInterner #70

Open
royaltm opened this issue May 26, 2024 · 2 comments
Open

Add fallible APIs for StringInterner #70

royaltm opened this issue May 26, 2024 · 2 comments

Comments

@royaltm
Copy link

royaltm commented May 26, 2024

First I want to say that this is a pretty awesome project. It provides interning for strings only but that's exactly what I needed. Other interning crates are usually more generic, but this always comes with some strings attached (no pun intended).

My point being - this is the only decent interning crate that works for no_std that I was able to find.

That said, string-interner API is designed with an assumption most modern projects make: the memory is the last thing to go. (so is the Rust std/alloc btw. but that's another story and it's currently changing)

Unfortunately in embedded systems this is not the case, and RAM is usually one of the fastest-depleting resources.

You might think it's a bit insane trying to build a JIT MAL compiler for a little MCU board, but that's exactly what I'm doing 🤔.

My humble request is if you can add a fallible API calls to StringInterner so users can take some action on failure instead of just relying on a watchdog to restart the firmware on panic.

The failure being either running out of symbols or RAM whichever comes first.

For example:

fn try_get_or_intern<T: AsRef<str>>(&mut self, string: T) -> Result<<B as Backend>::Symbol, TryReserveError>;
fn try_reserve(&mut self, additional: usize) -> Result<(), TryReserveError>;

#[derive(Debug, Clone, Copy)]
enum TryReserveError {
    NoMoreSymbols
    AllocationError
}

where try_reserve allows to change capacity ahead of time. This may also come with a complementary capacity method and a reserve method that panics.

I'm aware that this might be a big ask, involving substantial internal changes, but I'm ready to contribute myself if you are willing to go forward with this.

At least please let me know your thoughts. Thanks!

@Robbepop
Copy link
Owner

Robbepop commented May 27, 2024

Hi @royaltm ,

thank you for your feature request.

Given that Rust's own Vec type has try_reserve and push_within_capacity on nightly channel with similar semantics I think the addition of these proposed methods for StringInterner is fine.

Furthermore user with custom Symbols that are based on small values such as u16 or even u8 might profit a lot from these methods.

When implementing we should make sure to keep the Backend trait as simple as possible because it has to be implemented by all backends. I imagine that we could replace the current panicking methods with their try counterparts and have default implementations for the panicking implementations that simply forward to their try counterparts with a descriptive panic message.

Also we should simply name it Error instead of TryReserveError since it will be used for all the try methods and potential errors.

@Robbepop Robbepop changed the title A humble request for a fallible StringInterner API Add fallible APIs for StringInterner May 27, 2024
@royaltm
Copy link
Author

royaltm commented May 27, 2024

Thanks @Robbepop for your swift response! I'm glad you agreed to my request. I'll then attempt to implement changes according to your directions and see how it goes. Expect to hear from me soon with some more questions.

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

No branches or pull requests

2 participants