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

2022-11-08 meeting notes #118

Merged
merged 1 commit into from
Nov 13, 2022
Merged

2022-11-08 meeting notes #118

merged 1 commit into from
Nov 13, 2022

Conversation

GeoffreyBooth
Copy link
Member

Closes #117

@GeoffreyBooth GeoffreyBooth added the documentation Improvements or additions to documentation label Nov 8, 2022
@GeoffreyBooth GeoffreyBooth merged commit 305e8da into main Nov 13, 2022
@GeoffreyBooth GeoffreyBooth deleted the 2022-11-08 branch November 13, 2022 15:05
@JakobJingleheimer
Copy link
Member

Sorry I missed this PR. I enabled notifications for any activity on this repo, but it's not doing it :(

Did we say it would be okay to run the default hooks without the dedicated thread (when no custom hooks exist)? I assume if custom hooks exist, the defaults would also run in the dedicated thread.

Did we also decide to make defaultResolve sync? Something about enabling import.meta.resolve() to then be sync, regardless of the threading work. Or was that just a re-cap of the previous PR.

@GeoffreyBooth
Copy link
Member Author

GeoffreyBooth commented Nov 15, 2022

As I remember it:

  • When no user loaders are specified, run the default (internal) hooks on the regular/main thread like they currently run now. defaultResolve is already sync; change the function definition to remove the unnecessary async keyword, and map import.meta.resolve to it.

  • When user loaders are specified, create a single thread within which we run both the user and internal loader hooks. Use Atomics.wait or other tricks to keep import.meta.resolve sync even as the resolve chain is now a series of async functions.

  • When user loaders are specified and user code spawns a worker thread, do whatever makes the most sense 😄 Probably keep the loaders in the same thread that you already spawned to accompany the main thread code, as then all the loaders would share a single context even as the app code is spread across multiple threads; but if we need one loaders thread per application code thread then that’s unfortunate but hopefully acceptable. But I think it’s better to avoid multiple threads for the loaders context if possible.

cc @mcollina

@JakobJingleheimer
Copy link
Member

Cool, that's what I thought too. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node.js Loaders Team Meeting 2022-11-08
3 participants