Speed up mpileup by reducing passes through arrays and qual#1995
Merged
Conversation
The code has multiple passes through plp[i] getting ther qualities and comparing against conf->min_baseQ. This is done for printing the depth, which sequence bases to display, as well as obviously the quality values. All 3 of these are now computed in a single step, putting the values into kstrings. We still have additional excess loops if we ask for the optional columns, but that's not the default output and can be dealt with at a later stage. Also remove some of the excess seq_nt16_table and seq_nt_str back and forth, plus specialise for strand so we don't also have to call toupper and tolower plus detection of the "=" code. The combined speed up is 20-25% on the BAM files I tested (long and short read).
Member
|
This generally looks OK, although it would be good to check the various kstring operations for errors, otherwise there is a small risk of silently dropping data. It would be good to rename the shadowing |
Member
|
... or even better rename the shadowed one. Just rename it at the top, and the compiler will find any instances that need to be fixed up ... |
We now check ksprintf and kputc error codes. I haven't changed non-kstring I/O. We could go through changing all those too, but traditionally we don't check stdio at the per-putc level and rely on the error being propagated through to file closure. Also adjusted the main loop to avoid variable shadowing. (That's nothing to do with the speed up changes.)
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.
The code has multiple passes through plp[i] getting ther qualities and comparing against conf->min_baseQ. This is done for printing the depth, which sequence bases to display, as well as obviously the quality values. All 3 of these are now computed in a single step, putting the values into kstrings.
We still have additional excess loops if we ask for the optional columns, but that's not the default output and can be dealt with at a later stage.
Also remove some of the excess seq_nt16_table and seq_nt_str back and forth, plus specialise for strand so we don't also have to call toupper and tolower plus detection of the "=" code.
The combined speed up is 20-25% on the BAM files I tested (long and short read).