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

[MISC] Improve bulk_contains performance #45

Merged
merged 4 commits into from
Aug 24, 2023
Merged

Conversation

eseiler
Copy link
Member

@eseiler eseiler commented Aug 15, 2023

Things that will also help:

  • aligned alloc

Things that might help:

  • range interface for bulk_contains, especially in conjunction with bulk_count

The changes rely on auto-vectorization and might be done more elegantly by hand.

192 is faster than before, but relatively slow. 128 and 256 can be done in one step with AVX2, but 192 requires two. 192 is around 30% slower than 128 or 256.

bulk_contains benchmark with 1GiB data, throughput in hashes/sec:

bins old new speedup
64 78.30 117.13 1.50
128 59.33 121.20 2.04
192 41.43 84.47 2.04
256 42.04 122.27 2.91
1024 16.50 95.74 5.80

@vercel
Copy link

vercel bot commented Aug 15, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hibf ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 15, 2023 4:18pm

@seqan-actions seqan-actions added the lint [INTERNAL] used for linting label Aug 15, 2023
@seqan-actions seqan-actions removed the lint [INTERNAL] used for linting label Aug 15, 2023
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (b89792b) 91.10% compared to head (d16026b) 91.11%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #45   +/-   ##
=======================================
  Coverage   91.10%   91.11%           
=======================================
  Files          37       37           
  Lines        1226     1227    +1     
=======================================
+ Hits         1117     1118    +1     
  Misses        109      109           
Files Changed Coverage Δ
include/hibf/interleaved_bloom_filter.hpp 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@seqan-actions seqan-actions added lint [INTERNAL] used for linting and removed lint [INTERNAL] used for linting labels Aug 15, 2023
@seqan-actions seqan-actions added lint [INTERNAL] used for linting and removed lint [INTERNAL] used for linting labels Aug 15, 2023
@seqan-actions seqan-actions added lint [INTERNAL] used for linting and removed lint [INTERNAL] used for linting labels Aug 15, 2023
@seqan-actions seqan-actions added lint [INTERNAL] used for linting and removed lint [INTERNAL] used for linting labels Aug 15, 2023
@seqan-actions seqan-actions added the lint [INTERNAL] used for linting label Aug 15, 2023
@seqan-actions seqan-actions removed the lint [INTERNAL] used for linting label Aug 15, 2023
Comment on lines +711 to +714
#ifndef NDEBUG
assert(bin_words != 0u);
assert(hash_funs != 0u);
#else
Copy link
Member

Choose a reason for hiding this comment

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

auch wegen der Auto-vectorisierung? Denn eigentlich sollten asserts im release mode ja sowieso wegcompiliert werden oder?

Copy link
Member Author

Choose a reason for hiding this comment

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

I want to actually assert in Debug mode, and not just declare it as UB. No idea what happens (guaranteed) when I have both __builtin_unreachable() and an assert :)

@eseiler eseiler merged commit 5caf0e3 into seqan:main Aug 24, 2023
24 checks passed
@eseiler eseiler deleted the misc/perf branch August 24, 2023 12:35
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