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

Improve concurrency in multithreaded BGZF writer (bgzf_mt) #51

Closed
wants to merge 2 commits into
base: develop
from

Conversation

Projects
None yet
3 participants
@mlin

mlin commented Jan 3, 2014

This PR includes two commits that significantly increase compression throughput with the multithreaded BGZF writer (bgzf_mt), improving the performance of samtools sort, merge, and view -b in multithreaded mode.

The first commit yields most of the performance benefit by continuing to ingest data while multithreaded compression proceeds in the background. This change does not complicate things very much and hopefully should be fairly uncontroversial.

The second commit, which yields a smaller improvement, is a bit riskier because it introduces writing to the file handle from different threads. It currently works out that the writes are serialized without needing an explicit mutex, but this could be a maintenance concern going forward.

mlin added some commits Dec 27, 2013

Improve concurrency in multithreaded BGZF writer (bgzf_mt)
Instead of periodically stalling the caller of bgzf_write to compress
buffered data, run multithreaded compression on a 'background' workspace
while continuing to ingest data into a 'foreground' workspace. When the
foreground is full and the current round of background compression is
complete, write out the results and swap the foreground & background. This
significantly increases compression throughput with multiple cores under
most circumstances, because the worker threads don't have to wait as long
for the next batch of data to start compressing.

The n_threads parameter to bgzf_mt now specifies the number of worker
threads to spawn in addition to the main thread. Therefore, it's
theoretically possible for the process to utilize n_threads+1 CPUs.
However, in most applications the main thread is largely I/O-bound.

Maintaining foreground & background workspaces doubles the memory usage
associated with bgzf_mt, which was however modest to begin with
(~16MB*n_threads, based on the suggested upper limit of n_sub_blks).
bgzf_mt: write out compressed data from a worker thread instead of the
main thread. Prevents stalling the main thread on I/O in some
circumstances.

This change incurs some maintenance risk because it introduces use of
hwrite from different threads (the main thread and one worker thread)
without an explicit mutex. It does work out that only one thread is
writing at any given time, modulo a new assert in bgzf_raw_write, and this
will need to be maintained going forward.

@jmarshall jmarshall added this to the 1.1 milestone Mar 24, 2014

@jmarshall jmarshall modified the milestones: wishlist, 1.1 Feb 9, 2015

@mlin mlin referenced this pull request Jul 16, 2015

Closed

Orodeh ubcf overhaul #241

@jkbonfield

This comment has been minimized.

Show comment
Hide comment
@jkbonfield

jkbonfield Mar 20, 2017

Contributor

Obsoleted by the thread pool interface. Sorry this hung around for so long without commenting.

Contributor

jkbonfield commented Mar 20, 2017

Obsoleted by the thread pool interface. Sorry this hung around for so long without commenting.

@jkbonfield jkbonfield closed this Mar 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment