perf(pm): clone from extracted file index#2986
Conversation
elrrrrrrr
added
A-Pkg Manager
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a file indexing mechanism for extracted registry packages to optimize the cloning process within the installation scheduler. It adds the ExtractedPackageIndex and RegistryCacheEntry structures, updating the extraction and cloning utilities to generate and utilize these indices for more efficient file operations. The review feedback suggests improving maintainability by avoiding variable shadowing in clone_dir_from_index_sync and removing a redundant with_context call on a successful result.
| let mut warned_per_file = false; | ||
| for rel in index.files.iter() { | ||
| let src = real_src.join(rel); | ||
| let dst = dst.join(rel); |
There was a problem hiding this comment.
Shadowing the dst function argument inside the loop is risky. While it works correctly here because the loop body is fresh each iteration, it makes the code harder to reason about and could lead to bugs if the loop logic is modified to rely on the original dst later in the iteration. Using a distinct name like target_file would improve maintainability.
| } | ||
| } | ||
|
|
||
| Ok(()).with_context(|| err_msg) |
There was a problem hiding this comment.
The .with_context() call on Ok(()) is ineffective and redundant. anyhow::Context only attaches context to Err variants. Since this line is only reached if all operations succeeded, the context is never used. Additionally, the caller of clone_dir_from_index_sync already wraps the result with a similar context, making this local attempt unnecessary.
| Ok(()).with_context(|| err_msg) | |
| Ok(()) |
elrrrrrrr
force-pushed
the
exp/pm-install-clone-extracted-index
branch
2 times, most recently
from
11ef361 to
b0fcfc4
Compare
elrrrrrrr
force-pushed
the
exp/pm-install-clone-extracted-index
branch
from
b0fcfc4 to
46dedeb
Compare
📊 pm-bench-phases ·
|
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 10.90s | 0.41s | 8.05s | 7.52s | 737M | 320.0K |
| utoo-next | 11.21s | 1.12s | 8.28s | 9.48s | 991M | 127.6K |
| utoo-npm | 11.26s | 0.86s | 8.51s | 9.71s | 1.01G | 119.3K |
| utoo | 9.66s | 1.49s | 8.72s | 8.40s | 918M | 137.7K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 17.7K | 16.0K | 1.20G | 6M | 1.88G | 1.76G | 1M |
| utoo-next | 140.9K | 91.8K | 1.17G | 5M | 1.72G | 1.72G | 2M |
| utoo-npm | 171.6K | 101.3K | 1.17G | 6M | 1.72G | 1.71G | 2M |
| utoo | 104.3K | 41.7K | 1.17G | 6M | 1.72G | 1.71G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 1.75s | 0.08s | 3.21s | 0.86s | 517M | 163.2K |
| utoo-next | 2.53s | 0.05s | 4.11s | 1.71s | 614M | 89.6K |
| utoo-npm | 2.56s | 0.04s | 4.16s | 1.74s | 619M | 84.1K |
| utoo | 2.11s | 0.14s | 4.59s | 1.27s | 658M | 120.5K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 10.6K | 3.9K | 204M | 3M | 108M | - | 1M |
| utoo-next | 76.8K | 88.1K | 201M | 2M | 7M | 3M | 2M |
| utoo-npm | 76.5K | 90.3K | 201M | 2M | 7M | 3M | 2M |
| utoo | 15.2K | 20.5K | 203M | 3M | 7M | 3M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 10.74s | 0.96s | 4.78s | 7.32s | 687M | 219.6K |
| utoo-next | 8.77s | 1.72s | 4.04s | 8.27s | 475M | 62.7K |
| utoo-npm | 10.87s | 2.17s | 4.33s | 8.52s | 524M | 62.8K |
| utoo | 6.75s | 0.26s | 3.88s | 7.33s | 499M | 59.1K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 7.1K | 4.7K | 1.00G | 4M | 1.77G | 1.77G | 1M |
| utoo-next | 124.2K | 46.0K | 998M | 3M | 1.71G | 1.71G | 2M |
| utoo-npm | 155.7K | 42.8K | 999M | 4M | 1.71G | 1.71G | 2M |
| utoo | 83.2K | 31.4K | 998M | 3M | 1.71G | 1.71G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 4.16s | 0.23s | 0.16s | 1.93s | 134M | 32.5K |
| utoo-next | 3.77s | 0.67s | 0.40s | 2.90s | 79M | 18.4K |
| utoo-npm | 3.56s | 0.35s | 0.39s | 2.92s | 78M | 18.0K |
| utoo | 6.81s | 2.51s | 0.29s | 2.60s | 50M | 11.4K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 228 | 26 | 5M | 38K | 1.93G | 1.74G | 1M |
| utoo-next | 40.3K | 17.9K | 11K | 34K | 1.71G | 1.71G | 2M |
| utoo-npm | 40.4K | 17.1K | 445B | 731B | 1.71G | 1.71G | 2M |
| utoo | 13.2K | 9.0K | 8K | 31K | 1.72G | 1.71G | 2M |
npmmirror.com: no output captured.
1 participant
Summary
Stacked on #2984. This tests the p3 hypothesis that cold install pays avoidable cache-walk cost immediately after extracting a tarball.
read_dirwalk of the package cacheindex: None)Expected Impact
p3_cold_installcurrently does download -> extract-to-cache -> clone-from-cache. For freshly extracted registry tarballs, extract already parsed every file path, but clone walks the cache again to rediscover those paths. Reusing the extracted file index should reduce p3 filesystem syscalls and context switches without changing cache layout or p4 behavior.This is an AB experiment after #2985 showed demand download priority was not a p3 win.
Benchmark Notes
Public GHA npmjs run (
pm-bench-phases, linux, ant-design):p3_cold_install:utoo 5.87s,69.0K / 46.1K ctxvsutoo-npm 7.03s,111.2K / 52.2K ctxandutoo-next 9.39s,135.1K / 48.7K ctxp4_warm_link:utoo 2.04s,13.4K / 9.7K ctxvsutoo-npm 2.27s,41.1K / 19.1K ctxp1_resolve: stays positive from the scheduler stack,utoo 2.39s,15.2K / 20.2K ctxvsutoo-npm 3.34s,77.7K / 94.2K ctxnpmmirror.comdid not emit output in this GHA run.Validation
cargo fmtCARGO_NET_OFFLINE=true cargo clippy -p utoo-pm --all-targets -- -D warnings --no-depsCARGO_NET_OFFLINE=true cargo test -p utoo-pm service::install_scheduler -- --nocaptureCARGO_NET_OFFLINE=true cargo test -p utoo-pm util::extractor -- --nocaptureCARGO_NET_OFFLINE=true cargo test -p utoo-pm util::downloader -- --nocaptureCARGO_NET_OFFLINE=true cargo test -p utoo-pm util::cloner::hardlink_clone_tests -- --nocapture(0 tests on mac; Linux-only indexed hardlink test is covered by GHA)