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 FrozenIndexMap and FrozenIndexSet #10

Merged
merged 3 commits into from
Dec 2, 2020

Conversation

pczarn
Copy link
Contributor

@pczarn pczarn commented Nov 13, 2020

Closes #6

The StringInterner::get_or_intern has an unnecessary call to to_string which allocates. There is no clean workaround, AFAIK. We would need an improved Entry API. See also ideas such as entry_or_clone: Extend entry API to work on borrowed keys rust-lang/rfcs#1769

Yet another nice-to-have thing would be a stable custom allocation story. With custom allocators, we could easily replace the meat of the string_interner crate. Each interner would hold its own allocator, thereby gaining top performance.

example: StringInterner
Cargo.toml Outdated
@@ -10,5 +10,9 @@ repository = "https://github.com/manishearth/elsa"
keywords = ["data-structure", "map", "frozen", "cache", "arena"]
categories = ["data-structures", "caching"]

[features]
default = ["indexmap"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the feature be on by default, or not?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No extra dependencies by default, please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Manishearth fixed

@pczarn
Copy link
Contributor Author

pczarn commented Nov 15, 2020

I did the right Cargo.toml, travis and README changes for the string_interner example. (in the last commit)

@pczarn pczarn force-pushed the frozen_index_map_set branch from a38f8b2 to 0a46740 Compare November 15, 2020 09:45
@Manishearth
Copy link
Owner

Manishearth commented Dec 2, 2020

Looks great, thank you! Sorry for the delay in reviewing!

@Manishearth Manishearth merged commit f9970bc into Manishearth:master Dec 2, 2020
@Manishearth
Copy link
Owner

Feel free to open a PR that releases version 1.4

@zertosh zertosh mentioned this pull request Mar 6, 2021
@zertosh
Copy link
Contributor

zertosh commented Mar 6, 2021

@Manishearth, I took the liberty of putting up the bump PR (#11) because I would really like to use FrozenIndexSet.

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.

(Optional?) (Frozen)Index{Map,Set}
3 participants