perf(pacquet): cache per-wanted resolves in resolve_dependency_tree#11874
Merged
Conversation
Memoise `resolver.resolve()` and `extract_children()` on the tree-walk context. The first visit for a given `(alias, bare_specifier, optional)` wanted dep runs the resolver chain; later visits clone the cached `Arc<ResolveResult>` and skip `pick_package`'s cache-key formatting, HashMap probing, and semver matching. The first visit for a given `pkgIdWithPatchHash` walks the manifest for child specs; later visits clone the cached `Arc<Vec<ChildSpec>>` and skip the JSON traversal. Per-occurrence `NodeId`s and the cycle-break gate are unchanged, so peer-suffix variants for non-leaf packages stay intact. Mirrors pnpm's `resolveDependencies.ts:1584` `isNew` gate at the wanted-dep edge, which is the layer pacquet's recursion shape can short-circuit without restructuring the tree builder. Refs #11869.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds two Mutex-backed memo caches to TreeCtx: one memoizes Resolver::resolve results keyed by WantedKey, and the other caches extracted child dependency specs keyed by resolved package id; resolve_node uses both caches to avoid redundant resolver calls and child extractions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Micro-Benchmark ResultsLinux |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11874 +/- ##
==========================================
- Coverage 87.81% 87.81% -0.01%
==========================================
Files 205 205
Lines 24429 24435 +6
==========================================
+ Hits 21453 21457 +4
- Misses 2976 2978 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Integrated-Benchmark Report (Linux)Scenario: Frozen Lockfile
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.4525804598199996,
"stddev": 0.08995893406971224,
"median": 2.45816036902,
"user": 2.7514707599999997,
"system": 3.6400737999999997,
"min": 2.33235514402,
"max": 2.5681222340199996,
"times": [
2.4914863390199997,
2.5681222340199996,
2.3576718470199998,
2.5265824060199997,
2.35117401702,
2.39632288302,
2.42483439902,
2.33235514402,
2.53695559802,
2.5402997310199997
]
},
{
"command": "pacquet@main",
"mean": 2.41997151112,
"stddev": 0.0693732560628643,
"median": 2.39517302952,
"user": 2.7359202600000003,
"system": 3.6129360999999998,
"min": 2.36402228702,
"max": 2.5843067290199997,
"times": [
2.41464300102,
2.5843067290199997,
2.4525679020199997,
2.36402228702,
2.47260529802,
2.37422391902,
2.42385498702,
2.36497132502,
2.37570305802,
2.3728166050199997
]
},
{
"command": "pnpm",
"mean": 4.86786562212,
"stddev": 0.04867721206966998,
"median": 4.873290538519999,
"user": 8.22101646,
"system": 4.2458936,
"min": 4.78558162302,
"max": 4.94421742802,
"times": [
4.87286631702,
4.83291016302,
4.81735516702,
4.78558162302,
4.93493297302,
4.88349793302,
4.94421742802,
4.87538822802,
4.8737147600199995,
4.85819162902
]
}
]
}Scenario: Frozen Lockfile (Hot Cache)
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6775870665600001,
"stddev": 0.06282494608694471,
"median": 0.66084928646,
"user": 0.3848003399999999,
"system": 1.44082258,
"min": 0.64258224796,
"max": 0.85355834096,
"times": [
0.85355834096,
0.66795003296,
0.64971963896,
0.64743611596,
0.64809340996,
0.67774275696,
0.65834053196,
0.64258224796,
0.66335804096,
0.66708954896
]
},
{
"command": "pacquet@main",
"mean": 0.68745216286,
"stddev": 0.052139933568795906,
"median": 0.6646314664599999,
"user": 0.37911204,
"system": 1.46413518,
"min": 0.64836906196,
"max": 0.81132452396,
"times": [
0.74913326496,
0.66972587596,
0.65884282896,
0.65648559896,
0.68944233996,
0.66352303396,
0.66193520096,
0.64836906196,
0.81132452396,
0.66573989896
]
},
{
"command": "pnpm",
"mean": 2.72415558556,
"stddev": 0.07189148101839421,
"median": 2.72683385646,
"user": 3.303517539999999,
"system": 2.26407568,
"min": 2.5930973219599998,
"max": 2.86647064996,
"times": [
2.5930973219599998,
2.7251423089599998,
2.75829019596,
2.71176412296,
2.72852540396,
2.6422543629599997,
2.74934726596,
2.74419448196,
2.86647064996,
2.72246974096
]
}
]
}Scenario: Clean Install
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 5.04013150766,
"stddev": 0.13594233625965893,
"median": 5.03629419576,
"user": 6.7318084,
"system": 3.63209512,
"min": 4.82220692676,
"max": 5.27490736576,
"times": [
5.07435189876,
4.90698154776,
5.16964786676,
5.09093831476,
4.93915919476,
4.82220692676,
5.27490736576,
4.98335387676,
4.99823649276,
5.14153159176
]
},
{
"command": "pacquet@main",
"mean": 8.06240976466,
"stddev": 0.16646805538267123,
"median": 7.9836324017599996,
"user": 9.916978899999998,
"system": 3.5995423200000003,
"min": 7.89831855676,
"max": 8.32231873976,
"times": [
7.92757358976,
8.04008714276,
7.89831855676,
7.93546738076,
7.99486833176,
7.96783004876,
7.97239647176,
8.32231873976,
8.254879237759999,
8.31035814676
]
},
{
"command": "pnpm",
"mean": 6.76019949736,
"stddev": 0.11916698101214605,
"median": 6.76753108226,
"user": 11.0068547,
"system": 4.53153492,
"min": 6.57760223176,
"max": 6.96395388176,
"times": [
6.72916047776,
6.80695532176,
6.66251914476,
6.96395388176,
6.80774396676,
6.80590168676,
6.89805921376,
6.70971096276,
6.64038808576,
6.57760223176
]
}
]
}Scenario: Full Resolution
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 3.8679234050999995,
"stddev": 0.1357184294508379,
"median": 3.9000855666999996,
"user": 4.26757532,
"system": 2.1670843399999997,
"min": 3.6487417462,
"max": 4.0141400852,
"times": [
3.6963646291999996,
3.7773386382,
3.9734486932,
3.7855487082,
3.9890203532,
3.9944600642,
3.9635344232,
3.6487417462,
3.8366367101999996,
4.0141400852
]
},
{
"command": "pacquet@main",
"mean": 6.872818572499999,
"stddev": 0.09166698429525252,
"median": 6.8815033052,
"user": 7.33613512,
"system": 2.1367443399999995,
"min": 6.7345494242,
"max": 7.0094942391999995,
"times": [
6.9571458682,
6.8628484902,
6.9326353902,
6.8556373122,
6.9293306302,
6.8116096782,
6.9001581201999995,
6.7347765721999995,
6.7345494242,
7.0094942391999995
]
},
{
"command": "pnpm",
"mean": 4.4038914142,
"stddev": 0.1135758051974206,
"median": 4.3744588037,
"user": 5.5672006199999995,
"system": 2.5569048399999996,
"min": 4.2797037281999994,
"max": 4.6603969971999994,
"times": [
4.3952326012,
4.3260065412,
4.3536850062,
4.3026416682,
4.3354650622,
4.4856825752,
4.2797037281999994,
4.6603969971999994,
4.4371839982,
4.4629159642
]
}
]
} |
`pick_package` lives in `resolving-npm-resolver`, not in the deps-resolver crate, so the `[`pick_package`]` intra-doc links broke `cargo doc --document-private-items` under `-D warnings`. Drop the link form and keep the bare backticks.
This was referenced May 24, 2026
This was referenced Jun 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #11869.
Summary
resolve_nodeinresolve_dependency_tree.rsre-entered the resolver chain for every parent edge that reached a given package — on a typical npm graph (~1 300 unique packages with avg ~3 incoming paths), that's ~3× morepick_packagecalls than necessary. This PR memoises the two per-edge bits of work that don't depend on the visitor's ancestor chain:resolved_by_wanted: Mutex<HashMap<(alias, bare_specifier, optional), Arc<ResolveResult>>>— first caller for a given wanted dep runs the resolver chain; later callers clone theArcand skippick_package's cache-key formatting,HashMapprobing, and per-call semver matching.optionalis part of the key because the npm resolver toggles between abbreviated and full packument based onwanted.optional(pickPackage.ts:201), so a non-optional caller's abbreviated entry can't satisfy an optional one.children_specs_by_id: Mutex<HashMap<String, Arc<Vec<ChildSpec>>>>— first visit walks the manifest fordependencies+optionalDependenciestriples; later visits clone theArcand skip the JSON traversal.Per-occurrence
NodeIds and the cycle-break gate are unchanged, so peer-suffix variants for non-leaf packages stay intact. The change mirrors pnpm'sresolveDependencies.tsisNewgate at the wanted-dep edge — the layer pacquet's eager (non-lazy) tree builder can short-circuit without restructuring.Local benchmark numbers (macOS, virtual registry, alotta-files fixture)
integrated-benchmark --scenario=clean-install --runs=8 --warmup=1:pacquet@HEAD~1(#11867)pacquet@HEAD→
pacquet@HEADis 1.32 × faster (~24 % wall-time reduction).integrated-benchmark --scenario=full-resolution --runs=6 --warmup=1(warm cache, isolates the resolve phase):pacquet@HEAD~1pacquet@HEAD→ 1.19 × faster wall (~16 %), but the headline is −43 % user CPU — confirms the redundant resolver-chain work is what disappeared.
These are macOS-local numbers and noisier than CI; the issue's 40–60 % resolve-wall estimate referred to the ubuntu-latest profile. CI's integrated-benchmark workflow will produce the comparable numbers against
main.Why the conservative slice of the issue's proposal
#11869 sketched two caches:
resolved_subtree_node_id(collapses non-leafNodeIds across occurrences) andchildren_by_parent_id(caches the resolved child list). The issue itself flagged theNodeIdcollapse as risky because pacquet's peer resolver memoises byNodeIdand would lose per-occurrence peer-suffix variation if non-leaves collapsed — and pointed at the children-only cache as the fallback, "the win is still substantial because the recursive subtree walk is the dominant cost." This PR is that fallback shape, withresolved_by_wantedcovering the resolver-chain reentry side (analogous to pnpm'sresolvedPkgsByIdshort-circuit at the same call site) andchildren_specs_by_idcovering the children-extraction side. TheNodeIdcollapse remains an open follow-up if profiling on CI shows it's needed.Test plan
cargo nextest run -p pacquet-resolving-deps-resolver— 50 tests pass (diamond, cycle, peer-variant, patched, optional-propagation scenarios all green)cargo nextest run -p pacquet-package-manager— 298 tests passcargo clippy -p pacquet-resolving-deps-resolver --all-targets -- --deny warnings— cleanjust fmt— cleanclean-installandfull-resolutionscenarios — numbers abovemain— will post when CI publishesWritten by an agent (Claude Code, claude-opus-4-7).
Summary by CodeRabbit