perf: close the warm-resolve, symlink-churn, and download-concurrency gaps#12329
Conversation
Code Review by Qodo
1. Socket cap below concurrency
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details🔇 Additional comments (1)
📝 WalkthroughWalkthroughIncreases default network and lockfile verification concurrency bounds from 16–64 to 64–96 across pnpm and pacquet, implements a lazy non-hydrating ChangesDefault Network and Verification Concurrency Scaling
Symlink Normalization and Deprecation Detection
Sequence Diagram(s)sequenceDiagram
participant Picker as pick_package_from_meta
participant Versions as PackageVersions::is_deprecated
participant Store as package version fragments/cache
Picker->>Versions: query is_deprecated(version)
Versions->>Store: read hydrated cache or raw fragment text
Versions->>Versions: early-scan for "deprecated" substring
Versions->>Store: partial deserialize DeprecatedProbe (if needed)
Versions-->>Picker: boolean deprecated?
Picker-->>Picker: choose best candidate using deprecation flags
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 unit tests (beta)
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 |
PR Summary by QodoPerf: reduce warm-resolve CPU, avoid symlink churn, and raise download concurrency WalkthroughsDescription• Probe deprecated flags without hydrating full version manifests during resolve. • Reuse up-to-date relative symlinks by normalizing .. segments before comparing. • Raise default download/verification concurrency floors to better saturate fast registries. Diagramgraph TD
A["Resolver: publish-date filter"] --> B["Registry: PackageVersions"] --> C["Deprecated probe"]
D["FS: symlink reuse check"] --> E["lexical_normalize"]
F["pnpm: package-requester"] --> H["Default concurrency 64..96"]
G["pacquet: network client"] --> H
High-Level AssessmentThe following are alternative approaches to this PR: 1. Precompute deprecation flags at packument ingest time
2. Canonicalize symlink targets via filesystem resolution
3. Adaptive network concurrency based on observed latency/errors
Recommendation: Current approach is the best tradeoff for this PR’s goals: it removes known hot-path CPU (deprecation checks) and filesystem churn (relative symlink comparisons) without adding new I/O, and it raises concurrency in a simple, user-overridable way that aligns pnpm and pacquet defaults. Consider adaptive concurrency only if the higher floor triggers rate-limit complaints in the wild. File ChangesEnhancement (2)
Bug fix (1)
Refactor (1)
Tests (2)
Other (5)
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pacquet/crates/network/src/lib.rs`:
- Around line 594-603: Update the outdated docs for network_concurrency in the
config crate to match the new default range 64–96: replace the old formula
Math.min(64, Math.max(calcMaxWorkers() * 3, 16)) and any explanatory text with
Math.min(96, Math.max(calcMaxWorkers() * 3, 64)) and the accompanying
explanation (e.g., references to a 64 up-to-22-core floor, scaling beyond, and
96 cap). Edit the documentation block that describes network_concurrency /
calcMaxWorkers to use the new formula and wording so it aligns with the
implementation in network::network_concurrency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 824b3377-f2a5-4ec9-bc85-298b705f236a
📒 Files selected for processing (11)
.changeset/raise-default-network-concurrency.mdinstalling/deps-installer/src/install/verifyLockfileResolutions.tsinstalling/package-requester/src/packageRequester.tspacquet/crates/fs/src/symlink_dir.rspacquet/crates/fs/src/symlink_dir/tests.rspacquet/crates/lockfile-verification/src/verify_lockfile_resolutions.rspacquet/crates/network/src/lib.rspacquet/crates/registry/src/package_version.rspacquet/crates/registry/src/package_versions.rspacquet/crates/registry/src/package_versions/tests.rspacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Dylint
- GitHub Check: Doc
- GitHub Check: Code Coverage
- GitHub Check: Analyze (javascript)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use Standard Style with trailing commas, prefer functions over classes, declare functions after they are used (relying on hoisting), limit functions to no more than two or three arguments, and use a single options object for functions needing more parameters
Follow import order: standard libraries first, then external dependencies (sorted alphabetically), then relative imports
Do not write comments that restate what the code already says; rename variables, split helpers, or move checks to more obvious places instead
Do not repeat documentation at call sites that already lives on the callee; update the JSDoc once and let every call site benefit
Use JSDoc for the function's contract (preconditions, postconditions, edge cases, why the function exists), not for re-narrating the function body
Do not record past implementation shape, refactor history, or removed code in comments; use git log and git blame for that information instead
Write comments only when the reason for code is non-obvious, a hidden invariant exists, a workaround for a known bug is needed, or an exception to surrounding pattern is deliberate
Files:
installing/deps-installer/src/install/verifyLockfileResolutions.tsinstalling/package-requester/src/packageRequester.ts
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: Log emissions are part of matching pnpm — when porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(...), orstreamParser.write(...), mirror the call site, payload, and ordering so@pnpm/cli.default-reporterparses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plainStringor&strin Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too viaTryFrom<String>and/orFromStrand do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallibleFrom<String>constructor
If upstream TypeScript occasionally constructs a branded string without validation, exposefrom_str_uncheckedin Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization
Derive simple conversions for branded strings using#[derive(derive_more::From)]and#[derive(derive_more::Into)]instead of handwritingimplblocks; use manualimplonly when conversion needs custom logic
Model TypeScript string literal unions (like'auto' | 'always' | 'never') as Rustenums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like`${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide inCODE_STYLE_GUIDE.md— imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching,pipe-trait, error handling, test layout, and cloning ofArcandRc
Choose owned vs. borrowed parameters to minimize copies; widen to t...
Files:
pacquet/crates/lockfile-verification/src/verify_lockfile_resolutions.rspacquet/crates/fs/src/symlink_dir.rspacquet/crates/registry/src/package_version.rspacquet/crates/fs/src/symlink_dir/tests.rspacquet/crates/network/src/lib.rspacquet/crates/registry/src/package_versions/tests.rspacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rspacquet/crates/registry/src/package_versions.rs
🧠 Learnings (7)
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
installing/deps-installer/src/install/verifyLockfileResolutions.tsinstalling/package-requester/src/packageRequester.ts
📚 Learning: 2026-06-05T13:47:26.046Z
Learnt from: vsumner
Repo: pnpm/pnpm PR: 12190
File: installing/deps-installer/src/install/index.ts:2337-2343
Timestamp: 2026-06-05T13:47:26.046Z
Learning: In the pnpm/pnpm codebase, `PnpmError` automatically prefixes `err.code` with `ERR_PNPM_` when you pass a code that does not already start with `ERR_PNPM_` (it normalizes `this.code` via `code.startsWith('ERR_PNPM_') ? code : `ERR_PNPM_${code}``). Therefore, during code review you should NOT flag `new PnpmError(...)` call sites for passing a bare error code (e.g., `new PnpmError('FROZEN_STORE_INCOMPATIBLE_WITH_PNPR', ...)`); the resulting `err.code` will still be `ERR_PNPM_FROZEN_STORE_INCOMPATIBLE_WITH_PNPR`.
Applied to files:
installing/deps-installer/src/install/verifyLockfileResolutions.tsinstalling/package-requester/src/packageRequester.ts
📚 Learning: 2026-05-26T21:01:06.666Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11966
File: .changeset/require-tarball-integrity.md:6-6
Timestamp: 2026-05-26T21:01:06.666Z
Learning: In pnpm lockfile-related release notes/docs (especially changeset markdown), preserve URL hostnames exactly as they appear in pnpm-lock.yaml tarball resolution entries—keep hosts like `codeload.github.com`, `bitbucket.org`, and `gitlab.com` in lowercase. Do not “correct” them to title-case/preserve brand capitalization (e.g., LanguageTool rules like `GITHUB` capitalization) because these are literal URL fragments, not platform brand names.
Applied to files:
.changeset/raise-default-network-concurrency.md
📚 Learning: 2026-05-20T19:40:55.051Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11774
File: pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs:0-0
Timestamp: 2026-05-20T19:40:55.051Z
Learning: In the pacquet Rust code, ensure the semver implementation uses the `node-semver` crate (not `nodejs-semver`). `node-semver`’s public API does not include a `satisfies_with_prerelease`-style method; prerelease-tolerant matching should be implemented inline by first calling `Range::satisfies`, and when it rejects a prerelease version, retry matching against a stripped `MAJOR.MINOR.PATCH` base of the prerelease version.
Applied to files:
pacquet/crates/lockfile-verification/src/verify_lockfile_resolutions.rspacquet/crates/fs/src/symlink_dir.rspacquet/crates/registry/src/package_version.rspacquet/crates/fs/src/symlink_dir/tests.rspacquet/crates/network/src/lib.rspacquet/crates/registry/src/package_versions/tests.rspacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rspacquet/crates/registry/src/package_versions.rs
📚 Learning: 2026-05-22T00:08:44.646Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11837
File: pacquet/crates/resolving-npm-resolver/src/pick_package.rs:33-51
Timestamp: 2026-05-22T00:08:44.646Z
Learning: In the pnpm/pnpm repo’s pacquet Rust crates, do not flag Unicode ellipsis characters (U+2026, `…`) in Rust doc comments (`///` / `/** */`) as a lint violation. The pacquet crate’s `dylint.toml` only enables `perfectionist::derive_ordering`, and the Dylint `unicode-ellipsis` rule is not enabled for this project—so `…` in doc comments is an intentional, repo-consistent style.
Applied to files:
pacquet/crates/lockfile-verification/src/verify_lockfile_resolutions.rspacquet/crates/fs/src/symlink_dir.rspacquet/crates/registry/src/package_version.rspacquet/crates/fs/src/symlink_dir/tests.rspacquet/crates/network/src/lib.rspacquet/crates/registry/src/package_versions/tests.rspacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rspacquet/crates/registry/src/package_versions.rs
📚 Learning: 2026-05-20T23:07:58.444Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:07:58.444Z
Learning: When reviewing code in this pacquet Rust port, follow the upstream pnpm compatibility rule: only match pnpm’s behavior exactly. Do not propose review changes that intentionally deviate from pnpm’s documented/observed behavior, even if pnpm appears buggy. If you identify a real bug in pnpm behavior, the review should prioritize fixing it upstream in pnpm first, and avoid implementing a pnpm-behavior workaround here unless the same fix has already landed upstream.
Applied to files:
pacquet/crates/lockfile-verification/src/verify_lockfile_resolutions.rspacquet/crates/fs/src/symlink_dir.rspacquet/crates/registry/src/package_version.rspacquet/crates/fs/src/symlink_dir/tests.rspacquet/crates/network/src/lib.rspacquet/crates/registry/src/package_versions/tests.rspacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rspacquet/crates/registry/src/package_versions.rs
📚 Learning: 2026-06-06T18:58:37.156Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12243
File: pacquet/crates/package-manager/src/install_package_by_snapshot.rs:319-322
Timestamp: 2026-06-06T18:58:37.156Z
Learning: When reviewing Rust code, do not assume `matches!(expr, Pattern(_))` will move out of `expr` if `Pattern(_)` contains no by-value bindings. `matches!` desugars to a `match` that auto-borrows the scrutinee for discrimination, so even if `expr` is a non-`Copy` value behind a shared reference (e.g., `&T`), the macro should not move-out of the borrowed data purely due to `matches!`. Treat `matches!(&expr, Pattern(_))` as a readability/clarity improvement, not a correctness requirement. Only flag potential move-out-of-borrow risks when the pattern includes by-value bindings that would require moving the matched value.
Applied to files:
pacquet/crates/lockfile-verification/src/verify_lockfile_resolutions.rspacquet/crates/fs/src/symlink_dir.rspacquet/crates/registry/src/package_version.rspacquet/crates/fs/src/symlink_dir/tests.rspacquet/crates/network/src/lib.rspacquet/crates/registry/src/package_versions/tests.rspacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rspacquet/crates/registry/src/package_versions.rs
🔇 Additional comments (6)
pacquet/crates/registry/src/package_version.rs (1)
154-154: LGTM!pacquet/crates/registry/src/package_versions.rs (1)
27-27: LGTM!Also applies to: 29-36, 153-181
pacquet/crates/registry/src/package_versions/tests.rs (1)
130-185: LGTM!pacquet/crates/resolving-npm-resolver/src/pick_package_from_meta.rs (1)
30-30: LGTM!Also applies to: 349-355, 432-484
pacquet/crates/fs/src/symlink_dir.rs (1)
272-278: LGTM!Also applies to: 285-285
pacquet/crates/fs/src/symlink_dir/tests.rs (1)
213-244: LGTM!
…sion manifests With minimumReleaseAge active (the default), every range pick runs filter_pkg_metadata_by_publish_date, and any dist-tag pointing at a version outside the maturity cutoff repopulates by scanning all candidate versions, reading each candidate's deprecated flag. Reading that flag through PackageVersions::get hydrated the full manifest fragment (including the flattened catch-all map), which a time profile of a warm-cache fresh resolve of the babylon fixture showed as the single largest CPU consumer (~10 thread-seconds, all on the resolve task's critical path). PackageVersions::is_deprecated answers the same question from the raw fragment: a substring pre-check for the key text, then a single-field deserialize using the same normalization as PackageVersion::deprecated. The tag-repopulation loop now also parses candidate versions once per filter call instead of once per tag, matching the parsedSemverCache in pnpm's filterPkgMetadataByPublishDate, and the deprecated-pick fallback in pick_version_by_version_range uses the probe instead of hydrating every version. Warm-cache fresh resolve of the babylon fixture: resolve_workspace 7.5s -> 2.6s. Resolved version sets are byte-identical before/after across the vue and babylon fixtures.
|
Code review by qodo was updated up to the latest commit 1b14a25 |
Micro-Benchmark ResultsLinux |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12329 +/- ##
==========================================
+ Coverage 87.73% 87.74% +0.01%
==========================================
Files 291 291
Lines 36034 36061 +27
==========================================
+ Hits 31613 31642 +29
+ Misses 4421 4419 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Integrated-Benchmark Report (Linux)Each scenario reports direct installs and pnpr installs. Bencher consumes pacquet@HEAD and pnpr@HEAD. Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.36098859244,
"stddev": 0.2751960834433389,
"median": 4.321700333340001,
"user": 3.692216319999999,
"system": 3.4676878,
"min": 4.08531859984,
"max": 5.02521061284,
"times": [
5.02521061284,
4.41613649384,
4.34136288284,
4.30203778384,
4.17363209784,
4.10601188484,
4.46826957184,
4.49667217484,
4.08531859984,
4.19523382184
]
},
{
"command": "pacquet@main",
"mean": 10.495090559340001,
"stddev": 0.04368957912307264,
"median": 10.499637919840001,
"user": 3.4223905199999995,
"system": 3.4700007999999998,
"min": 10.421427666840001,
"max": 10.54418796384,
"times": [
10.49737135384,
10.534607367840001,
10.50190448584,
10.52172009184,
10.54418796384,
10.54272491284,
10.43359894884,
10.46460143284,
10.48876136884,
10.421427666840001
]
},
{
"command": "pnpr@HEAD",
"mean": 2.37351074154,
"stddev": 0.13071851500842108,
"median": 2.3129642008399998,
"user": 2.6732438199999997,
"system": 3.0275495,
"min": 2.23582939284,
"max": 2.59644759384,
"times": [
2.46624290484,
2.59644759384,
2.38754424184,
2.30613696184,
2.57849777184,
2.29086821984,
2.23582939284,
2.25059538484,
2.31979143984,
2.30315350384
]
},
{
"command": "pnpr@main",
"mean": 5.493573556339999,
"stddev": 0.12985650097980853,
"median": 5.42261265234,
"user": 2.46874622,
"system": 3.0077152999999996,
"min": 5.38419008684,
"max": 5.73721085784,
"times": [
5.40424213984,
5.41833116484,
5.39692983284,
5.73721085784,
5.42689413984,
5.56150239384,
5.39638005784,
5.68901555284,
5.38419008684,
5.52103933684
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.71392609116,
"stddev": 0.04312658480751863,
"median": 0.71241331886,
"user": 0.38508790000000004,
"system": 1.35659194,
"min": 0.66050793036,
"max": 0.82165743236,
"times": [
0.7130566353600001,
0.71264286336,
0.7134202243600001,
0.71218377436,
0.66050793036,
0.69047882136,
0.82165743236,
0.73030609136,
0.7089041353600001,
0.6761030033600001
]
},
{
"command": "pacquet@main",
"mean": 0.67648206136,
"stddev": 0.02431367457716857,
"median": 0.67749422086,
"user": 0.37206870000000003,
"system": 1.3535840399999999,
"min": 0.64871316136,
"max": 0.71074656436,
"times": [
0.6501481463600001,
0.68959147136,
0.65126240036,
0.71074656436,
0.64871316136,
0.69847161636,
0.69318046136,
0.70078140036,
0.65652842136,
0.66539697036
]
},
{
"command": "pnpr@HEAD",
"mean": 0.7870517100600001,
"stddev": 0.016078732024606468,
"median": 0.7841958068600001,
"user": 0.3970135,
"system": 1.35893504,
"min": 0.7667215483600001,
"max": 0.81204732436,
"times": [
0.80795864736,
0.77458346536,
0.77900284736,
0.76976982236,
0.78783877636,
0.7809148823600001,
0.80420305536,
0.7874767313600001,
0.7667215483600001,
0.81204732436
]
},
{
"command": "pnpr@main",
"mean": 0.8290630129600001,
"stddev": 0.0892308243463461,
"median": 0.80983359936,
"user": 0.4016039,
"system": 1.35579964,
"min": 0.75179426136,
"max": 1.05944252436,
"times": [
0.82454076736,
0.77508822436,
0.80552702536,
0.81726694536,
0.88200411536,
1.05944252436,
0.80844764236,
0.81121955636,
0.7552990673600001,
0.75179426136
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 3.9028256427400003,
"stddev": 0.0919393965677526,
"median": 3.88818942164,
"user": 3.5319932400000007,
"system": 3.36894944,
"min": 3.77409682164,
"max": 4.01768871664,
"times": [
3.87247307764,
4.01707743664,
3.96593412864,
3.77409682164,
4.01768871664,
3.8242307116400003,
3.9039057656400002,
3.9959917156399998,
3.8632546426400003,
3.7936034106400003
]
},
{
"command": "pacquet@main",
"mean": 8.67168462714,
"stddev": 0.06453334244984488,
"median": 8.678641392140001,
"user": 3.4072294400000005,
"system": 3.3236599399999998,
"min": 8.57493974564,
"max": 8.75643240164,
"times": [
8.59818023464,
8.60093761864,
8.70113984664,
8.65246347664,
8.66434508364,
8.73211242264,
8.75643240164,
8.57493974564,
8.69293770064,
8.74335774064
]
},
{
"command": "pnpr@HEAD",
"mean": 2.3067456292399995,
"stddev": 0.1705240221297092,
"median": 2.24297990264,
"user": 2.51860704,
"system": 2.9206605400000005,
"min": 2.12074581764,
"max": 2.58649557464,
"times": [
2.58649557464,
2.17029072164,
2.26949845364,
2.18382706964,
2.21646135164,
2.47975431764,
2.4365072526400002,
2.13052289364,
2.47335283964,
2.12074581764
]
},
{
"command": "pnpr@main",
"mean": 5.23705295474,
"stddev": 0.1348929322330615,
"median": 5.18460447214,
"user": 2.30648304,
"system": 2.90310324,
"min": 5.12475922364,
"max": 5.56939923864,
"times": [
5.56939923864,
5.37997183764,
5.17512703364,
5.12475922364,
5.16537455064,
5.18418693864,
5.18049513964,
5.21525634464,
5.19093723464,
5.18502200564
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.0963855200800001,
"stddev": 0.015767306790172277,
"median": 1.09531269858,
"user": 1.07811382,
"system": 1.6948843800000002,
"min": 1.07923595858,
"max": 1.12822867258,
"times": [
1.0843741545799999,
1.10518979458,
1.10962504958,
1.08285628958,
1.07923595858,
1.10039225658,
1.10313817458,
1.12822867258,
1.0805817095799999,
1.0902331405799999
]
},
{
"command": "pacquet@main",
"mean": 1.12899275058,
"stddev": 0.02717144550092835,
"median": 1.12023069858,
"user": 1.1274129199999998,
"system": 1.6807867799999996,
"min": 1.10546917158,
"max": 1.19010319158,
"times": [
1.1111581555799999,
1.12367230058,
1.19010319158,
1.12989797058,
1.10546917158,
1.12926990558,
1.16302963658,
1.11092953858,
1.1096085385799999,
1.11678909658
]
},
{
"command": "pnpr@HEAD",
"mean": 0.7328936994799999,
"stddev": 0.10307706496911469,
"median": 0.6929744930799999,
"user": 0.34540121999999995,
"system": 1.3039644799999999,
"min": 0.68044786558,
"max": 1.01652413858,
"times": [
0.68651350458,
0.68745702958,
1.01652413858,
0.70042317158,
0.68133094058,
0.68394698958,
0.69849195658,
0.76313062958,
0.68044786558,
0.73067076858
]
},
{
"command": "pnpr@main",
"mean": 0.7052651398800002,
"stddev": 0.053483854114212616,
"median": 0.68803665258,
"user": 0.33607941999999996,
"system": 1.3196786799999998,
"min": 0.67831662858,
"max": 0.8561760645800001,
"times": [
0.69794131958,
0.68889870858,
0.70086527658,
0.68660046958,
0.67831662858,
0.69201663858,
0.8561760645800001,
0.68188742558,
0.68277427058,
0.68717459658
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.6055066480999995,
"stddev": 0.052931214444024294,
"median": 2.5937599026999996,
"user": 1.52497646,
"system": 1.9640156000000002,
"min": 2.5425557031999997,
"max": 2.7340704901999997,
"times": [
2.7340704901999997,
2.5593596302,
2.5425557031999997,
2.5813255141999996,
2.5947445061999996,
2.5835727792,
2.5927752992,
2.6118493901999997,
2.6386140231999997,
2.6161991452
]
},
{
"command": "pacquet@main",
"mean": 4.8271101946999995,
"stddev": 0.02348898761626899,
"median": 4.829924288699999,
"user": 1.54020556,
"system": 1.9433221,
"min": 4.7904453552,
"max": 4.8601295752,
"times": [
4.7982534082,
4.8393144812,
4.810471404199999,
4.7904453552,
4.8508686552,
4.8452545682,
4.8149871132,
4.820534096199999,
4.8408432902,
4.8601295752
]
},
{
"command": "pnpr@HEAD",
"mean": 0.6799896228000001,
"stddev": 0.012080537940404326,
"median": 0.6823985617000001,
"user": 0.33468455999999996,
"system": 1.3077908999999999,
"min": 0.6657399492,
"max": 0.7012546852,
"times": [
0.7012546852,
0.6805471602000001,
0.6854097562,
0.6919225582,
0.6699023292,
0.6863578782,
0.6680819802000001,
0.6664299682,
0.6842499632000001,
0.6657399492
]
},
{
"command": "pnpr@main",
"mean": 0.7146822166,
"stddev": 0.07270580727996194,
"median": 0.6854143107,
"user": 0.34258555999999996,
"system": 1.3053927,
"min": 0.6786600332,
"max": 0.9151457172,
"times": [
0.6830623142000001,
0.6925622112,
0.7108437752000001,
0.6873692052,
0.7358830532,
0.6834594162000001,
0.6805265872,
0.6786600332,
0.9151457172,
0.6793098532
]
}
]
} |
|
| Branch | pr/12329 |
| Testbed | pacquet |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 3,902.83 ms(-57.61%)Baseline: 9,206.72 ms | 11,048.06 ms (35.33%) |
| isolated-linker.fresh-install.cold-cache.hot-store | 📈 view plot 🚷 view threshold | 2,605.51 ms(-47.85%)Baseline: 4,995.80 ms | 5,994.96 ms (43.46%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,096.39 ms(-21.97%)Baseline: 1,405.14 ms | 1,686.17 ms (65.02%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 4,360.99 ms(-56.99%)Baseline: 10,138.46 ms | 12,166.15 ms (35.85%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 713.93 ms(+9.01%)Baseline: 654.91 ms | 785.90 ms (90.84%) |
|
| Branch | pr/12329 |
| Testbed | pnpr |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsflag.
Click to view all benchmark results
| Benchmark | Latency | milliseconds (ms) |
|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot | 2,306.75 ms |
| isolated-linker.fresh-install.cold-cache.hot-store | 📈 view plot | 679.99 ms |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot | 732.89 ms |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot | 2,373.51 ms |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot | 787.05 ms |
…check force_symlink_dir's up-to-date check joined the existing link's relative contents onto the link's parent and compared the result verbatim against the wanted target. The relative contents symlink_dir writes contain parent segments (.pnpm layout links are ../../<pkg>/node_modules/<name>), so the joined path never compared equal and every such link was unlinked and recreated instead of reused. Node's path.relative — which upstream symlink-dir's isExistingSymlinkUpToDate builds on — resolves its arguments, so upstream treats those links as up to date. Run both sides through lexical_normalize before comparing. A time profile of the babylon fixture's warm install tail was dominated by exactly this unlink + symlink churn; the fix cuts the warm-cache install from 6.8s to 4.7s and the warm frozen install from 4.2s to 2.3s on that fixture.
The default was min(64, max(workers * 3, 16)), deriving the floor from the CPU count. Downloads are I/O-bound: on a 4-vCPU CI runner the formula yields 16 concurrent requests, leaving a low-latency registry unsaturated while multi-hundred-tarball installs drain 16 at a time. The new default is min(96, max(workers * 3, 64)). The networkConcurrency setting still overrides it. Applied to pnpm's package-requester, the lockfile-resolution verifier fan-out that mirrors its floor, and the same two spots in pacquet.
|
Code review by qodo was updated up to the latest commit c99510c |
The Lint and Test (ubuntu-latest) job started failing on main and on PR branches with the linker dying mid-build — explicitly with "No space left on device", or with "ld terminated with signal 7 [Bus error]" (a SIGBUS on the linker's memory-mapped output when the disk fills). The job builds the workspace twice over (clippy --all-targets plus the nextest test build), and the combined debug artifacts no longer fit in the hosted runner's free disk. Remove the preinstalled toolchains the job never touches (.NET, Android, GHC, CodeQL — roughly 25 GB) before compiling anything.
|
Code review by qodo was updated up to the latest commit f86db31 |
A near-full affected-packages test sweep plus the pnpr Rust build exhausted the hosted runner's disk mid-test — the runner worker died with "No space left on device" while the test step was still running. Same remedy as the pacquet lint-and-test job: remove the preinstalled toolchains the job never touches (.NET, Android, GHC, CodeQL — roughly 25 GB) before doing any work.
|
Code review by qodo was updated up to the latest commit 28e8cee |
Benchmark Results
Run 27363215302 · 10 runs per scenario · triggered by @zkochan |
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Motivation
The vlt.sh benchmarks (2026-06-11 run, pacquet 0.11.3) show pacquet several times slower than the fastest package managers in the warm-metadata fresh-resolve cells (
cache: 3.9–8.1x), the cold-cache frozen-install cells (lockfile: up to 10x on vue), andclean. Profiling the babylon and vue fixtures locally (macOS time profiles of the warm fresh resolve and the install tail) surfaced three independent causes, fixed here.Changes
1. Deprecation probing without manifest hydration (pacquet)
With
minimumReleaseAgeactive (the default), every range pick runsfilter_pkg_metadata_by_publish_date, and any dist-tag pointing outside the maturity cutoff (next,beta,canary, a too-freshlatest) repopulates by scanning all candidates and reading each candidate'sdeprecatedflag. Each read hydrated the full version manifest — a completeserde_jsonparse including the flattened catch-all map. On babylon's warm fresh resolve this was the single largest CPU consumer (~10 thread-seconds, all on the resolve task's critical path).PackageVersions::is_deprecatednow answers from the raw fragment (substring pre-check, then a single-field deserialize with the same normalization asPackageVersion::deprecated), the tag-repopulation loop parses candidate versions once per filter call (mirroring theparsedSemverCachein pnpm'sfilterPkgMetadataByPublishDate), and the deprecated-pick fallback uses the probe instead of hydrating every version.babylon warm fresh resolve:
resolve_workspace7.5s → 2.6s.2. Relative-symlink up-to-date check (pacquet)
force_symlink_dirjoined an existing link's relative contents onto the link parent and compared the result verbatim against the wanted target. Virtual-store links contain..segments (../../<pkg>/node_modules/<name>), so the joined path never compared equal and every up-to-date symlink was unlinked and recreated. Node'spath.relative— which upstreamsymlink-dir'sisExistingSymlinkUpToDatebuilds on — resolves its arguments, so pnpm treats those links as current. Both sides now pass throughlexical_normalize. The babylon install tail was dominated by exactly this unlink+symlink churn.babylon warm install: 6.8s → 4.7s; warm frozen install: 4.2s → 2.3s.
3. Default network concurrency floor 16 → 64 (pnpm + pacquet)
The default was
min(64, max(workers * 3, 16)). Downloads are I/O-bound, not CPU-bound: on a 4-vCPU CI runner the formula yields 16 concurrent requests, so a low-latency registry drains 600–1300-tarball installs 16 at a time while staying unsaturated — a large share of the cold-cell (lockfile/clean) gap on the benchmark runners. The default is nowmin(96, max(workers * 3, 64)); thenetworkConcurrencysetting still overrides it. Applied to@pnpm/installing.package-requester, the lockfile-resolution verifier fan-out that mirrors its floor, and the same two spots in pacquet. Changeset included (minor). This is a user-visible defaults change on both stacks — flagging it explicitly for review.Local results (M-series macOS, vlt fixtures, isolated store/cache)
cachecache+lockfilecachecache+lockfilevue's warm cells are now ahead of every competitor measured locally; babylon's
cachecell closed from ~2.5x behind the leader to ~1.35x (the remainder is the per-file store-integrity verify and per-file linking that the pnpm store contract requires).Validation
cargo nextest: registry, resolving-npm-resolver, resolving-deps-resolver, lockfile-verification, network, fs, tarball, package-manager, cli — 1300+ tests, all green; new unit tests cover the deprecation probe (string/bool/empty/corrupt shapes, nested-key false positives) and cross-parent relative-symlink reuse (fails without the fix).--lockfile-onlyoutput is byte-identical before/after on vue; on babylon the resolved package-version sets are identical across 6 runs (3 per binary). The babylon lockfile does flap between runs in the peer-suffix shape ofwebpack-dev-server@5.2.2((bufferutil@4.1.0)(utf-8-validate@5.0.10)appearing/disappearing) — this is pre-existing nondeterminism reproducible with the unmodified binary against itself, in the optional-peer area; worth a separate issue.cargo doc -D warnings, dylint) pass; eslint (root config) andtsgo --buildpass for the two touched TS packages.Written by an agent (Claude Code, claude-fable-5).
Summary by CodeRabbit
New Features
Performance
Improvements
Tests
Chores
Also in this PR: CI disk-space fix
The
Lint and Test (ubuntu-latest)job was failing on main and on PR branches with the linker dying mid-build (No space left on device, orld terminated with signal 7 [Bus error]— a SIGBUS on the linker's mmap'd output when the disk fills). The job builds the workspace twice over (clippy--all-targets+ the nextest build) and the combined debug artifacts no longer fit on the hosted runner. Added a Linux-only step that removes the preinstalled .NET/Android/GHC/CodeQL toolchains (~25 GB) before compiling. The TypeScript test workflow hit the same wall on this PR (the runner worker itself died with ENOSPC mid-sweep — touchingpackage-requestermakes the affected-packages scope near-total, on top of the pnpr Rust build), sotest.ymlgot the same cleanup step.