Perf regression in Node 22/24 when loading JS files

Version

v22.21.0

Platform

any

What steps will reproduce the bug?

There is a performance regression when loading JS files from a location with latency (e.g. network path).

Unfortunately I don't know how to provide a sample to reproduce it in other conditions than my own environment, BUT, I think I identified the root cause in the NodeJS code (see below).

How often does it reproduce? Is there a required condition?

The issue is not present in:

v20.19.5
v21.4.0

The issue is partially fixed in:

v22.18.0
v22.19.0

It is present in:

v21.5.0
v22.21.0
v24.10.0

What is the expected behavior? Why is that the expected behavior?

The expected behavior is to load files as fast as in v20.x.

What do you see instead?

Files take up to 4.5 times slower to load (in my environment).

Additional information

I cloned the repo and tested each commit to reach the following conclusion:

The regression was introduced in v21.5.0 by src: move package_json_reader cache to c++ #50322.
A first fix in v22.19.0 (src: add cache to nearest parent package json #59086) fixed the issue partially but introduced an increase of memory consumption.
A second fix in v22.21.0 (src: reduce the nearest parent package JSON cache size #59888) resolved the memory issue but reintroduced the performance regression.

I went through the code of each PR and here is my understanding:

The initial change src: move package_json_reader cache to c++ #50322 moved the search of a package.json file from the JS side to the C++ side. I'm not a C++ dev but I believe a part of the previous algorithm was missed and now the cache only applies to the content of the package.json but not to the file system search. So when walking up the file tree, each possible location for the package.json is tested again and again. It becomes particularly painful when the package.json is located high up the file tree.That would very well explain why the performance degraded when the file system operations are costly, and was not seen in a purely local context.
The first fix reintroduced caching on the JS side (which probably defeats at least partially the purpose of the initial change), but it duplicated the content of the package.json for each starting path location, thus increasing the consumed memory significantly.
The second fix reintroduced the loop over the file system search on the JS side (which is kind of funny because it completely rolls back the initial change) while still not caching the results of the previous operations, thus bringing back the performance regression.

I tested the following change in the method findParentPackageJSON of lib\internal\modules\package_json_reader.js and it does resolve the performance issue:

const packageJSONLocationCache = new SafeMap();
// [...]
const maybePackageJSONPath = checkPath + path.sep + 'package.json';
if (packageJSONLocationCache.has(maybePackageJSONPath)) {
  if (packageJSONLocationCache.get(maybePackageJSONPath)) {
    return maybePackageJSONPath
  } else {
    continue
  }
}
const stat = internalFsBinding.internalModuleStat(checkPath + path.sep + 'package.json');

const packageJSONExists = stat === 0;
packageJSONLocationCache.set(maybePackageJSONPath, packageJSONExists)
if (packageJSONExists) {
  return maybePackageJSONPath;
}

However I'm not sure this should be merged. If the intent of the initial change was to bring all that logic to the C++ side for performance reasons, it might be preferable to rollback all the changes done on the JS side and to fix the caching on the C++ side.

Unfortunately, I'm a JS/TS dev and I failed miserably to fix the C++ code 😢

@Tyoneb

@Tyoneb Do you have a self-contained reproduction that demonstrates the regression? I'm the author of the third PR you linked there. I considered adding essentially the same cache you suggest at the time, but it didn't really move the needle on any of the specific tests I was using. My understanding of the performance regression in 1 (addressed by the first cache in 2) was that it was about C++ / JS FFI boundary-crossing, not filesystem operations.

Hey, thanks for your quick feedback!

Do you have a self-contained reproduction that demonstrates the regression? I'm the author of the third PR you linked there. I considered adding essentially the same cache you suggest at the time, but it didn't really move the needle on any of the specific tests I was using.

I guess that it depends on the setup. If you load everything from your local disk, the file system operations generally don't cost much, but when you do a simple stat across the network (which is my use case), the impact can be quite significant.

I don't know how to force delayed FS operations to simulate a network latency, which is why I did not provide a reproduction sample. Maybe it could be simulated by having a large and deep folder structure with many different files, with a single package.json at its root, but I haven't tried.

My understanding of the performance regression in 1 (addressed by the first cache in 2) was that it was about C++ / JS FFI boundary-crossing, not filesystem operations.

I'm not so sure about that analysis. The main difference I see is that the initial change moved the whole filetree walk and checks for package.json to the C++ side, instead of looping and caching the results on the JS side. I would say that the boundary-crossing happened only once per file, where the current implementation loops on the JS side and crosses to the C++ side every time it checks for the existence of a package.json. So it seems to me that the change made sense and reduced the boundary-crossing. But the fact that the results of the FS operations were not cached (only the read+parse) introduced the regression compared to the initial implementation. However I haven't been able to fix the C++ caching to validate my hypothesis, which is why I opened this issue.

For the sake of adding a bit more details, here is an example with a simple folder structure:

A
├─── B
│    └─── C
│         └─── C1.js
│    └─── D
│         └─── D1.js
└─── package.json

When you load C1.js, the algorithm will perform a fs.stat (or the equivalent on C++ side):

/A/B/C/package.json → nope
/A/B/package.json → nope
/A/package.json → found! Let's read and parse its content.

Then when you load D1.js, it will follow the same pattern:

/A/B/D/package.json → nope
/A/B/package.json → this check has already been performed, there is no need to call the FS again, let's read its results from the cache
/A/package.json → this check has already been performed and the file was found and parsed, let's retrieve its content from the second cache

Even if the file system operations are not particularly slow, they're still more costly than a simple Map#get and so caching their results do make sense in my opinion (even if the cache implies a slight increase of memory consumption).

@Tyoneb

@Tyoneb Oh gotcha, your original message mentioned loading from a network path; I should have read it more carefully!

My understanding of the performance regression in 1 (addressed by the first cache in 2) was that it was about C++ / JS FFI boundary-crossing, not filesystem operations.

I'm not so sure about that analysis. The main difference I see is that the initial change moved the whole filetree walk and checks for package.json to the C++ side, instead of looping and caching the results on the JS side.

It also adopted simdjson for parsing package.json, which I understood to be the primary intent of that change, but I'm not totally sure! I was mostly relying on the stated reasoning in #59086

I agree with your analysis overall, though! To restate it slightly differently in case it's helpful for other readers:

The implementation prior to src: move package_json_reader cache to c++ #50322 cached the result of read. If package.json existed at that path, it would cache the contents. Importantly, if package.json didn't exist at that path, it would also implicitly cache the failed call to open at that path, which is why it performs fine if filesystem operations are slow. The C++-side cache introduced in that change doesn't cache a negative result like the JS-side cache did, which is the first regression.
My change in src: reduce the nearest parent package JSON cache size #59888 introduces a bunch of uncached stat calls

We probably could introduce that second cache like you suggest. That's what I tested originally, but wasn't a meaningful improvement in my testing (admittedly with a fast local disk). Alternatively, I wonder if what was reported in #58126 and addressed in #59086 as boundary crossing overhead is actually just about the original set of uncached filesystem operations. That would explain why I wasn't personally able to reproduce that regression on versions of Node that had it. If that's true, maybe it's better to unwind all of this and just fix the caching in C++. I'm in a similar boat to you where I'm a lot more comfortable in JS than I am in C++ though!

Here's a draft of a change that I think may handle this in C++ reasonably, if it turns out that the boundary-crossing overhead described in #59086 isn't an issue: #60425

@michaelsmithxyz

I've tested locally the fix from #60425 in the v22.x branch and I confirm that the performance issue is resolved! Thanks @michaelsmithxyz!

As for the discussion around the boundary-crossing, the measures in my environment show the following:

v20.19.5 → 1.273s
v21.4.0 → 657.488ms (I believe the perf improvement comes from the V8 upgrade)
v21.5.0 → 3.951s (this version contains the change from #50322)
v22.21.1-pre → 4.012s (latest v22 at the time I ran the test)
v22.21.1-pre (with fix from #60425) → 904.5ms

The values don't really matter since I can't share a reproducible sample, but I'm posting them just to compare.

In the end, it seems that the change from #50322 does have a negative performance impact, at least in some cases. Since the author of the PR claimed that it boosted the perf in their benchmark regarding vite --version, I don't know what to conclude.

Tagging @anonrig in case you have an opinion on the matter!

I'll take a detailed look but #59888 is a significant regression that defeats the purpose of my original PR - make package json finding a single C++ call.

Please tag me on pull requests. Happy to review.

I'll take a more in depth look soon.

@Tyoneb

@Tyoneb can you share your code/with similar directory structure so I can reproduce it on my own.

@anonrig

I think there may be multiple things going on here, which might be why this has maybe been a bit of a moving target.

The first, which is what this issue is about, is about performance when we're using a slow filesystem. I was able to reproduce this by just mounting a local directory as an NFS mount (from localhost) with noac and running the benchmarks in #59888. The story here is (this is discussed above but to recap again):

src: move package_json_reader cache to c++ #50322 moved the package.json resolution and JSON parsing to C++, including the cache for parsed package.json objects, but in doing so changed the cache behavior subtly such that the case where BindingData::GetPackageJSON is called with a path that doesn't exist is not cached. This means that when BindingData::TraverseParent loops and tries identical paths that don't exist repeatedly, we attempt to open and read those same paths repeatedly, which degrades performance, particularly on slow filesystems.
src: add cache to nearest parent package json #59086 resolved this by caching by checkPath in getNearestParentPackageJSON on the JS side. This was a big regression in memory usage a lot of the time, because distinct modules often share a package.json, but it avoids the repeated filesystem walk for multiple calls with the same checkPath
src: reduce the nearest parent package JSON cache size #59888 resolves the memory usage regression but does so by reintroducing essentially the same uncached filesystem traversal regression that was there originally

The other factor that I think affects performance differences we're observing across the changes, particularly on fast filesystems, is the lazy parsing of imports and exports on package config objects in package_json_reader.js. There are a few things going on here:

I don't think the attempt to lazily parse and memoize imports and exports on demand works as expected because the getters are set on an object that's immediately spread, meaning we just immediately run them both anyway: https://github.com/nodejs/node/blob/main/lib/internal/modules/package_json_reader.js#L79
The memoization there only works if a given package.json file maps to the same package config instance every time. Of the three changes listed above, this is only true in src: reduce the nearest parent package JSON cache size #59888 which I think is why I observed such a significant improvement in the "require date-fns in a loop" benchmark. On other versions, we parse the same data repeatedly.

I pushed a change to #60425 that attempts to work around this by adding weird two-level caching to package_json_reader.js. It works, but it feels kind of gross. I don't think we should have to call the C++ GetNearestParentPackageJSON if we're going to just throw the result away and use the instance we already have, but it at least validates the idea. I don't see a super obvious way to do this better other than maybe dropping the lazy parsing idea and parsing exports and imports in C++. @anonrig if you have thoughts though, I'd love to hear them!

@anonrig

Hi @anonrig, have you been able to take a look at #60425? I believe it's the right approach as it reverts changes on the JS side and simply adds the necessary caching on the C++ side to reduce the number of FS operations.

I'd really like to get this issue fixed soon because there is no Node 22.x version that's really usable in my environment right now☹️.

@Tyoneb

@Tyoneb yes, i think it's the right approach

test(cli): raise lifecycle suite timeout to 60s for Windows Node 22 CI

Navigation Menu

Repository navigation

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

Version

Platform

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior? Why is that the expected behavior?

What do you see instead?

Additional information

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Version

Platform

What steps will reproduce the bug?

How often does it reproduce? Is there a required condition?

What is the expected behavior? Why is that the expected behavior?

What do you see instead?

Additional information

Activity

michaelsmithxyz commented on Oct 25, 2025

Tyoneb commented on Oct 25, 2025

michaelsmithxyz commented on Oct 25, 2025

michaelsmithxyz commented on Oct 27, 2025

Tyoneb commented on Oct 28, 2025

anonrig commented on Oct 28, 2025

anonrig commented on Oct 28, 2025

michaelsmithxyz commented on Oct 29, 2025

Tyoneb commented on Nov 3, 2025

anonrig commented on Nov 3, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions