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.

Sign up for GitHub

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

Jump to bottom

Tracking Issue: Syncify the ESM Loader #55782

Open
21 tasks
GeoffreyBooth opened this issue Nov 8, 2024 · 9 comments
Open
21 tasks

Tracking Issue: Syncify the ESM Loader #55782

GeoffreyBooth opened this issue Nov 8, 2024 · 9 comments
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders performance Issues and PRs related to the performance of Node.js.

Comments

@GeoffreyBooth
Copy link
Member

GeoffreyBooth commented Nov 8, 2024 •
edited by JakobJingleheimer
Loading

The code under lib/internal/modules/esm, a.k.a. the ESM loader, contains many functions that are async. We should refactor as many of these as possible, ideally all of them, to be synchronous. This should improve the performance of evaluating ESM code, bringing it roughly on par with the speed of running CommonJS code.

Longer term, once the ESM loader is synchronous and we land the synchronous module customization hooks, we could deprecate monkey-patching the CommonJS loader and merge together the CommonJS and ESM loaders, eliminating duplication: #50356.

This issue will track our progress syncifying the various files and functions of the ESM loader until we can get as much of it to be as synchronous as possible.

The files to be updated, all underlib/internal/modules:

  • run_main.js: asyncRunEntryPointWithESMLoader
  • esm/fetch_module.js: fetchWithRedirects
  • esm/fetch_module.js: isLocalAddress
  • esm/hooks.js: Hooks class (the async methods here probably don’t need updating as they will be removed once we migrate to the synchronous customization hooks)
  • esm/hooks.js: nextHookFactory
  • esm/load.js: getSource esm: syncify default path of ModuleLoader.load #57419
  • esm/load.js: defaultLoad esm: syncify default path of ModuleLoader.load #57419
  • esm/loader.js: ModuleLoader.eval
  • esm/loader.js: ModuleLoader.getModuleJobForImport
  • esm/loader.js: ModuleLoader.loadAndTranslate
  • esm/loader.js: ModuleLoader.import
  • esm/loader.js: ModuleLoader.load
  • esm/module_job.js: ModuleJob._link
  • esm/module_job.js: ModuleJob._instantiate
  • esm/module_job.js: ModuleJob.run
  • esm/module_job.js: ModuleJobSync.run
  • esm/translators.js: wasm handler, via translators.set('wasm', ...
  • esm/utils.js: importModuleDynamicallyCallback
  • esm/utils.js: initializeHooks (might not need updating as we will remove this once the synchronous customization hooks land
  • esm/worker.js: customizedModuleWorker (might not need updating as we will remove this once the synchronous customization hooks land
  • esm/worker.js: handleMessage (might not need updating as we will remove this once the synchronous customization hooks land

@nodejs/loaders @mcollina @JakobJingleheimer @joyeecheung
@GeoffreyBooth GeoffreyBooth added esm Issues and PRs related to the ECMAScript Modules implementation. performance Issues and PRs related to the performance of Node.js. labels Nov 8, 2024
@marco-ippolito
Copy link
Member

marco-ippolito commented Nov 8, 2024 •
edited
Loading

Maybe moving it in the loaders repo would be more appropriate since its a effort that is gonna require different iterations?

@mcollina
Copy link
Member

mcollina commented Nov 8, 2024

This task is better handled by a large group of people, and this repo is more visible. Once the first few steps are done, it can easily be tagged as "good first issue"

@avivkeller avivkeller added the loaders Issues and PRs related to ES module loaders label Nov 8, 2024
@nomagick
Copy link

I would like to mention my issue here.
Please check if it's worth fixing.

@cu8code
Copy link

cu8code commented Nov 16, 2024 •
edited
Loading

Hi @mcollina and @marco-ippolito,
Is anyone currently working on any of these issues? I’d love to help resolve a few. Could you recommend a good starting point? I’d like to begin with something low-risk—what would be the best issue to pick?

Also what is the best method to test if my code is improving performance?

@joyeecheung joyeecheung mentioned this issue Feb 18, 2025
Closed
@joyeecheung
Copy link
Member

This may be necessary to prevent a race condition like prettier/prettier#17139 (comment)

@joyeecheung joyeecheung mentioned this issue Feb 23, 2025
Merged
@GeoffreyBooth GeoffreyBooth mentioned this issue Mar 9, 2025
Closed
@GeoffreyBooth
Copy link
Member Author

@cu8code Sorry to not notice your comment earlier. If you'd like to help with this effort, you can pick any of the functions listed in the top issue and try to refactor them to be synchronous. The lowest hanging fruit are the ones that are async functions but never actually await anything (perhaps because they used to contain async logic but no longer do, for example). Besides prioritizing the easiest parts, I would also prioritize the hottest code paths: the functions involved in initial load of the most common use case like node app.js where app.js is an ES module. I would make the PRs as small as possible so that any resulting issues can be isolated as narrowly as possible, along the lines of #57390.

@JakobJingleheimer JakobJingleheimer mentioned this issue Mar 12, 2025
Open
@GeoffreyBooth GeoffreyBooth mentioned this issue Mar 14, 2025
Open
@marco-ippolito
Copy link
Member

  • esm/fetch_module.js: fetchWithRedirects

  • esm/fetch_module.js: isLocalAddress

Can be removed since we removed http imports

@ntindle

This comment has been minimized.

Sign in to view
@JakobJingleheimer

This comment has been minimized.

Sign in to view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders performance Issues and PRs related to the performance of Node.js.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@mcollina @GeoffreyBooth @nomagick @JakobJingleheimer @joyeecheung @ntindle @marco-ippolito @avivkeller @cu8code