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

Implement ContainerHash: a simple hashing structure for containers #324

Merged
merged 46 commits into from
Jul 21, 2020

Conversation

rodsan0
Copy link
Member

@rodsan0 rodsan0 commented Jul 2, 2020

ContainerHash utilizes hash_combine to assign a unique hash to a container, allowing their use in sets and unordered_map keys.

rodsan0 added 4 commits July 2, 2020 17:25
ContainerHash is a simple hashing algorithm for containers, based on the
Boost implementation.
It is useful for cases where a container is needed as a key in a
hash-dependant container, such as a set or an unordered_map.
Copy link
Member

@mmore500 mmore500 left a comment

Choose a reason for hiding this comment

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

Looks good! Needs tests?

source/tools/hash_utils.h Show resolved Hide resolved
source/tools/hash_utils.h Outdated Show resolved Hide resolved
source/tools/hash_utils.h Outdated Show resolved Hide resolved
rodsan0 added 22 commits July 5, 2020 22:59
A span is to an array what a string_view is to a string. Basically, it
is a lightweight and fast non-owning way to pass arrays
around while conserving their length.
Right now, we are using span-lite, an implementation of std::span, since
the later one will be available in C++20
This will dramatically reduce the number of collisions.
This test uses template metaprogramming* to test BitSets of different
length. In the future, it would be a good idea to update the rest of the
tests to make use of this technique too.

* in particular, integer sequences.
@rodsan0 rodsan0 requested a review from mmore500 July 11, 2020 02:46
Copy link
Member

@mmore500 mmore500 left a comment

Choose a reason for hiding this comment

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

Looking good! Almost there.

source/tools/BitSet.h Outdated Show resolved Hide resolved
source/tools/BitSet.h Outdated Show resolved Hide resolved
source/tools/BitSet.h Show resolved Hide resolved
source/tools/hash_utils.h Outdated Show resolved Hide resolved
source/tools/hash_utils.h Outdated Show resolved Hide resolved
source/tools/hash_utils.h Outdated Show resolved Hide resolved
source/tools/matchbin_metrics.h Outdated Show resolved Hide resolved
tests/tools/BitSet.cc Show resolved Hide resolved
tests/tools/hash_utils.cc Outdated Show resolved Hide resolved
Copy link
Member

@mmore500 mmore500 left a comment

Choose a reason for hiding this comment

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

This pull request adds a useful utility to hash any container with a begin/end while fixing a cavalcade of existing bugs/shortcomings that were unearthed along the way.

source/tools/matchbin_metrics.h Outdated Show resolved Hide resolved
tests/tools/BitSet.cc Outdated Show resolved Hide resolved
tests/tools/BitSet.cc Show resolved Hide resolved
tests/tools/hash_utils.cc Outdated Show resolved Hide resolved
source/tools/hash_utils.h Outdated Show resolved Hide resolved
@mmore500 mmore500 merged commit 5c2b68b into master Jul 21, 2020
@mmore500 mmore500 deleted the containerhash branch July 21, 2020 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants