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

With isolation enabled tinypool keeps worker count at a minimum #23

Closed
Demivan opened this issue Feb 8, 2022 · 6 comments · Fixed by #24
Closed

With isolation enabled tinypool keeps worker count at a minimum #23

Demivan opened this issue Feb 8, 2022 · 6 comments · Fixed by #24

Comments

@Demivan
Copy link

Demivan commented Feb 8, 2022

If isolation is enabled _ensureMinimumWorkers is called when worker is done. But it only creates a worker if worker count is less then minimum.

Because minimum worker count is half of physical core count right now, this means that Tinypool is using only a quarter of actual PC cores. This is really inefficient. On my 16 core CPU Tinypool is using only 4 cores.

@Demivan Demivan changed the title With isolation enabled tinypool keeps worker count an a minimum With isolation enabled tinypool keeps worker count at a minimum Feb 8, 2022
@Aslemammad
Copy link
Member

So we've got two solutions now:
1: #22
2: making your quarter of cores into half of them!

Which one do you suggest? For now, two is more appropriate, which can be a bug! and then 1

@Demivan
Copy link
Author

Demivan commented Feb 8, 2022

I think I will add a minThreads/maxThreads override in Vitest for now.
Tinypool just needs a better scheduling for isolated workers. It should keep using available threads.

@dominikg
Copy link

dominikg commented Feb 8, 2022

I think I will add a minThreads/maxThreads override in Vitest for now. Tinypool just needs a better scheduling for isolated workers. It should keep using available threads.

https://github.com/vitest-dev/vitest/blob/578258e026922ab2bbbebca5de864570b7101de5/packages/vitest/src/node/config.ts#L103

@Demivan
Copy link
Author

Demivan commented Feb 8, 2022

vitest-dev/vitest#705

@Aslemammad Aslemammad mentioned this issue Feb 8, 2022
@Aslemammad
Copy link
Member

@Demivan Could you check #24 and try it!

@Aslemammad
Copy link
Member

@Demivan Please let's keep Vitest aligned with tinypool! So let's not override, so we can benefit from improvements properly 😄

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 a pull request may close this issue.

3 participants