perf(pm): prioritize demand install downloads#2985
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 refactors the download scheduling logic by replacing the single download_queue with two distinct queues: download_demand_queue and download_preload_queue. This change allows the scheduler to prioritize on-demand downloads over preloads. The ensure_download and pump_downloads_with methods were updated to handle this prioritization and support promoting preloads to demand status when a waiter is added. A review comment pointed out that the promotion logic could lead to duplicate entries in the demand queue if multiple waiters arrive for the same package before the download starts, and provided a suggestion to prevent this by checking if the waiter list is currently empty.
| } | ||
|
|
||
| if let Some(waiters) = self.download_waiters.get_mut(&key) { | ||
| let promote_to_demand = waiter.is_some() && !self.download_active.contains(&key); |
There was a problem hiding this comment.
The current logic for "promoting" a preload to a demand download will redundantly push the package to the download_demand_queue every time a new waiter arrives, as long as the download hasn't started yet. Since download_waiters is empty for preloads and non-empty for demand downloads, you should check waiters.is_empty() to ensure the package is only added to the demand queue once. This prevents the demand queue from being flooded with duplicate entries for the same package in high-concurrency scenarios.
| let promote_to_demand = waiter.is_some() && !self.download_active.contains(&key); | |
| let promote_to_demand = waiter.is_some() && waiters.is_empty() && !self.download_active.contains(&key); |
📊 pm-bench-phases ·
|
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 7.50s | 0.23s | 10.69s | 6.45s | 698M | 292.5K |
| utoo-next | 7.86s | 1.50s | 10.63s | 8.07s | 1.03G | 125.5K |
| utoo-npm | 6.93s | 0.02s | 10.84s | 7.98s | 1000M | 123.7K |
| utoo | 6.86s | 0.07s | 11.54s | 7.52s | 961M | 140.4K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 17.3K | 18.3K | 1.20G | 6M | 1.88G | 1.76G | 1M |
| utoo-next | 159.4K | 106.2K | 1.17G | 6M | 1.72G | 1.71G | 2M |
| utoo-npm | 142.0K | 101.4K | 1.17G | 5M | 1.72G | 1.72G | 2M |
| utoo | 91.5K | 48.7K | 1.23G | 7M | 1.72G | 1.72G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 2.14s | 0.05s | 4.78s | 0.93s | 511M | 188.4K |
| utoo-next | 2.80s | 0.03s | 5.47s | 1.47s | 631M | 84.5K |
| utoo-npm | 2.93s | 0.05s | 5.66s | 1.50s | 620M | 78.3K |
| utoo | 2.26s | 0.06s | 6.38s | 1.15s | 660M | 119.6K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 9.2K | 5.1K | 204M | 3M | 108M | - | 1M |
| utoo-next | 83.1K | 93.8K | 201M | 2M | 7M | 3M | 2M |
| utoo-npm | 82.3K | 96.1K | 201M | 2M | 7M | 3M | 2M |
| utoo | 16.2K | 18.1K | 204M | 3M | 7M | 3M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 5.46s | 0.12s | 5.96s | 6.14s | 577M | 192.1K |
| utoo-next | 7.66s | 3.40s | 5.03s | 7.14s | 490M | 59.3K |
| utoo-npm | 5.55s | 0.14s | 5.22s | 7.36s | 489M | 60.9K |
| utoo | 4.74s | 0.14s | 5.01s | 6.87s | 469M | 54.5K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 7.4K | 6.7K | 1.00G | 4M | 1.77G | 1.77G | 1M |
| utoo-next | 132.0K | 46.2K | 998M | 4M | 1.71G | 1.71G | 2M |
| utoo-npm | 119.3K | 53.9K | 997M | 3M | 1.71G | 1.71G | 2M |
| utoo | 77.0K | 45.3K | 1.08G | 3M | 1.71G | 1.71G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 2.22s | 0.03s | 0.15s | 1.27s | 136M | 32.7K |
| utoo-next | 1.90s | 0.06s | 0.58s | 2.53s | 79M | 18.5K |
| utoo-npm | 1.96s | 0.04s | 0.60s | 2.55s | 80M | 18.5K |
| utoo | 1.75s | 0.03s | 0.38s | 2.18s | 50M | 11.2K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 546 | 17 | 5M | 29K | 1.93G | 1.76G | 1M |
| utoo-next | 41.0K | 18.3K | 4K | 22K | 1.71G | 1.71G | 2M |
| utoo-npm | 43.9K | 19.3K | 1K | 3K | 1.71G | 1.71G | 2M |
| utoo | 14.4K | 9.3K | 4K | 8K | 1.72G | 1.71G | 2M |
npmmirror.com: no output captured.
1 participant
Summary
Stacked on #2984. This isolates one p3 hypothesis: preloaded tarball downloads should not block downloads that already have clone/materialize waiters.
Result
This experiment does not currently look like a merge candidate.
GHA npmjs,
ant-design, run26045724890:p3_cold_install:utoo 6.73s ±2.05, ctx74.3K / 42.9K26044780108:utoo 5.84s ±0.46, ctx70.7K / 43.6Kp4_warm_link:utoo 2.11s ±0.07, ctx13.3K / 9.4K, essentially stable vs perf(pm): serialize scheduler extract writes #2984Conclusion: prioritizing demand downloads does not reduce p3 ctx and regresses/does not improve wall in the public GHA signal. Keep this PR as an AB record unless a later rerun contradicts it.
Expected Impact
For
p3_cold_install, preload can currently occupy available download slots while BFS has already discovered packages needed by materialization. Prioritizing demand downloads should reduce idle time before extract/clone and may improve wall time without changing cache/store semantics.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 -- --nocapture