perf(pm): add resolver demand mainloop#3067
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 dependency resolution logic from a two-phase approach (preload and build) to a demand-driven parallel BFS resolution. It introduces a ResolverManifestCache and a ManifestState to manage manifest jobs, waiters, and de-duplication, while moving concrete manifest tasks to provider tasks. The feedback highlights that several key functions, including build_deps_with_config, build_deps_with_config_output, and run_main_loop_bfs, are missing the RegistryClient trait bound, which is necessary for handling non-registry dependencies and proper error conversion.
| receiver: &E, | ||
| ) -> Result<(), ResolveError<R::Error>> | ||
| where | ||
| R: ManifestProvider, |
There was a problem hiding this comment.
The build_deps_with_config function is missing the RegistryClient trait bound. This is required because it calls build_deps_with_config_output, which in turn calls run_main_loop_bfs. The main loop relies on several helper functions (like process_dependency and handle_resolved_registry_manifest) that specifically require R to implement RegistryClient to handle non-registry dependencies and error conversion.
R: ManifestProvider + RegistryClient,References
- When parsing a dependency specifier with an explicit protocol (e.g., 'github:'), a malformed value should fall back to a resolver that honors the user's intent and provides a clear error (e.g., 'Git' for 'github:'), rather than silently falling back to a generic resolver (e.g., 'Registry').
| receiver: &E, | ||
| ) -> Result<ResolverManifestCache, ResolveError<R::Error>> | ||
| where | ||
| R: ManifestProvider, |
There was a problem hiding this comment.
The build_deps_with_config_output function is missing the RegistryClient trait bound. This bound is necessary for the underlying run_main_loop_bfs to function correctly, as it delegates to logic that requires RegistryClient capabilities (e.g., resolving git/http dependencies via process_dependency).
R: ManifestProvider + RegistryClient,References
- When parsing a dependency specifier with an explicit protocol (e.g., 'github:'), a malformed value should fall back to a resolver that honors the user's intent and provides a clear error (e.g., 'Git' for 'github:'), rather than silently falling back to a generic resolver (e.g., 'Registry').
| let start = tokio::time::Instant::now(); | ||
| ) -> Result<ResolverManifestCache, ResolveError<R::Error>> | ||
| where | ||
| R: ManifestProvider, |
There was a problem hiding this comment.
The run_main_loop_bfs function requires the RegistryClient trait bound. Without it, calls to process_dependency (line 1408), handle_resolved_registry_manifest (line 1440), and the registry_error helper (which requires R::Error: From<RegistryError>) will fail to compile. RegistryClient provides the necessary error bounds and interface for these operations.
R: ManifestProvider + RegistryClient,References
- When parsing a dependency specifier with an explicit protocol (e.g., 'github:'), a malformed value should fall back to a resolver that honors the user's intent and provides a clear error (e.g., 'Git' for 'github:'), rather than silently falling back to a generic resolver (e.g., 'Registry').
elrrrrrrr
force-pushed
the
perf/pm-review-resolver-provider-jobs
branch
from
f1d2e5f to
016f723
Compare
elrrrrrrr
force-pushed
the
perf/pm-review-resolver-demand-mainloop
branch
from
20f3f4a to
39a248b
Compare
📊 pm-bench-phases ·
|
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 9.24s | 0.06s | 10.63s | 10.27s | 800M | 342.8K |
| utoo-next | 8.58s | 0.59s | 10.82s | 12.22s | 1019M | 126.2K |
| utoo-npm | 8.51s | 0.14s | 11.07s | 12.33s | 975M | 120.0K |
| utoo | 8.16s | 0.16s | 11.53s | 12.23s | 1016M | 150.5K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 14.8K | 18.4K | 1.22G | 6M | 1.94G | 1.82G | 1M |
| utoo-next | 124.1K | 88.5K | 1.19G | 5M | 1.77G | 1.77G | 2M |
| utoo-npm | 125.6K | 87.9K | 1.19G | 5M | 1.77G | 1.77G | 2M |
| utoo | 109.6K | 64.0K | 1.19G | 6M | 1.77G | 1.76G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 1.87s | 0.03s | 4.08s | 1.11s | 529M | 166.9K |
| utoo-next | 2.93s | 0.14s | 5.11s | 1.82s | 618M | 89.4K |
| utoo-npm | 2.99s | 0.12s | 5.23s | 2.14s | 615M | 78.9K |
| utoo | 2.37s | 0.05s | 5.92s | 1.69s | 641M | 130.8K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 8.1K | 4.9K | 205M | 3M | 109M | - | 1M |
| utoo-next | 48.5K | 71.1K | 202M | 2M | 7M | 3M | 2M |
| utoo-npm | 72.9K | 97.0K | 203M | 3M | 7M | 3M | 2M |
| utoo | 15.8K | 19.8K | 205M | 3M | 7M | 3M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 6.69s | 0.17s | 6.39s | 9.59s | 668M | 220.0K |
| utoo-next | 6.01s | 0.19s | 5.04s | 10.54s | 490M | 60.5K |
| utoo-npm | 7.22s | 1.62s | 5.27s | 10.82s | 512M | 64.5K |
| utoo | 6.85s | 2.15s | 5.17s | 10.66s | 528M | 67.1K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 5.3K | 6.1K | 1.02G | 4M | 1.82G | 1.82G | 1M |
| utoo-next | 99.6K | 49.2K | 1017M | 3M | 1.76G | 1.76G | 3M |
| utoo-npm | 122.0K | 50.6K | 1017M | 3M | 1.76G | 1.76G | 3M |
| utoo | 111.0K | 52.6K | 1017M | 3M | 1.76G | 1.76G | 3M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 3.46s | 0.07s | 0.20s | 2.48s | 136M | 32.6K |
| utoo-next | 2.20s | 0.17s | 0.50s | 3.87s | 79M | 18.9K |
| utoo-npm | 2.10s | 0.04s | 0.50s | 3.84s | 80M | 18.6K |
| utoo | 2.20s | 0.09s | 0.49s | 3.82s | 80M | 18.6K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 232 | 22 | 5M | 26K | 1.98G | 1.79G | 1M |
| utoo-next | 40.8K | 18.2K | 5K | 10K | 1.76G | 1.76G | 3M |
| utoo-npm | 42.9K | 19.7K | 4K | 24K | 1.76G | 1.76G | 3M |
| utoo | 41.5K | 19.5K | 10K | 24K | 1.77G | 1.76G | 3M |
npmmirror.com: no output captured.
1 participant
Summary
Review Focus