perf(pm): add install scheduler state#3071
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 an InstallScheduler to manage package installation tasks—including downloading, extracting, and cloning—with defined concurrency limits. The install_packages function has been updated to utilize this scheduler when available. However, the implementation is currently incomplete as it lacks the parent field in CloneSpec and the blocked_by_parent logic in SchedulerState, both of which are referenced in the included unit tests and are necessary for the intended parent-child clone ordering. These omissions will result in compilation errors.
| struct CloneSpec { | ||
| package: PackageFetch, | ||
| target: PathBuf, | ||
| } |
There was a problem hiding this comment.
The CloneSpec struct is missing the parent field, which is referenced in the unit tests (e.g., line 558) and mentioned in the PR description as a key feature for parent-child clone ordering. This will cause a compilation error.
| struct SchedulerState { | ||
| rx: mpsc::UnboundedReceiver<Command>, | ||
| shutdown: bool, | ||
| download_limit: usize, | ||
| extract_limit: usize, | ||
| clone_limit: usize, | ||
| download_done: HashMap<String, PathBuf>, | ||
| download_active: HashSet<String>, | ||
| fetch_waiters: HashMap<String, Vec<CloneSpec>>, | ||
| download_queue: VecDeque<PackageFetch>, | ||
| extract_active: HashSet<String>, | ||
| extract_queue: VecDeque<DownloadedPackage>, | ||
| clone_done: HashSet<PathBuf>, | ||
| clone_active: HashSet<PathBuf>, | ||
| clone_waiters: HashMap<PathBuf, Vec<CloneResponder>>, | ||
| clone_queue: VecDeque<ReadyClone>, | ||
| done_tx: mpsc::UnboundedSender<OpDone>, | ||
| done_rx: mpsc::UnboundedReceiver<OpDone>, | ||
| ops: FuturesUnordered<tokio::task::JoinHandle<OpDone>>, | ||
| } |
There was a problem hiding this comment.
The SchedulerState struct is missing the blocked_by_parent field (likely a HashMap<PathBuf, Vec<CloneSpec>> or similar) used in the tests (e.g., line 564). Without this field and the associated logic in queue_clone, the parent-child ordering functionality is unimplemented despite being a stated goal of this PR.
elrrrrrrr
force-pushed
the
perf/pm-review-installer-primitives
branch
from
4a43f72 to
d5c497f
Compare
elrrrrrrr
force-pushed
the
perf/pm-review-installer-scheduler-state
branch
from
26200e4 to
98fcb0e
Compare
📊 pm-bench-phases ·
|
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 9.51s | 0.19s | 10.55s | 10.50s | 774M | 341.9K |
| utoo-next | 8.39s | 0.37s | 10.75s | 12.75s | 1.05G | 127.9K |
| utoo-npm | 8.35s | 0.19s | 10.77s | 12.80s | 1000M | 130.7K |
| utoo | 8.55s | 0.53s | 10.66s | 12.57s | 963M | 118.9K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 15.0K | 18.1K | 1.22G | 6M | 1.93G | 1.81G | 1M |
| utoo-next | 126.3K | 90.4K | 1.19G | 5M | 1.77G | 1.76G | 2M |
| utoo-npm | 131.4K | 88.4K | 1.19G | 5M | 1.77G | 1.76G | 2M |
| utoo | 124.8K | 76.7K | 1.19G | 5M | 1.77G | 1.77G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 1.81s | 0.04s | 4.05s | 1.03s | 516M | 163.0K |
| utoo-next | 2.82s | 0.04s | 5.35s | 1.68s | 614M | 80.8K |
| utoo-npm | 3.03s | 0.06s | 5.61s | 2.02s | 609M | 83.5K |
| utoo | 2.86s | 0.04s | 5.38s | 1.71s | 620M | 88.6K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 7.8K | 4.7K | 205M | 3M | 109M | - | 1M |
| utoo-next | 45.6K | 68.3K | 202M | 2M | 7M | 3M | 2M |
| utoo-npm | 70.4K | 87.8K | 202M | 2M | 7M | 3M | 2M |
| utoo | 45.4K | 69.6K | 202M | 2M | 7M | 3M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 6.69s | 0.13s | 6.44s | 10.17s | 659M | 223.2K |
| utoo-next | 5.92s | 0.17s | 5.06s | 11.04s | 480M | 60.8K |
| utoo-npm | 5.87s | 0.23s | 5.15s | 11.11s | 482M | 61.8K |
| utoo | 6.07s | 0.38s | 5.14s | 10.92s | 469M | 58.2K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 4.1K | 6.6K | 1.02G | 3M | 1.82G | 1.82G | 1M |
| utoo-next | 93.0K | 45.5K | 1017M | 2M | 1.76G | 1.76G | 2M |
| utoo-npm | 97.2K | 44.7K | 1018M | 3M | 1.76G | 1.76G | 2M |
| utoo | 83.5K | 53.9K | 1017M | 2M | 1.76G | 1.76G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 3.39s | 0.09s | 0.20s | 2.41s | 136M | 34.0K |
| utoo-next | 2.47s | 0.06s | 0.50s | 3.91s | 81M | 18.7K |
| utoo-npm | 2.38s | 0.08s | 0.49s | 3.89s | 80M | 18.4K |
| utoo | 2.24s | 0.07s | 0.43s | 3.58s | 54M | 12.3K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 341 | 33 | 5M | 32K | 1.98G | 1.81G | 1M |
| utoo-next | 44.7K | 20.4K | 2K | 9K | 1.76G | 1.76G | 2M |
| utoo-npm | 41.3K | 18.7K | 2K | 19K | 1.76G | 1.76G | 2M |
| utoo | 21.0K | 12.2K | 2K | 5K | 1.77G | 1.76G | 2M |
npmmirror.com: no output captured.
elrrrrrrr
force-pushed
the
perf/pm-review-installer-primitives
branch
from
57d6322 to
a612540
Compare
1 participant
Summary
Review Focus