Skip to content

module: avoid allocating a cache key string for every require() - #63884

Open
MarshallOfSound wants to merge 2 commits into
nodejs:mainfrom
MarshallOfSound:perf/cache-only
Open

MarshallOfSound wants to merge 2 commits into
nodejs:mainfrom
MarshallOfSound:perf/cache-only

Conversation

@MarshallOfSound

Copy link
Copy Markdown
Member

Description of Change

Every require() call β€” including fully cached ones β€” allocates a fresh `${parent.path}\x00${request}` string to key the relative-resolve fast path in Module._load. On cache hits that allocation and its GC churn are the largest per-call cost.

This PR splits the cache into two levels so hits do two allocation-free lookups instead: an outer SafeMap keyed by parent.path (a string the module system already retains, with a V8-cached hash) and an inner { __proto__: null } object keyed by the raw request.

  • The inner level is a plain object rather than a Map on purpose: property access internalizes the key, which handles dynamically built specifiers (require(base + name)) far better than Map.get's per-call hash-and-compare β€” and it preserves the string coercion the old template literal performed.
  • Invalidation is unchanged: stale hits delete the inner entry, failed loads clean up in the existing finally, and delete require.cache[...] + re-require behaves as before.
  • Memory is neutral (12.97 β†’ 12.91 MB heap after loading a 2,000-directory tree) since the concatenated keys are no longer retained.

Observable edge: parent.path is now an identity Map key rather than being string-coerced, and is read twice on the first load into a directory β€” only reachable if module.path is replaced with a non-string or an accessor, neither of which is documented usage.

benchmark/compare.js shows significant wins on every relevant module-loader.js config β€” cached +7.8…10.7%, cold +5.6…8.0% β€” with clean nulls on the circular/deep benchmarks where this path isn't hot.

Full benchmark results

10 samples per configuration, Welch t-test (*** p<0.001, ** p<0.01, * p<0.05):

module/module-loader.js cache='true'  n=1000 files=500 dir='abs' name=''            +10.72% ***
module/module-loader.js cache='true'  n=1000 files=500 dir='abs' name='/'            +8.08% ***
module/module-loader.js cache='true'  n=1000 files=500 dir='abs' name='/index.js'    +7.79% ***
module/module-loader.js cache='true'  n=1000 files=500 dir='rel' name=''            +10.30% ***
module/module-loader.js cache='true'  n=1000 files=500 dir='rel' name='/'           +10.20% ***
module/module-loader.js cache='true'  n=1000 files=500 dir='rel' name='/index.js'    +9.24% ***
module/module-loader.js cache='false' n=1000 files=500 dir='abs' name=''             +6.20% ***
module/module-loader.js cache='false' n=1000 files=500 dir='abs' name='/'            +6.18% ***
module/module-loader.js cache='false' n=1000 files=500 dir='abs' name='/index.js'    +7.89% **
module/module-loader.js cache='false' n=1000 files=500 dir='rel' name=''             +8.01% ***
module/module-loader.js cache='false' n=1000 files=500 dir='rel' name='/'            +5.61% *
module/module-loader.js cache='false' n=1000 files=500 dir='rel' name='/index.js'    +6.79% ***
module/module-require.js n=10000 type='.js'                                          +3.06% **
module/module-require.js n=10000 type='.json'                                        +2.67% ***
module/module-require.js n=10000 type='dir'                                          +0.02%
module/module-loader-circular.js n=1000                                              +0.22%
module/module-loader-deep.js cache='true' files=1000 (both exts)                  Β±1.9% (n.s.)

Ad-hoc workload-shape microbenchmarks land between βˆ’9% (300 dynamic-specifier children) and βˆ’58% (cached-require hot loop) load-phase time, neutral on cold loads β€” scripts and fixture generators: https://gist.github.com/MarshallOfSound/ad35b07ad5ebf9c5fec8e4daa4ec04b4

Checklist

  • I have built and tested this change
  • test/parallel/test-module-*, test-require-*, test-cjs-* pass
  • Manually verified cache-hit identity, delete require.cache + re-require, failed-load retry, circular requires, require.resolve with paths

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-bot nodejs-github-bot added module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. labels Jun 12, 2026
The relative resolve cache was keyed by a concatenated
${parent.path}\x00${request} string, allocating a new key for every
require() call including fully cached ones. Key the cache by the
parent directory first (a Map keyed by the already-retained
module.path string) and then by the request (a dictionary object,
whose property access internalizes dynamically-constructed
specifiers). Faster on every measured workload shape and slightly
smaller in memory, since the concatenated keys are no longer
retained.

Signed-off-by: Sam Attard <sattard@anthropic.com>
@MarshallOfSound
MarshallOfSound marked this pull request as ready for review June 12, 2026 18:23

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Comment thread lib/internal/modules/cjs/loader.js Outdated
Comment thread lib/internal/modules/cjs/loader.js Outdated
Only create the per-directory relative resolve cache bucket once
there is a resolved filename to store in it, so builtin loads and
failed resolutions no longer leave empty buckets behind. Read
parent.path once per Module._load() call.

Signed-off-by: Sam Attard <sattard@anthropic.com>
Assisted-by: Claude
@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.
βœ… Project coverage is 90.33%. Comparing base (141a504) to head (a50d67a).
⚠️ Report is 1601 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/modules/cjs/loader.js 96.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main   #63884       +/-   ##
===========================================
- Coverage   92.00%   90.33%    -1.67%     
===========================================
  Files         379      789      +410     
  Lines      166972   273294   +106322     
  Branches    25569    52227    +26658     
===========================================
+ Hits       153621   246889    +93268     
- Misses      13057    16862     +3805     
- Partials      294     9543     +9249     
Files with missing lines Coverage Ξ”
lib/internal/modules/cjs/loader.js 97.99% <96.00%> (+18.27%) ⬆️

... and 667 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants