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

Derive Eq for RandomState #32068

Closed
wants to merge 2 commits into from
Closed

Derive Eq for RandomState #32068

wants to merge 2 commits into from

Conversation

mpdn
Copy link
Contributor

@mpdn mpdn commented Mar 5, 2016

It should be possible to see if two hash functions are equal. In my case I need this in order to quickly union two Bloom filters (a simple bitwise OR if the hash functions are equal).

@sfackler
Copy link
Member

sfackler commented Mar 6, 2016

Test doesn't appear to compile: https://travis-ci.org/rust-lang/rust/builds/113961471#L3473

@mpdn
Copy link
Contributor Author

mpdn commented Mar 6, 2016

Errors should be fixed now. I ran make check before, but I must have misunderstood the output.

@alexcrichton
Copy link
Member

Tagging with T-libs so we can discuss in triage (just some new API surface area)

@alexcrichton alexcrichton self-assigned this Mar 6, 2016
@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 6, 2016
@huonw
Copy link
Member

huonw commented Mar 16, 2016

We discussed this in the libs meeting, and we weren't entirely convinced by the motivation.

In particular, it seems to me that something like unioning bloom filters would be better served by having a custom hasher that's the same globally, so bitor-ing always works (also, how does one union two bloom filters if the hashers are different? Storing all the elements added to be able rehash them? Isn't the point of a bloom filter to avoid that sort of memory overhead?). Additionally, as it stands, two RandomStates created with new or default are exceedingly unlikely to ever be equal.

Could you expand a little on your thoughts here?

@mpdn
Copy link
Contributor Author

mpdn commented Mar 17, 2016

... would be better served by having a custom hasher that's the same globally, so bitor-ing always works

Not by much. You would still need them to have the same size and number of buckets in order to union them. Ensuring equal hash function, buckets, and size is easy enough; you just clone a Bloom filter (which you can already do, as RandomState implements Clone).

Having a single, global hash function would work, but it would be less flexible (you would be bound to use whatever hash function the library defines), and it would not make unioning much easier due to the restrictions on size and number of buckets mentioned above.

also, how does one union two bloom filters if the hashers are different? Storing all the elements added to be able rehash them? Isn't the point of a bloom filter to avoid that sort of memory overhead?

Yeah, you can't union two different filters at all, at least not without iterating over all the original items inserted into them. I should probably have formulated myself a bit differently.

@alexcrichton alexcrichton removed the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 17, 2016
@bors
Copy link
Contributor

bors commented Mar 18, 2016

☔ The latest upstream changes (presumably #32248) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to resubmit with a rebase!

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.

5 participants