Skip to content

Allow key type to be unsized and prefer str to String as key type #516

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kxxt
Copy link

@kxxt kxxt commented May 15, 2025

Rationale

Currently, moka wraps the key type inside an Arc. Coupled with the usage of ToOwned trait,
this creates a two level pointer indirection and waste of some space if we use String as the key type, which is commonly used.
This PR creates a new trait ToOwnedArc which converts the type to be optimally wrapped inside an Arc
and then removes the Sized requirement on key type.

Warning

Although in general, Arc<String> seems to be an anti-pattern. But for some workload that already has a String,
it could avoid the potential reallocation when converting that String into Arc<str>.
So this PR might bring some negative performance effects. Well, I have made the ToOwnedArc trait less breaking so that users can continue to use String as they wish.

And this PR of course introduces BREAKING changes.

Benchmark

Unfortunately, mokabench does not have benches for string key type. So I currently don't have any benchmark results to back this PR.
Running mokabench with this PR produces similar results without this PR, though.

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 this pull request may close these issues.

1 participant