Pool waits a few seconds before restarting process

[sheremet-va]

While debugging #579 (reproduction: https://github.com/purepear/vitest-element-plus-debug, command npm run vitest-element), I noticed workers don't start for a few seconds after the first four are finished. I think there might be a bug that causes this performance regression.

[Aslemammad]

Thank you so much, Vladimir, I'll check it out.

[Aslemammad]

I released 0.3.1, could you check if the problem is still there so I work on it! If not, then the release fixed it and it was the same issue as the isolateWorkers issue.

Let me know.

[sheremet-va]

Still the same

[Aslemammad]

Ok, I'll put the time, hope I can get you a proper result soon!

[purepear]

Hey 👋
Just FYI, I've update https://github.com/purepear/vitest-element-plus-debug to Vitest 0.28.3 and updated the code and README with the latest finding from this thread vitest-dev/vitest#579

[Aslemammad]

@purepear Hey, Thank you so much for your work!
Just a couple of questions, do you think [email protected] improved anything in Vitest? And how do you think we can solve this issue and improve performance using tinypool? Let me know.

[purepear]

Hey @Aslemammad, I noticed some improvements in Vitest 0.28.3 but I can't tell if they are directly related to [email protected]. I put some results in this comment.

Regarding this issue, there are 2 things that I noticed:

• When running tests in jsdom environment, there is some overhead. I assume there's a cost to import/initialize jsdom for each worked.
• But jsdom aside, I ran a simple node environment test 100 times (npm run vitest-calc in the repo mentioned above) and logged the time in each test file. I looked for the delay after the first batch of tests (as @sheremet-va mentioned) and I did notice a delay but it was not just after the first batch - it was after each batch (batch = threads number). The only question I can think of is, how much delay are we expecting when starting each worker(~50ms?). In my results it's about ~200ms.

Here are example results from the first couple of batches of a test run (9 threads in my case).
I converted the logged timestamps to milliseconds relative to the first logged time.
I also split the obvious worker delay with new lines.

0
2
0
7
9
15
15
15
19

198
201
205
210
213
214
216
223
247

394
400
402
407
409
412
426
431
451

622
622
629
629
637
646
658
659
684

[Aslemammad]

Sure, I see, I do not know what to expect around worker initialization since it is relative, but I'll take a look.

Let me know if you want to work on this issue, since you are doing most of the work, and thank you for that.

[purepear]

Now that I'm reading the original issue again, I think this might actually be an issue that I can't replicate locally. @sheremet-va mentioned "workers don't start for a few seconds after the first four are finished" which might be sth specific to his setup ("few seconds" sounds like a noticeable thing).

Aside from that, I'll try to do some investigation into worker initialisation time in the following weeks when I have time.

Thanks for the incredible work you are doing! 🙇

[Aslemammad]

@purepear Thank you so much, sure, feel free to do so.

[Aslemammad]

After experimenting for a few days, I found out that for the element case, the imports are taking most of the time, which is on the test side and not on the vitest or tinypool side.

Second, around the batches, the reason is that the calc test cases have the same speed (since they're the same), so initialization and destroying for all workers, happen at the same time so they all start and end roughly a few ms before and after.

If we do something like this, we see that workers won't wait for each other, and some will start when the worker is done.

test('sum works', async () => {
  await new Promise((resolve) => setTimeout(resolve, (process.__tinypool_state__?.workerId) * 1000));
  expect(sum(1, 2)).toBe(3)
})

For instance, here we'd have two workers running with workerId 1, one with workerId 2, and one with workerId 3 in the first 3-4 seconds, and the same formula goes with rest.

So the only issue is the initialization cost which is not something that tinypool can handle. Still, one way to improve performance in vitest is to remove imports in the worker file or even delay them or condition them.

I'll let this issue open for a while in case someone finds a solution in tinypool! Let me know your opinions.

[AriPerkkio]

I think this is fixed by nodejs/node#51526. Earlier Node had randomly issues when terminating Workers. Tinypool starts new workers only after the previous ones have closed so that max thread count is not exceeded. The root cause for this reported issue could very well be the linked Node issue.

Update your Node to >=20.12.0 and see if the issues still persists. If it does, feel free to open new issue.