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 proc find to heapqueue #14628

Merged
merged 8 commits into from
Jun 10, 2020
Merged

Add proc find to heapqueue #14628

merged 8 commits into from
Jun 10, 2020

Conversation

c-blake
Copy link
Contributor

@c-blake c-blake commented Jun 10, 2020

stride double hashing) part of recent sets & tables changes (which has
still been causing bugs over a month later (e.g., two days ago
#13794) as well as still having
several "figure this out" implementation question comments in them (see
just diffs of this PR).

This topic has been discussed in many places:
  #13393
  #13418
  #13440
  #13794

Alternative/non-mandatory stronger integer hashes (or vice-versa opt-in
identity hashes) are a better solution that is more general (no illusion
of one hard-coded sequence solving all problems) while retaining the
virtues of linear probing such as cache obliviousness and age-less tables
under delete-heavy workloads (still untested after a month of this change).

The only real solution for truly adversarial keys is a hash keyed off of
data unobservable to attackers.  That all fits better with a few families
of user-pluggable/define-switchable hashes which can be provided in a
separate PR more about `hashes.nim`.

This PR carefully preserves the better (but still hard coded!) probing
of the  `intsets` and other recent fixes like `move` annotations, hash
order invariant tests, `intsets.missingOrExcl` fixing, and the move of
`rightSize` into `hashcommon.nim`.
@c-blake
Copy link
Contributor Author

c-blake commented Jun 10, 2020

I think those failing tioselectors tests have nothing to do with this PR and it's probably fine to merge. Let me know if otherwise.

@Araq
Copy link
Member

Araq commented Jun 10, 2020

Needs a .since annotation and a changelog entry.

@c-blake
Copy link
Contributor Author

c-blake commented Jun 10, 2020

The 2 test failures remain about other things. Anything else?

Copy link
Contributor

@dom96 dom96 left a comment

Choose a reason for hiding this comment

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

LGTM.

@dom96 dom96 merged commit 6aa971d into nim-lang:devel Jun 10, 2020
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.

3 participants