src: move package_json_reader cache to c++ #50322

anonrig · Oct 21, 2023

Co-authored by @lemire

Local Benchmark

My local benchmarks says that running vite --version is 5% faster

❯ hyperfine 'node ../sveltejs-realworld/node_modules/vite/dist/node/cli.js --version' 'out/Release/node ../sveltejs-realworld/node_modules/vite/dist/node/cli.js --version' -w 10
Benchmark 1: node ../sveltejs-realworld/node_modules/vite/dist/node/cli.js --version
  Time (mean ± σ):     101.4 ms ±   0.6 ms    [User: 96.6 ms, System: 10.8 ms]
  Range (min … max):   100.3 ms … 102.5 ms    28 runs

Benchmark 2: out/Release/node ../sveltejs-realworld/node_modules/vite/dist/node/cli.js --version
  Time (mean ± σ):      96.3 ms ±   0.5 ms    [User: 90.9 ms, System: 10.1 ms]
  Range (min … max):    95.6 ms …  98.1 ms    30 runs

Summary
  out/Release/node ../sveltejs-realworld/node_modules/vite/dist/node/cli.js --version ran
    1.05 ± 0.01 times faster than node ../sveltejs-realworld/node_modules/vite/dist/node/cli.js --version

TODOs

Move modules functionality to node_modules.cc
- internalModuleReadJSON
Move caching of package_json_reader to C++
- Make node_modules.cc snapshottable
Update run_main.js to just avoid constructing the whole object for the entry point.
- We could even call containsModuleSyntax from C++
Move getPackageScopeConfig from package_config.js to C++ side, and cache it on node_modules.cc
Replace all packageJSONReader calls that looks for specific attribute to have equivalent C++ functions
Investigate if imports & exports JSONParse needs to be cached on JS

Caveats

ERR_INVALID_PACKAGE_CONFIG does not include JSON.Parse() error message anymore. This mainly because we're using simdjson to avoid JS allocation while parsing for performance reasons.
exports/imports without string, array or object value will now throw ERR_INVALID_PACKAGE_CONFIG instead of returning an invalid value.

Benchmarks

Startup bench: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1468

nodejs-github-bot · Oct 21, 2023

Review requested:

@nodejs/gyp
@nodejs/loaders
@nodejs/startup

src/node_modules.cc

PR-URL: nodejs#50322 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

Co-authored-by: Daniel Lemire <daniel@lemire.me> PR-URL: nodejs#50322 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

PR-URL: nodejs#50322 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

Co-authored-by: Daniel Lemire <daniel@lemire.me> PR-URL: nodejs#50322 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

PR-URL: nodejs#50322 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

PR-URL: #50322 Backport-PR-URL: #56590 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

Co-authored-by: Daniel Lemire <daniel@lemire.me> PR-URL: #50322 Backport-PR-URL: #56590 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

PR-URL: #50322 Backport-PR-URL: #56590 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

Notable changes: crypto: * update root certificates to NSS 3.104 (Richard Lau) #55681 deps: * (SEMVER-MINOR) add simdjson (Yagiz Nizipli) #50322 doc: * add LJHarb to collaborators (Jordan Harband) #56132 * enforce strict policy to semver-major releases (Rafael Gonzaga) #55732 * add jazelly to collaborators (Jason Zhang) #55531 module: * (SEMVER-MINOR) merge config with `package_json_reader` (Yagiz Nizipli) #50322 src: * (SEMVER-MINOR) move package resolver to c++ (Yagiz Nizipli) #50322 tools: * fix root certificate updater (Richard Lau) #55681 PR-URL: #56699

Notable changes: crypto: * update root certificates to NSS 3.104 (Richard Lau) #55681 deps: * (SEMVER-MINOR) add simdjson (Yagiz Nizipli) #50322 doc: * add LJHarb to collaborators (Jordan Harband) #56132 * enforce strict policy to semver-major releases (Rafael Gonzaga) #55732 * add jazelly to collaborators (Jason Zhang) #55531 esm: * mark import attributes and JSON module as stable (Nicolò Ribaudo) #55333 module: * (SEMVER-MINOR) merge config with `package_json_reader` (Yagiz Nizipli) #50322 src: * (SEMVER-MINOR) move package resolver to c++ (Yagiz Nizipli) #50322 tools: * fix root certificate updater (Richard Lau) #55681 PR-URL: #56699

PR-URL: #50322 Backport-PR-URL: #56590 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

