Skip to content

perf(pm): chunked parallel writes within tarball extraction + Tar Slip guard - #2825

Closed
elrrrrrrr wants to merge 2 commits into
nextfrom
perf/install-path
Closed

elrrrrrrr wants to merge 2 commits into
nextfrom
perf/install-path

Conversation

@elrrrrrrr

Copy link
Copy Markdown
Contributor

Summary

Second of 4 split PRs from #2818. Single-file change in crates/pm/src/util/extractor.rs covering two related improvements to the install-path tarball extraction:

  1. Chunked parallel writes — replace per-file par_iter with par_chunks(32), cutting rayon task count 32× while preserving IO-overlap parallelism across cores
  2. Tar Slip guard — reject entries whose path is absolute or contains .. components

Performance impact

CI p3_cold_install: utoo 5.74s vs bun 7.71s (utoo ~2s ahead).

A/B verified essential: removing intra-package par_chunks (sequential for entry in &entries inside each rayon task) regressed p3 +3.67s and exploded σ from 0.04 → 2.85 (IO can't interleave across cores).

Why these go together

Single function change — both edits touch the same extract_tarball_sync body. Splitting further would force a no-op intermediate state that doesn't compile cleanly.

Test plan

Context

🤖 Generated with Claude Code

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request optimizes the synchronous tarball extraction process by batching file writes into chunks using Rayon's par_chunks, which reduces task overhead and work-stealing pressure. Feedback suggests that using a fixed chunk size of 32 may lead to core under-utilization for smaller packages and recommends dynamically calculating the chunk size based on the number of entries and available threads.

Comment on lines +170 to +186
.par_chunks(WRITE_CHUNK_SIZE)
.try_for_each(|chunk| -> Result<()> {
for entry in chunk {
let mut file = fs::File::create(&entry.path)
.with_context(|| format!("Failed to create: {}", entry.path.display()))?;
file.write_all(&entry.content)
.with_context(|| format!("Failed to write: {}", entry.path.display()))?;

// Skip chmod for 0o644 (most files) — File::create() already
// produces this via umask (0o666 & ~0o022 = 0o644).
#[cfg(unix)]
if entry.mode != 0o644 {
fs::set_permissions(&entry.path, fs::Permissions::from_mode(entry.mode)).ok();
}
}
Ok(())
})?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The use of par_chunks(WRITE_CHUNK_SIZE) with a fixed size of 32 may lead to significant core under-utilization for packages with a small to moderate number of files. For example, a package with 64 files will only utilize 2 cores, even on a machine with 16+ cores. While chunking reduces Rayon task overhead, a fixed chunk size might be too aggressive for smaller packages. Consider dynamically calculating the chunk size based on the number of entries and the available parallelism (e.g., entries.len() / rayon::current_num_threads()) to ensure all cores are utilized while still amortizing overhead for very large packages.

@elrrrrrrr
elrrrrrrr marked this pull request as draft April 25, 2026 15:22
@elrrrrrrr
elrrrrrrr changed the base branch from next to perf/bench-infra April 25, 2026 15:29
Base automatically changed from perf/bench-infra to next April 27, 2026 02:14
…p guard

Two folded changes that started life as separate commits on the
parent perf branch:

1. **Sequential / chunked parallel writes** (was: ad0dee9 →
   7ab17b8). The old per-file `par_iter().for_each(write)` paid
   work-stealing futex park/unpark overhead per write. Each entry is
   <64 KB and a single fs::File::create + write_all returns in μs —
   rayon scheduler dominated. Switch to `entries.par_chunks(32)
   .try_for_each(...)`: each rayon task writes a contiguous run of
   32 files sequentially. Cuts task count by 32× while keeping
   multi-core IO-overlap parallelism.

   Cross-package parallelism is preserved by the outer
   `rayon::spawn` in `extract_tarball`, which itself was already
   landed on `next` via earlier work.

   Verified essential by an A/B on the parent branch: removing the
   intra-package par_chunks (sequential `for entry in &entries`
   inside each rayon task) regressed CI p3_cold_install +3.67s and
   exploded σ from 0.04 → 2.85 — IO can't interleave across cores
   when each tarball serialises its own writes.

2. **Tar Slip guard** — reject tar entries whose path is absolute
   or contains `..` components before joining with the destination.
   Without this an attacker-controlled tarball could overwrite
   arbitrary files via paths like `../../etc/foo` or
   `/etc/passwd`. `tar` crate does not enforce this by default;
   `npm` and `pnpm` both validate. We log+skip such entries.

Both changes touch the same single function so they commit together.

CI bench shows p3_cold_install at 5.74s vs bun 7.71s (utoo +2s
ahead). PR description in #2818 documents the full A/B journey.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@utooland utooland deleted a comment from github-actions Bot Apr 27, 2026
@elrrrrrrr elrrrrrrr added the benchmark Run pm-bench on PR label Apr 27, 2026
Compares this PR's utoo against next-branch HEAD (the merged baseline)
instead of just utoo-npm (latest published, can be days/weeks behind).
The utoo-next column isolates THIS PR's perf delta from any other
unmerged-since-publish work.

Two new build jobs (build-next-{linux,mac-arm64}) checkout origin/next
and build utoo from there in parallel with the main builds. Bench
phases pick up both artifacts via the new setup-utoo-next-baseline
composite action and pass utoo-next through PM_LIST.

Build jobs gate on the same `benchmark` label / dispatch trigger as
bench-phases — they only fire when bench-phases will actually run.

Bench script (bench/pm-bench-phases.sh) gets parallel utoo-next
support: UTOO_NEXT_BIN env, UTOO_NEXT_CACHE, and case statements
mirroring the existing utoo-npm pattern across install_cmd,
resolve_cmd, write_prepare, capture_footprint, seed_for_phase.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

📊 pm-bench-phases · a036fb1 · linux (ubuntu-latest)

Workflow run — ant-design

PMs: utoo (this branch) · utoo-next (next-branch baseline) · utoo-npm (latest published) · bun (latest)

npmjs.org

p0_full_cold

PM wall ±σ user sys RSS pgMinor
bun 8.94s 0.30s 9.99s 10.07s 626M 316.9K
utoo-next 10.12s 0.25s 11.22s 13.02s 1.25G 158.2K
utoo-npm 10.14s 0.22s 11.33s 13.06s 1.19G 145.7K
utoo 10.29s 0.61s 11.17s 12.72s 1.16G 145.5K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 16.5K 17.5K 1.17G 7M 1.85G 1.74G 1M
utoo-next 171.0K 147.0K 1.15G 4M 1.70G 1.69G 2M
utoo-npm 172.9K 158.6K 1.15G 5M 1.70G 1.69G 2M
utoo 169.9K 106.4K 1.15G 4M 1.70G 1.69G 2M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 2.23s 0.07s 3.86s 1.06s 470M 173.8K
utoo-next 5.54s 0.30s 5.97s 1.11s 430M 70.4K
utoo-npm 5.61s 0.08s 6.01s 1.11s 429M 71.4K
utoo 5.58s 0.08s 5.97s 1.18s 431M 72.0K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 10.9K 3.6K 200M 3M 104M - 1M
utoo-next 66.8K 2.6K 203M 2M 9M 5M 2M
utoo-npm 67.2K 2.6K 204M 2M 9M 5M 2M
utoo 66.5K 2.8K 204M 2M 9M 5M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 7.09s 0.74s 6.14s 10.01s 562M 205.1K
utoo-next 7.72s 1.64s 5.60s 11.24s 871M 106.2K
utoo-npm 8.22s 2.77s 5.64s 11.53s 803M 110.9K
utoo 7.55s 2.28s 5.49s 11.29s 833M 101.4K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 6.2K 7.7K 1004M 4M 1.75G 1.75G 1M
utoo-next 112.1K 72.0K 975M 3M 1.69G 1.69G 2M
utoo-npm 124.0K 87.6K 976M 3M 1.69G 1.69G 2M
utoo 118.8K 67.3K 976M 3M 1.69G 1.69G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 3.11s 0.16s 0.22s 2.30s 136M 32.9K
utoo-next 2.53s 0.06s 0.63s 3.88s 82M 19.2K
utoo-npm 2.31s 0.19s 0.62s 3.86s 83M 19.2K
utoo 2.42s 0.09s 0.61s 3.88s 82M 18.7K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 296 26 7M 26K 1.90G 1.74G 1M
utoo-next 48.7K 21.4K 24K 12K 1.69G 1.69G 2M
utoo-npm 49.8K 20.9K 24K 11K 1.69G 1.69G 2M
utoo 48.4K 21.6K 24K 29K 1.70G 1.69G 2M

npmmirror.com

p0_full_cold

PM wall ±σ user sys RSS pgMinor
bun 28.58s 6.98s 9.15s 9.72s 520M 362.4K
utoo-next 22.36s 1.46s 8.30s 14.19s 767M 117.8K
utoo-npm 21.67s 1.05s 8.32s 13.92s 814M 110.1K
utoo 26.56s 3.33s 8.18s 13.77s 745M 105.6K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 95.5K 3.6K 1.13G 12M 1.85G 1.74G 2M
utoo-next 248.0K 109.3K 1009M 9M 1.69G 1.69G 2M
utoo-npm 241.8K 111.3K 990M 9M 1.69G 1.69G 2M
utoo 237.4K 62.4K 1008M 9M 1.69G 1.69G 2M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 2.61s 0.11s 3.70s 1.20s 509M 204.7K
utoo-next 5.82s 0.26s 2.08s 0.55s 75M 16.0K
utoo-npm 5.56s 0.13s 2.07s 0.56s 75M 15.5K
utoo 13.11s 12.84s 2.13s 0.57s 75M 15.9K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 16.2K 3.9K 151M 3M 106M - 2M
utoo-next 46.8K 672 14M 2M - 4M 2M
utoo-npm 46.5K 708 14M 2M - 4M 2M
utoo 48.1K 498 13M 2M - 4M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 14.86s 0.43s 5.92s 9.45s 237M 93.9K
utoo-next 21.67s 0.47s 6.01s 12.51s 670M 94.6K
utoo-npm 24.21s 3.30s 6.09s 12.55s 664M 106.2K
utoo 24.95s 1.25s 5.95s 12.14s 695M 94.0K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 50.2K 4.9K 1009M 8M 1.75G 1.75G 2M
utoo-next 177.6K 102.1K 976M 7M 1.69G 1.69G 2M
utoo-npm 184.2K 99.8K 976M 7M 1.69G 1.69G 2M
utoo 181.7K 60.8K 988M 7M 1.69G 1.69G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 5.72s 4.34s 0.19s 2.27s 135M 30.9K
utoo-next 2.39s 0.12s 0.58s 3.84s 86M 19.9K
utoo-npm 2.71s 0.70s 0.58s 3.83s 86M 20.2K
utoo 2.35s 0.03s 0.56s 3.82s 84M 19.0K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 861 20 7M 81K 1.90G 1.73G 2M
utoo-next 48.9K 22.1K 471K 31K 1.69G 1.69G 2M
utoo-npm 48.4K 19.9K 469K 21K 1.69G 1.69G 2M
utoo 47.8K 21.0K 481K 26K 1.69G 1.69G 2M

@elrrrrrrr

Copy link
Copy Markdown
Contributor Author

Closing — clean A/B doesn't show the expected gain

After the bench infra got a utoo-next baseline column (next-branch HEAD, clean A/B), the chunked-write change measured against the trunk baseline shows no real improvement and a clear regression on npmmirror:

linux p3_cold_install (the phase this change targets):

Registry utoo-next (trunk baseline) utoo (this PR) Delta
npmjs 7.72s ±1.64s 7.55s ±2.28s -0.17s (within σ — no improvement)
npmmirror 21.67s ±0.47s 24.95s ±1.25s +3.28s (clear regression, well outside σ)

The earlier "vs utoo-npm faster by 1.4s" reading on this PR was misleading — utoo-npm @ 1.0.27 was published 10 days ago, so the gap conflated this PR with 6 other PRs merged onto next since then. The previous A/B from #2818 ("removing par_chunks regressed +3.67s") was run on a parent perf branch carrying additional commits — re-tested standalone here, the chunked-write change doesn't carry its weight.

Closing rather than tweaking — entries.par_iter().for_each(write) is fine as-is. The Tar Slip guard mentioned in the original commit is already on next via #2665.

🤖 Generated with Claude Code

@elrrrrrrr elrrrrrrr closed this Apr 27, 2026
@github-actions

Copy link
Copy Markdown

📊 pm-bench-phases · a036fb1 · mac (macos-latest)

Workflow run — ant-design

PMs: utoo (this branch) · utoo-next (next-branch baseline) · utoo-npm (latest published) · bun (latest)

npmjs.org

p0_full_cold

PM wall ±σ user sys RSS pgMinor
bun 18.50s 1.70s 6.31s 17.89s 752M 48.5K
utoo-next 24.40s 2.62s 10.76s 25.91s 1.07G 105.1K
utoo-npm 20.90s 1.14s 10.23s 21.91s 1.03G 98.4K
utoo 26.51s 1.14s 11.77s 29.09s 957M 97.1K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 15.8K 137.9K - - 1.79G 1.91G 1M
utoo-next 13.9K 367.5K - - 1.64G 1.83G 2M
utoo-npm 13.6K 357.6K - - 1.64G 1.83G 2M
utoo 13.9K 364.9K - - 1.64G 1.87G 2M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 2.63s 0.18s 2.86s 1.33s 492M 31.9K
utoo-next 7.22s 0.92s 5.62s 3.88s 539M 36.4K
utoo-npm 7.67s 1.14s 5.53s 3.49s 544M 36.8K
utoo 6.76s 0.97s 5.40s 3.22s 547M 37.4K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 31 19.9K - - 110M - 1M
utoo-next 18 72.4K - - 28M 5M 2M
utoo-npm 38 73.2K - - 28M 5M 2M
utoo 57 74.4K - - 28M 5M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 14.91s 4.13s 3.14s 13.71s 555M 36.1K
utoo-next 16.15s 3.52s 4.11s 17.77s 822M 80.5K
utoo-npm 13.52s 3.11s 3.54s 13.69s 755M 77.7K
utoo 15.38s 0.31s 4.41s 20.64s 705M 76.5K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 4.4K 126.9K - - 1.70G 1.94G 1M
utoo-next 1.6K 233.3K - - 1.61G 1.84G 2M
utoo-npm 1.5K 231.6K - - 1.61G 1.84G 2M
utoo 1.5K 224.6K - - 1.61G 1.84G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 4.62s 0.31s 0.10s 2.06s 51M 3.8K
utoo-next 3.71s 0.11s 0.50s 2.59s 91M 6.8K
utoo-npm 3.91s 0.32s 0.51s 2.64s 94M 7.1K
utoo 3.34s 0.39s 0.50s 2.56s 91M 6.9K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 16.4K 817 - - 1.87G 1.91G 1M
utoo-next 12.5K 74.5K - - 1.61G 1.85G 2M
utoo-npm 12.8K 72.6K - - 1.61G 1.85G 2M
utoo 13.3K 71.2K - - 1.64G 1.85G 2M

npmmirror.com

p0_full_cold

PM wall ±σ user sys RSS pgMinor
bun 36.34s 6.40s 7.25s 22.04s 562M 36.4K
utoo-next 31.74s 0.95s 5.76s 14.86s 789M 78.7K
utoo-npm 34.15s 2.65s 7.29s 20.74s 746M 76.2K
utoo 32.10s 1.32s 5.67s 14.29s 716M 78.9K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 14.8K 169.6K - - 1.79G 1.90G 2M
utoo-next 999 437.8K - - 1.61G 1.88G 2M
utoo-npm 4.0K 399.7K - - 1.61G 1.88G 2M
utoo 907 384.7K - - 1.61G 1.83G 2M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 3.82s 0.04s 2.62s 1.49s 510M 33.2K
utoo-next 7.13s 0.30s 1.40s 0.73s 78M 5.7K
utoo-npm 7.09s 0.43s 1.41s 0.74s 78M 5.7K
utoo 13.37s 11.04s 1.75s 0.92s 79M 5.7K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 38 29.0K - - 111M - 2M
utoo-next 6 47.3K - - - 4M 2M
utoo-npm 6 47.6K - - - 4M 2M
utoo 27 47.2K - - - 4M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 22.53s 0.06s 3.79s 16.02s 259M 17.2K
utoo-next 38.49s 0.17s 6.91s 25.80s 644M 72.8K
utoo-npm 36.35s 2.99s 5.95s 20.94s 598M 71.0K
utoo 47.55s 14.64s 6.20s 21.72s 663M 74.6K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 1.8K 162.2K - - 1.65G 1.92G 2M
utoo-next 1.7K 334.4K - - 1.61G 1.84G 2M
utoo-npm 1.6K 344.9K - - 1.61G 1.84G 2M
utoo 1.8K 317.0K - - 1.61G 1.84G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 5.98s 2.21s 0.12s 2.35s 44M 3.4K
utoo-next 5.25s 0.24s 0.71s 3.72s 90M 6.6K
utoo-npm 5.37s 0.83s 0.73s 3.87s 94M 6.9K
utoo 5.60s 0.42s 0.71s 4.04s 90M 6.8K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 13.0K 1.0K - - 1.79G 1.91G 2M
utoo-next 12.3K 75.5K - - 1.61G 1.83G 2M
utoo-npm 11.7K 75.8K - - 1.61G 1.83G 2M
utoo 12.7K 75.4K - - 1.61G 1.83G 2M

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmark Run pm-bench on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant