Skip to content

perf(cache): memoize package.json dep path per CachedPath#250

Merged
stormslowly merged 2 commits into
mainfrom
worktree-opt_find_package_json
May 28, 2026
Merged

perf(cache): memoize package.json dep path per CachedPath#250
stormslowly merged 2 commits into
mainfrom
worktree-opt_find_package_json

Conversation

@stormslowly
Copy link
Copy Markdown
Collaborator

@stormslowly stormslowly commented May 28, 2026

Why

package_json() is called on every parent walked by find_package_json. In dep-tracking workloads (rspack), profiling shows ~97% of those calls resolve to None — there's no package.json at that level.

Each None cache-hit was paying, on every call, for the same data:

Step Cost per call
self.path.join("package.json") fresh PathBuf alloc
ResolverPath::from(PathBuf)Arc::from(...) second alloc + Arc header
hash_path(...) inside ResolverPath::new re-hash full absolute path bytes

Before / After (one cache-hit None push)

Before After
Allocations 2 (PathBuf + Arc<Path>) 0
Hashing full path hash 0
Hot ops join + Arc::from + hash_path + push OnceLock::get (atomic load) + Arc::clone + u64 copy + push

What

Add a lazy std::sync::OnceLock<ResolverPath> field on CachedPathImpl. The first None miss at each CachedPath builds the <dir>/package.json ResolverPath once and stores it; subsequent misses on the same CachedPath clone it.

No behavioral change — the ResolverPath pushed into ctx.missing_dependencies is byte-for-byte identical to the previous payload.

Note on CodSpeed memory-mode regression

A small memory-mode regression on the rspack-resolver microbench is expected. The new OnceLock<ResolverPath> slot adds a per-CachedPath allocation when populated, and the microbench does not init missing_dependencies, so the compensating savings on the hot path (eliminated join + Arc::from + hash_path per call) do not surface here. The net effect is positive in real dep-tracking workloads (e.g. rspack).

Test

  • cargo test --features __internal_bench --lib: 141 pass (6 pre-existing PnP failures from missing fixtures, same on main).
  • cargo test --test integration_test: 9/9 pass, including dependencies which exercises resolve_with_context.
  • Local rspack-resolver single-thread callgrind: program totals 8.056B → 8.062B Ir (noise; this microbench doesn't init missing_dependencies so the new path is dormant — the win surfaces in callers that do track deps).

In dep-tracking workloads, package_json() is called on every parent walked
by find_package_json, and ~97% of those resolve to None. Each None hit was
re-joining <dir>/package.json, re-allocating an Arc<Path>, and re-hashing
the path bytes inside ResolverPath::from before pushing to missing_dependencies.

Memoize the ResolverPath per CachedPathImpl via std::sync::OnceLock so the
subsequent pushes are just an atomic load + Arc::clone + u64 copy.
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 28, 2026

Merging this PR will degrade performance by 4.94%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 3 regressed benchmarks
✅ 7 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory resolver[[single-threaded]resolve with many extensions] 12.8 MB 13.4 MB -4.48%
Memory resolver[pnp resolve] 8.4 KB 8.7 KB -3.22%
Memory resolver[resolve from symlinks] 12 MB 12.9 MB -7.09%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing worktree-opt_find_package_json (796a32f) with main (080188f)

Open in CodSpeed

Use the existing `impl From<PathBuf> for ResolverPath` instead of the
explicit `ResolverPath::new(Arc::from(pb.into_boxed_path()))` chain.

Note: a CodSpeed memory-mode regression on the rspack-resolver
microbench is expected. The new `OnceLock<ResolverPath>` slot adds a
small per-`CachedPath` allocation when populated, and the microbench
does not init `missing_dependencies`, so the compensating savings on
the hot path (removed `join` + `Arc::from` + `hash_path` per call) do
not surface in this bench. The net effect is positive in real
dep-tracking workloads.
@stormslowly stormslowly marked this pull request as ready for review May 28, 2026 09:22
Copilot AI review requested due to automatic review settings May 28, 2026 09:22
@stormslowly
Copy link
Copy Markdown
Collaborator Author

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a per-CachedPath lazy memoization of the <dir>/package.json ResolverPath so that the hot missing_dependencies push triggered by every None result from package_json avoids re-doing PathBuf allocation, Arc<Path> allocation, and full-path hashing on each call. Behavior is unchanged; only the cost of repeated pushes for the same directory is reduced.

Changes:

  • Add package_json_dep_path: std::sync::OnceLock<ResolverPath> field to CachedPathImpl, initialized in new.
  • Add package_json_dep_path() helper that builds <self.path>/package.json once and clones it on subsequent calls.
  • Replace the two self.path.join("package.json") constructions in package_json's warm-hit None and cold-miss Ok(None) branches with the memoized helper.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@stormslowly stormslowly merged commit 87f8009 into main May 28, 2026
21 of 22 checks passed
@stormslowly stormslowly deleted the worktree-opt_find_package_json branch May 28, 2026 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants