node icon indicating copy to clipboard operation
node copied to clipboard

Do not drain worker tasks on main thread

Open dinfuehr opened this issue 1 year ago • 11 comments

Hi!

This PR stops draining worker tasks on the main thread. I believe it's not necessary since V8 will join its own worker tasks as part of v8::Isolate::Dispose. It may also cause deadlocks now that those worker tasks may ask the main thread to perform a GC, see the discussion in this V8 issue here. This should help enable concurrent sparkplug again, which will get/is disabled with this PR.

Do you know why Node is draining worker tasks here? Am I seeing right that NodePlatform::DrainTasks is also invoked from the event loop here? If so, this might be a problem for performance as we block the main thread until those worker tasks are finished when the event loop is empty.

I have to admit that I didn't run tests yet for that PR and it may require V8 patches as well (see the V8 issue linked above) but I opened the PR already to discuss this.

dinfuehr avatar Apr 06 '23 13:04 dinfuehr