-
Notifications
You must be signed in to change notification settings - Fork 3
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
WIP: add Ctrl-C interrupt #253
Open
bluegenes
wants to merge
19
commits into
main
Choose a base branch
from
ctrlc
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains 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
verbose benchmarking results (in progress):
manysketch with ctrl-c:
|
see #40 (comment) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
addresses #40
This PR introduces a
ThreadManager
struct that contains a syncsender, a writer thread, and an atomic bool,interrupted
, which is set totrue
ifCtrl-C
is detected.ThreadManager
has three functions:new
for building a new instance and setting upCtrl-C
handlingsend
for sending data to the writer threadperform_cleanup
, to close the threads. This standardizes cleanup across interruption and successful completion.In each main function, we instantiate a
ThreadManager
and wrap in inArc<Mutex>
for safe sharing across threads. Then, while iterating through e.g. queries or against signatures, we check the status of theinterrupted
bool each iteration. Ifinterrupted
is ever true, we return, foregoing the remaining iterations.Sometimes the return is faster than others -- it really depends on how long each iteration takes (aka how often we are checking for interruption).
This PR adds this
ctrl-c
capture to:pairwise
manysearch
manysketch
multisearch
fastmultigather
manysearch
I want to punt the remaining commands to an issue for future work, since they require some refactoring (
fastgather
/fastmultigather
/cluster
do not usesend
/recv
) or it wouldn't even really be useful (index
,check
) since we only provide a super lightweight wrapper around core functionality.Notes/Questions:
In progress: benchmark to ensure this doesn't inflate runtimes.
benchmark summary
12k ICTV viral genomes, scaled=200
79.8m comparisons