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

HashMap using RawTable from hashbrown #54

Closed
wants to merge 3 commits into from

Conversation

hansihe
Copy link

@hansihe hansihe commented Dec 10, 2019

This is a copy/paste of the HashMap implementation from hashbrown.

This depends on rust-lang/hashbrown#133, which adds allocator support to hashbrown's RawTable. Do not merge before this lands.

This is split into two parts:

  1. Add a unstable_core_alloc feature that uses the Alloc trait in std instead of the copied internal version.
  2. Add a collections_hash feature that depends on both the unstable_core_alloc feature and the hashbrown crate. I thought it was best to add a new feature for this since it adds a whole new dependency.

Bikeshedding of the feature names is encouraged. If this is a welcome addition, I would be open to adding HashSet as well, which should require significantly less code.

The `unstable_core_alloc` feature implements the `Alloc` trait in
`core::alloc` instead of copying the implementation and implementing
that.
Copy link
Owner

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this.

However, I don't feel comfortable maintaining this code. I think a better path forward would be to push on getting custom allocators in libstd and in the meantime, get this crate and hashbrown both working with the allocator_api crate's trait (a copy of the unstable Alloc trait from liballoc), so that you could connect the two without either this repo or hashbrown needing to maintain/fork a bunch of code.

@TethysSvensson
Copy link
Contributor

I am also doubtful of whether this would actually be useful in practice, since bumpalo is not well-suited for applications in need of frequent re-allocations.

@fitzgen
Copy link
Owner

fitzgen commented Jan 3, 2020

Closing this as explained in #54 (review)

@fitzgen fitzgen closed this Jan 3, 2020
@Amanieu
Copy link
Contributor

Amanieu commented Jan 3, 2020

I'm happy to accept a PR adding allocator_api support to hashbrown. You can extend your existing PR (rust-lang/hashbrown#133) to add allocator support to the full HashMap API instead of just the internal RawTable API.

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.

4 participants