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

rework hash_combine and hash_mix for performance #441

Merged
merged 2 commits into from
Jul 9, 2023

Conversation

trws
Copy link
Contributor

@trws trws commented Jul 7, 2023

This ended up being a small change, tweaking the hash_combine function to work more like the boost version and selecting hash_mix based on size of size_t rather than the argument. The end result is that for spack inputs 5.6.2 performs almost exactly as well as 5.5.2, which is a 30%+ speedup in some tests.

This ended up being a small change, tweaking the hash_combine function
to work more like the boost version and selecting hash_mix based on size
of size_t rather than the argument.  The end result is that for spack
inputs 5.6.2 performs almost exactly as well as 5.5.2, which is a 30%+
speedup in some tests.
@rkaminsk
Copy link
Member

rkaminsk commented Jul 7, 2023

This ended up being a small change, tweaking the hash_combine function to work more like the boost version and selecting hash_mix based on size of size_t rather than the argument. The end result is that for spack inputs 5.6.2 performs almost exactly as well as 5.5.2, which is a 30%+ speedup in some tests.

I am puzzled. As far as I understand hash functions, this should be a really bad one. Now hash_combine is even commutative. I'll have to dig a little deeper to see what is actually happening here.

@rkaminsk
Copy link
Member

rkaminsk commented Jul 7, 2023

I just played around a bit and the key element of this patch seems to be applying the hash_mix function in hash_combine. With this we can also get the same speed up with a small twist to the original implementation. I'll commit a suggestion also cleaning up the mess with the templates I introduced a bit. 😉

EDIT: Sorry for the noise. Compilation on MacOS has to be fixed.

@rkaminsk
Copy link
Member

rkaminsk commented Jul 7, 2023

If this works for you, I would merge it like this. And thanks for digging into this. I don't think I would have found it myself.

@rkaminsk rkaminsk merged commit 4dec6a3 into potassco:master Jul 9, 2023
rkaminsk added a commit that referenced this pull request Jul 9, 2023
rkaminsk added a commit that referenced this pull request Jul 9, 2023
improve hash table performance (see #441)
tgamblin added a commit to spack/spack that referenced this pull request Feb 15, 2024
5.7.0 was just released. It includes a number of changes requested and/or
upstreamed by Spack developers, e.g.:

* API for accessing optimization priorities: potassco/clingo#406
* Hash optimization: potassco/clingo#441
* Contributing Guide: potassco/clingo#465
* Hiding more ELF symbols:
  * potassco/clingo#447
  * potassco/clingo#449
alalazo pushed a commit to spack/spack that referenced this pull request Feb 16, 2024
5.7.0 was just released. It includes a number of changes requested and/or
upstreamed by Spack developers, e.g.:

* API for accessing optimization priorities: potassco/clingo#406
* Hash optimization: potassco/clingo#441
* Contributing Guide: potassco/clingo#465
* Hiding more ELF symbols:
  * potassco/clingo#447
  * potassco/clingo#449
mathomp4 pushed a commit to mathomp4/spack that referenced this pull request Mar 27, 2024
5.7.0 was just released. It includes a number of changes requested and/or
upstreamed by Spack developers, e.g.:

* API for accessing optimization priorities: potassco/clingo#406
* Hash optimization: potassco/clingo#441
* Contributing Guide: potassco/clingo#465
* Hiding more ELF symbols:
  * potassco/clingo#447
  * potassco/clingo#449
teaguesterling pushed a commit to teaguesterling/spack that referenced this pull request Jun 15, 2024
5.7.0 was just released. It includes a number of changes requested and/or
upstreamed by Spack developers, e.g.:

* API for accessing optimization priorities: potassco/clingo#406
* Hash optimization: potassco/clingo#441
* Contributing Guide: potassco/clingo#465
* Hiding more ELF symbols:
  * potassco/clingo#447
  * potassco/clingo#449
kwryankrattiger pushed a commit to kwryankrattiger/spack that referenced this pull request Jul 9, 2024
5.7.0 was just released. It includes a number of changes requested and/or
upstreamed by Spack developers, e.g.:

* API for accessing optimization priorities: potassco/clingo#406
* Hash optimization: potassco/clingo#441
* Contributing Guide: potassco/clingo#465
* Hiding more ELF symbols:
  * potassco/clingo#447
  * potassco/clingo#449
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.

2 participants