Optimise samtools depth histogram incrementing code.#2078
Merged
Conversation
The incr with and without -q qual filter are now in their own
functions. These functions use HTS_OPT3 to force -O3 as gcc -O2
refuses to vectorise much at all before gcc 12.0. With O3 enforced,
we no longer need manual loop unrolling too, so the code is simpler.
Also removed the wrap-around vs non-wrap-around case as given the
external function we can handle both together quite easily with one
extra conditional.
Speed ups vary by compiler, but are particularly significant with clang.
Benchmarks of add_depth + incr_qual* with and without qual filtering
on 10 million novaseq reads.
depth -q10
gcc7 gcc13 clang16
dev -O2 12620 10219 8536
dev -O3 10933 10847 8321
PR -O2 10498 8020 5802
PR -O3 11087 9397 6312
depth (no -q)
gcc gcc13 clang16
dev -O2 10677 7097 4997
dev -O3 9564 8448 5340
PR -O2 8605 6528 4414
PR -O3 9231 7620 4369
NB: oddly -O3 is poorer than -O2 with HTS_OPT3 on key functions.
It's clear there is still a significant speed difference between gcc
and clang, but it's elsewhere in the add_depth function..
507ef56 to
cc1ba84
Compare
Contributor
Author
|
The speed tests using develop htslib branch (built with system gcc) and develop samtools (built using gcc7, gcc13 and clang16) are 14159, 10545, 11302 respectively. Those are perf record counters for the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NB: both of these are benchmarked with samtools/htslib#1805 in situ. Without that the numbers are considerably higher again. Samtools depth now takes about 40% longer than the pure-decode
test_view -B in.bamloop, so it's diminishing returns.The incr with and without -q qual filter are now in their own functions. These functions use HTS_OPT3 to force -O3 as gcc -O2 refuses to vectorise much at all before gcc 12.0. With O3 enforced, we no longer need manual loop unrolling too, so the code is simpler.
Also removed the wrap-around vs non-wrap-around case as given the external function we can handle both together quite easily with one extra conditional.
Speed ups vary by compiler, but are particularly significant with clang. Benchmarks of add_depth + incr_qual* with and without qual filtering on 10 million novaseq reads.
depth -q10
depth (no -q)
NB: oddly -O3 is poorer than -O2 with HTS_OPT3 on key functions.
It's clear there is still a significant speed difference between gcc and clang, but it's elsewhere in the add_depth function..