Skip to content
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

Force xopen to use the main thread when cores==1 #572

Merged
merged 2 commits into from
Oct 6, 2021

Conversation

rhpvorderman
Copy link
Collaborator

@rhpvorderman rhpvorderman commented Oct 5, 2021

Currently cutadapt opens external programs on the default setting, thus using more than 1 core by default. Even if cores is explicitly set to 1.
Cores ==0 does not help the situation.

EDIT: As shown in many many benchmarks. There is overhead to using multithreading. The best way to utilize a cluster is to open a multitude of single threaded processes. Cutadapt is fast enough to do this in reasonable time.

EDIT2: A hidden flag that explicitly sets the xopen threads could be an alternative.

src/cutadapt/__main__.py Outdated Show resolved Hide resolved
@rhpvorderman
Copy link
Collaborator Author

Oh by the way. Using xopen with threads=0 might incur a hefty performance penalty due to reasons which I highlight (and solve) in this PR: pycompression/xopen#78

@marcelm
Copy link
Owner

marcelm commented Oct 6, 2021

I saw the PR, thanks, will look at it later. Not sure if I have time today, though

@marcelm marcelm merged commit d75f110 into marcelm:main Oct 6, 2021
@marcelm
Copy link
Owner

marcelm commented Oct 6, 2021

Thanks!

@rhpvorderman rhpvorderman deleted the patch-3 branch October 6, 2021 08:13
@rhpvorderman
Copy link
Collaborator Author

Ah thanks, for letting me know you saw it. Take your time!
I also think this is a bug in CPython actually. So I will post a bug there and make a modification upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants