Skip to content

Commit

Permalink
Add an inline version of bgzf_read for small reads.
Browse files Browse the repository at this point in the history
The bgzf_read function is long and not something that's likely to get
inlined even if it was in the header.

However most of the time our calls request a small amount of data and
they fit within the buffer we've read, so we offer a static inline to
do the memcpy when we can, falling back to the long function when we
cannot.

In terms of CPU time it's not much difference, but the key thing is
that it's often CPU time saved in a main thread given the bulk of the
decode is often threaded.  An example of test_view -B -@16

develop:

    real    0m48.158s
    user    6m2.901s
    sys     0m28.134s

    real    0m48.730s
    user    6m3.707s
    sys     0m28.473s

    real    0m48.653s
    user    6m5.215s
    sys     0m28.637s

This PR:

    real    0m41.731s
    user    5m59.780s
    sys     0m30.393s

    real    0m41.945s
    user    6m0.367s
    sys     0m30.426s

So we can see it's a consistent win when threading, potentially 10-15%
faster throughput depending on work loads.
  • Loading branch information
jkbonfield committed Apr 19, 2024
1 parent deeb9f0 commit eca5c35
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 0 deletions.
20 changes: 20 additions & 0 deletions htslib/bgzf.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#define HTSLIB_BGZF_H

#include <stdint.h>
#include <string.h>
#include <sys/types.h>

#include "hts_defs.h"
Expand Down Expand Up @@ -143,6 +144,25 @@ typedef struct BGZF BGZF;
HTSLIB_EXPORT
ssize_t bgzf_read(BGZF *fp, void *data, size_t length) HTS_RESULT_USED;

/**
* bgzf_read optimised for small quantities, as a static inline
* See bgzf_read() normal function for return values.
*/
static inline ssize_t bgzf_read_small(BGZF *fp, void *data, size_t length) {
// A block length of 0 implies current block isn't loaded (see
// bgzf_seek_common). That gives negative available so careful on types
if ((ssize_t)length < fp->block_length - fp->block_offset) {
// Short cut the common and easy mode
memcpy((uint8_t *)data,
(uint8_t *)fp->uncompressed_block + fp->block_offset,
length);
fp->block_offset += length;
return length;
} else {
return bgzf_read(fp, data, length);
}
}

/**
* Write _length_ bytes from _data_ to the file. If no I/O errors occur,
* the complete _length_ bytes will be written (or queued for writing).
Expand Down
2 changes: 2 additions & 0 deletions sam.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ DEALINGS IN THE SOFTWARE. */
#include "htslib/hts_expr.h"
#include "header.h"

#define bgzf_read bgzf_read_small

#include "htslib/khash.h"
KHASH_DECLARE(s2i, kh_cstr_t, int64_t)
KHASH_SET_INIT_INT(tag)
Expand Down

0 comments on commit eca5c35

Please sign in to comment.