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

Default threads for webapp are set too high #1511

Closed
corneliusroemer opened this issue Jul 12, 2024 · 2 comments · Fixed by #1512
Closed

Default threads for webapp are set too high #1511

corneliusroemer opened this issue Jul 12, 2024 · 2 comments · Fixed by #1512
Labels
t:bug Type: bug, error, something isn't working t:feat Type: request of a new feature, functionality, enchancement

Comments

@corneliusroemer
Copy link
Member

I recently showed some Nextclade web. We dropped some 100 SC2 sequences and I was surprised how long it took for the first results to show up.

The likely culprit is that the number of threads to use was set at 8 (likely the default for their system, as I don't think they have customized it).

On my own system, I've experimented a little and settled on 3 threads - even though I have 10 cores.

There are good reasons to reduce our default to something like 3 for Nextclade web:

  1. Speedup is negligent for more threads
  2. Thread setup has a startup cost proportional to the number of threads used
  3. Most users of Nextclade web use (only) 10-100 sequences

If even for my routine analysis of 3000 SC2 sequences I don't benefit from more than 3 threads, then the average user definitely does not.

On top, startup delay is more important than time to finish analysis as people can already see partial results even if the analysis takes time.

Overall, I propose we set the default number of threads to use for Web to min(2, cores). Those who use Nextclade for large amounts of sequences can up the number of threads, but there will be few. Overall changing the default will make Nextclade snappier for almost everyone and leave better first impressions.

@corneliusroemer corneliusroemer added t:bug Type: bug, error, something isn't working t:feat Type: request of a new feature, functionality, enchancement needs triage Mark for review and label assignment labels Jul 12, 2024
@corneliusroemer
Copy link
Member Author

corneliusroemer commented Jul 12, 2024

Curiously, we reduced the threads from 8 to 3 in the past - I wonder how this user got 8 and not 3:

Some users have observed long startup times of the analysis in Nextclade Web. In this release we decreased the default number of processing threads from 8 to 3, such that startup time is now a little faster.

Note, I don't remember who the user was, but we were on safari. Maybe this matters.

@corneliusroemer
Copy link
Member Author

corneliusroemer commented Jul 12, 2024

I can reproduce that on safari we set the default number of threads to 8! In contrast to what the code suggests:

export const DEFAULT_NUM_THREADS = 2
export const MINIMUM_NUM_THREADS = 1
export const MAXIMUM_NUM_THREADS = 3
export const MEMORY_BYTES_PER_THREAD_MINIMUM = 200 * 1024 * 1024

This is Nextclade 3.8.1 on Safari on my M1 Mac 14.5
Safari 2024-07-12 10 46 07

Repro on Firefox:
Firefox 2024-07-12 10 47 35

On Chrome, the default is 3 - interestingly. So it seems that our code for thread defaults only works on Chromium?

Google Chrome 2024-07-12 10 48 27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:bug Type: bug, error, something isn't working t:feat Type: request of a new feature, functionality, enchancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants