perf(pm): share resolver placement helpers#3040
Draft
elrrrrrrr wants to merge 1 commit into
Draft
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the dependency resolution logic in crates/ruborist/src/resolver/builder.rs by extracting shared logic into modular helper functions. Specifically, it introduces place_resolved_dependency, process_dependency_with_resolved, chain_err, and handle_processed to handle package placement, error chaining, and event reporting. These changes improve code reuse between different resolver paths and simplify the main BFS loop. I have no feedback to provide as no review comments were submitted.
195cbfd to
b9501b4
Compare
ab1867e to
e677e3a
Compare
3c19175 to
8b32ee5
Compare
e677e3a to
48257d5
Compare
📊 pm-bench-phases ·
|
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 9.05s | 0.09s | 10.46s | 9.98s | 697M | 325.5K |
| utoo-next | 8.07s | 0.25s | 10.77s | 12.11s | 1.02G | 122.8K |
| utoo-npm | 8.20s | 0.01s | 11.00s | 12.37s | 965M | 121.9K |
| utoo | 8.50s | 0.08s | 11.04s | 12.07s | 1.01G | 122.5K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 16.8K | 19.3K | 1.19G | 7M | 1.86G | 1.75G | 1M |
| utoo-next | 113.7K | 82.8K | 1.16G | 5M | 1.71G | 1.70G | 2M |
| utoo-npm | 128.8K | 95.2K | 1.16G | 5M | 1.71G | 1.70G | 2M |
| utoo | 121.0K | 69.8K | 1.17G | 6M | 1.71G | 1.70G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 2.21s | 0.08s | 4.00s | 1.16s | 513M | 186.8K |
| utoo-next | 3.06s | 0.12s | 5.08s | 1.90s | 617M | 95.6K |
| utoo-npm | 3.09s | 0.05s | 5.31s | 2.22s | 613M | 81.4K |
| utoo | 3.28s | 0.02s | 5.68s | 2.07s | 625M | 90.6K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 10.2K | 4.3K | 202M | 3M | 107M | - | 1M |
| utoo-next | 52.2K | 70.4K | 200M | 3M | 7M | 3M | 2M |
| utoo-npm | 76.5K | 92.9K | 200M | 3M | 7M | 3M | 2M |
| utoo | 60.3K | 76.8K | 202M | 3M | 7M | 3M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 6.78s | 0.30s | 6.33s | 9.74s | 596M | 204.8K |
| utoo-next | 6.77s | 1.56s | 5.11s | 10.71s | 476M | 61.3K |
| utoo-npm | 8.71s | 2.41s | 5.28s | 10.82s | 542M | 64.7K |
| utoo | 5.82s | 0.06s | 5.11s | 10.43s | 503M | 67.2K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 6.6K | 7.1K | 1019M | 4M | 1.76G | 1.76G | 1M |
| utoo-next | 116.6K | 53.6K | 990M | 3M | 1.70G | 1.70G | 2M |
| utoo-npm | 133.5K | 52.4K | 990M | 4M | 1.70G | 1.70G | 2M |
| utoo | 108.8K | 53.1K | 990M | 3M | 1.70G | 1.70G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 3.45s | 0.04s | 0.17s | 2.53s | 134M | 31.4K |
| utoo-next | 2.42s | 0.18s | 0.51s | 3.82s | 79M | 18.2K |
| utoo-npm | 2.34s | 0.07s | 0.55s | 3.82s | 80M | 18.2K |
| utoo | 2.21s | 0.07s | 0.51s | 3.75s | 79M | 18.5K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 197 | 22 | 21K | 16K | 1.87G | 1.76G | 1M |
| utoo-next | 43.0K | 19.7K | 7K | 8K | 1.70G | 1.70G | 2M |
| utoo-npm | 43.0K | 20.3K | 8K | 9K | 1.70G | 1.70G | 2M |
| utoo | 41.9K | 19.7K | 13K | 11K | 1.71G | 1.70G | 2M |
npmmirror.com: no output captured.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
cargo fmtcargo check -p utoo-ruboristcargo test -p utoo-ruborist resolver::builder::tests::test_build -- --nocapturecargo test -p utoo-ruborist resolver::builder::tests::test_resolve_high_level_api -- --nocapturecargo clippy --all-targets -- -D warnings --no-depsSplit Plan
Part of the resolver stack split from source PR #3028. This PR isolates placement/event/error helpers so the follow-up demand mainloop PR can focus on scheduling and manifest state instead of graph mutation mechanics.