module: have a single hooks thread for all workers#52706
Merged
nodejs-github-bot merged 13 commits intoMay 8, 2024
Conversation
Collaborator
|
Review requested:
|
1025b03 to
fa24d02
Compare
fa24d02 to
ee16c17
Compare
Member
|
@nodejs/loaders |
GeoffreyBooth
approved these changes
Apr 26, 2024
03f69a8 to
2fdf7b3
Compare
JakobJingleheimer
approved these changes
Apr 27, 2024
Member
JakobJingleheimer
left a comment
There was a problem hiding this comment.
Yahoo! 🙌 great work @dygabo. thank you for this!
a few nits, and a couple small tweaks requested.
aduh95
reviewed
Apr 27, 2024
aduh95
reviewed
Apr 27, 2024
aduh95
reviewed
Apr 27, 2024
aduh95
reviewed
Apr 27, 2024
aduh95
approved these changes
Apr 27, 2024
Member
Author
|
I added a testcase for a scenario where a worker thread import operation would call process.exit. I.e.
I think this is a change in behavior compared to the previous version. Is this scenario covered correctly now? Is this an acceptable change in behavior? |
bb59bc8 to
7f86cde
Compare
This implements reusing the single esm loader hooks thread for all the subsequent workers.
It started as a trial to solve the test contributed by @GeoffreyBooth from #50752. The test content is taken over from that branch and only slightly modified to also support transitive worker threads (that are started by some worker and not by the main thread). This is probably a not very common use case but the interface supports it so I put in the test as well.
With this initial commit all tests are passing.
Let's start a discussion and if there are additional usages that are missed by the implementation or if something needs improvements, let me know. If the implementation matches the expectations, we could close the test-only PR and continue the discussion here. Let me know your thoughts.