Co-authored-by: Daniel Lemire <daniel@lemire.me> PR-URL: #50322 Backport-PR-URL: #56590 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

PR-URL: #50322 Backport-PR-URL: #56590 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

PR-URL: nodejs#50322 Backport-PR-URL: nodejs#56590 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

Co-authored-by: Daniel Lemire <daniel@lemire.me> PR-URL: nodejs#50322 Backport-PR-URL: nodejs#56590 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

PR-URL: nodejs#50322 Backport-PR-URL: nodejs#56590 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>

BridgeAR · May 2, 2025

@anonrig this seems to cause a major performance regression. I did not look deeper into this but it's causing test code to run about 10x slower when running the following example code: DataDog/dd-trace-js#5360

anonrig · May 2, 2025

@anonrig this seems to cause a major performance regression. I did not look deeper into this but it's causing test code to run about 10x slower when running the following example code: DataDog/dd-trace-js#5360

@BridgeAR Happy to take a look. Can you share a reproduction that doesn't require any external knowledge, and open a new issue and tag me?

BridgeAR · May 2, 2025

I can't look into it closer at the moment. Since about 80% of all time is taken in Node.js when running the example, I guess it should be fine to check the source code to cause the problem.
It is actually just a redirected issue report, due to originally being reported in aws/aws-cdk#33576.

I'll open a issue about it though.

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Oct 21, 2023

anonrig force-pushed the package-json-cache branch 8 times, most recently from e462554 to ba0be9b Compare October 22, 2023 00:34

GeoffreyBooth mentioned this pull request Oct 22, 2023

[DO NOT MERGE] test: duplicate async_hooks tests in esm #44323

Draft

anonrig force-pushed the package-json-cache branch 4 times, most recently from 5158e35 to 2690c64 Compare October 23, 2023 19:16

joyeecheung reviewed Oct 24, 2023

View reviewed changes

src/node_modules.cc Outdated Show resolved Hide resolved

anonrig force-pushed the package-json-cache branch 12 times, most recently from 2bfbcdb to 8309e95 Compare October 25, 2023 19:31

joyeecheung mentioned this pull request Jan 13, 2025

[v20.x backport] src: move package_json_reader cache to c++ #56590

Closed

marco-ippolito mentioned this pull request Jan 22, 2025

v20.18.3 proposal #56699

Merged

joyeecheung mentioned this pull request Jan 24, 2025

[v20.x] backport unflagging of require(esm), part 1 (of 4?) #56730

Closed

BridgeAR mentioned this pull request May 2, 2025

[BUG]: Severe performance degradation on Node 22 DataDog/dd-trace-js#5360

Open

BridgeAR mentioned this pull request May 2, 2025

Significant performance regression with C++ package_json_reader cache #58126

Closed

IlyasShabi mentioned this pull request Jul 16, 2025

src: add cache to nearest parent package json #59086

Merged

This was referenced Sep 8, 2025

Significant heap usage regression in Node 22.19.0 #59733

Closed

src: reduce the nearest parent package JSON cache size #59888

Merged

Tyoneb mentioned this pull request Oct 24, 2025

Perf regression in Node 22/24 when loading JS files #60397

Closed

michaelsmithxyz mentioned this pull request Oct 27, 2025

src: cache missing package.json files in the C++ package config cache #60425

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

src: move package_json_reader cache to c++ #50322

src: move package_json_reader cache to c++ #50322

Uh oh!

anonrig commented Oct 21, 2023 •

edited

Loading

Uh oh!

nodejs-github-bot commented Oct 21, 2023

Uh oh!

Uh oh!

BridgeAR commented May 2, 2025

Uh oh!

anonrig commented May 2, 2025 •

edited

Loading

Uh oh!

BridgeAR commented May 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

16 participants

Search code, repositories, users, issues, pull requests...

Uh oh!

src: move package_json_reader cache to c++ #50322

src: move package_json_reader cache to c++ #50322

Uh oh!

Conversation

anonrig commented Oct 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Local Benchmark

TODOs

Caveats

Benchmarks

Uh oh!

nodejs-github-bot commented Oct 21, 2023

Uh oh!

Uh oh!

BridgeAR commented May 2, 2025

Uh oh!

anonrig commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BridgeAR commented May 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

16 participants

anonrig commented Oct 21, 2023 •

edited

Loading

anonrig commented May 2, 2025 •

edited

Loading