module: synchronously load most ES modules

[GeoffreyBooth]

Building on #55782, this PR uses the path @joyeecheung created for require(esm) to synchronously resolve and load all ES modules that lack top-level await, which is the vast majority of modules. The sync path is used when no async loader hooks, --import flags, or --inspect-brk are active; it falls back to the existing async path otherwise. Top-level await presence can only be determined after the module graph is instantiated, so if TLA is detected the already-instantiated graph falls back to async evaluation. In all cases the behavior is identical to the existing async path.

On current main, an ES module graph generates 14 + 5N promises for N modules; so 19 promises for a single module graph (one entry point that doesn’t import anything), 24 promises if that entry point imports one file, 29 promises for a three-module graph and so on.

In this PR, only one promise is created regardless of graph size: the low-level V8 module.evaluate() call that happens within module.evaluateSync(), where an immediately-resolved promise is created even for modules that don’t have top-level await. But still, it’s only one promise for an entire application, no matter how big the app is.

This PR adds a benchmark that focuses on the module loading flow that this PR improves:

                                              confidence improvement accuracy (*)   (**)  (***)
esm/startup-esm-graph.js n=100 modules='0250'                 0.71 %       ±3.39% ±4.47% ±5.74%
esm/startup-esm-graph.js n=100 modules='0500'                 0.45 %       ±3.15% ±4.15% ±5.33%
esm/startup-esm-graph.js n=100 modules='1000'                 1.96 %       ±3.19% ±4.21% ±5.40%
esm/startup-esm-graph.js n=100 modules='2000'                 1.08 %       ±3.12% ±4.11% ±5.28%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 4 comparisons, you can thus
expect the following amount of false-positive results:
  0.20 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.04 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

So basically it’s within the margin of error.

[nodejs-github-bot]

Review requested:

• @nodejs/loaders
• @nodejs/performance

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.41%. Comparing base (d7ea02d) to head (ed5d760).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #62530      +/-   ##
==========================================
+ Coverage   90.37%   90.41%   +0.03%     
==========================================
  Files         792      792              
  Lines      275683   275798     +115     
  Branches    52854    52875      +21     
==========================================
+ Hits       249147   249353     +206     
+ Misses      16947    16848      -99     
- Partials     9589     9597       +8     

Files with missing lines	Coverage Δ
lib/internal/modules/esm/loader.js	99.90% <100.00%> (+0.10%)	⬆️
lib/internal/modules/esm/module_job.js	99.58% <100.00%> (+0.31%)	⬆️
lib/internal/modules/run_main.js	100.00% <100.00%> (ø)

... and 35 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mcollina]

lgtm

[joyeecheung]

The PR description says there is an improvement but the number shows a regression?

Although I don't think "a flat graph importing hundreds/thousands of modules" is a representative use case, so a regression probably doesn't matter all that much anyway. A more typical graph probably consists of a lot of nodes each with a dozen or so imports..

[GeoffreyBooth]

The PR description says there is an improvement but the number shows a regression?

My apologies, I ran the benchmark where the new binary was built with --node-builtin-modules-path "$(pwd)", so it was running much slower due to loading the built-in modules from the current path. When I ran the benchmark again with a properly built binary, the results are nearly indistinguishable from main.

A more typical graph probably consists of a lot of nodes each with a dozen or so imports.

I updated the benchmark to create a tree with 10 imports per node, as large as necessary to match the desired size of the graph. I updated the PR description with the new results. Basically, they’re inconclusive, as you might expect for such a small change. Promises just don’t add much overhead.

[JakobJingleheimer]

LGTM and sounds right. Would be great to get 19 promises → 1 😁

[mcollina]

lgtm

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/73697/

[github-actions]

This pull request has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

[github-actions]

Benchmark GHA (esm / startup-esm-graph.js): https://github.com/nodejs/node/actions/runs/34718545834

Results

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.

Benchmark results:

                                             confidence improvement accuracy (*)    (**)   (***)
esm/startup-esm-graph.js n=30 modules='0250'                -0.29 %      ±10.83% ±14.27% ±18.31%
esm/startup-esm-graph.js n=30 modules='0500'                 0.29 %       ±8.96% ±11.81% ±15.16%
esm/startup-esm-graph.js n=30 modules='1000'                 0.66 %       ±7.71% ±10.16% ±13.04%
esm/startup-esm-graph.js n=30 modules='2000'                 1.63 %       ±7.79% ±10.27% ±13.18%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 4 comparisons, you can thus
expect the following amount of false-positive results:
  0.20 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.04 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.

[github-actions]

Benchmark GHA (esm / startup-esm-graph): https://github.com/nodejs/node/actions/runs/34828910699

Results

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.

Benchmark results:

                                             confidence improvement accuracy (*)    (**)   (***)
esm/startup-esm-graph.js n=30 modules='0250'                 0.98 %      ±11.94% ±15.74% ±20.19%
esm/startup-esm-graph.js n=30 modules='0500'                 0.94 %       ±9.85% ±12.98% ±16.66%
esm/startup-esm-graph.js n=30 modules='1000'                -0.62 %       ±8.88% ±11.71% ±15.02%
esm/startup-esm-graph.js n=30 modules='2000'                -0.63 %       ±9.41% ±12.40% ±15.91%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 4 comparisons, you can thus
expect the following amount of false-positive results:
  0.20 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.04 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.

[GeoffreyBooth]

@joyeecheung Do you want to take another look at this one?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/77988/

[panva]

https://ci.nodejs.org/job/node-test-commit-windows-fanned/80823/
test/es-module/test-esm-cjs-named-error.mjs

---
duration_ms: 248.0
exitcode: 1
severity: fail
stack: |-
  node:internal/modules/run_main:111
      triggerUncaughtException(
      ^

  AssertionError [ERR_ASSERTION]: entry point should show the CommonJS named export hint
      at file:///c:/workspace/node-test-binary-windows-js-suites/node/test/es-module/test-esm-cjs-named-error.mjs:84:8
      at process.processTicksAndRejections (node:internal/process/task_queues:104:5) {
    generatedMessage: false,
    code: 'ERR_ASSERTION',
    actual: false,
    expected: true,
    operator: '==',
    diff: 'simple'
  }

  Node.js v27.0.0-pre
...

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/78012/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/78016/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/78019/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/78022/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/78042/

[joyeecheung]

implementation LGTM, thanks, just some comments left for the tests.

[joyeecheung]

nit: I think if we return the callback result in asyncRunEntryPointWithESMLoader we can just return job.run()'s result here and we can cut the promise count further on the async path by simply returning the promise instead of awaiting on them.