perf(pm): stage resolver primitives (provider + state machinery)#3077
perf(pm): stage resolver primitives (provider + state machinery)#3077elrrrrrrr wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the provider module in ruborist, defining traits and types (ManifestProvider, ManifestJob, ManifestFullData, etc.) to serve as the manifest provider boundary for resolver drivers. The review feedback highlights a critical issue where ManifestProvider unconditionally requires Send and Sync supertrait bounds, which breaks compatibility with WASM environments where !Send or !Sync types are common. Additionally, several enums (ManifestFullData, ManifestJob, ManifestJobDone, and ProviderFullManifestBytes) are missing Debug and Clone derives, which are highly recommended for logging, debugging, and testing.
| #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] | ||
| #[cfg_attr(not(target_arch = "wasm32"), async_trait)] | ||
| pub trait ManifestProvider: RegistryClient + Clone + Send + Sync + 'static { |
There was a problem hiding this comment.
The ManifestProvider trait uses #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] to allow non-Send futures on WASM. However, the trait itself still unconditionally requires Send and Sync as supertrait bounds. This prevents !Send or !Sync types (which are common in WASM/browser environments due to JS interop) from implementing ManifestProvider on WASM.\n\nWe should make these supertrait bounds conditional on the target architecture using a conditional helper trait.
| #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] | |
| #[cfg_attr(not(target_arch = "wasm32"), async_trait)] | |
| pub trait ManifestProvider: RegistryClient + Clone + Send + Sync + 'static { | |
| #[cfg(not(target_arch = "wasm32"))] | |
| pub trait ManifestProviderBounds: Send + Sync {} | |
| #[cfg(not(target_arch = "wasm32"))] | |
| impl<T: Send + Sync> ManifestProviderBounds for T {} | |
| #[cfg(target_arch = "wasm32")] | |
| pub trait ManifestProviderBounds {} | |
| #[cfg(target_arch = "wasm32")] | |
| impl<T> ManifestProviderBounds for T {} | |
| #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] | |
| #[cfg_attr(not(target_arch = "wasm32"), async_trait)] | |
| pub trait ManifestProvider: RegistryClient + Clone + ManifestProviderBounds + 'static { |
| #[derive(Clone)] | ||
| pub enum ManifestFullData { |
There was a problem hiding this comment.
| #[derive(Clone)] | ||
| pub enum ManifestJob { |
There was a problem hiding this comment.
| } | ||
|
|
||
| /// Result of one provider job. | ||
| pub enum ManifestJobDone { |
There was a problem hiding this comment.
The ManifestJobDone enum represents the result of a provider job but does not derive Debug or Clone. Adding these derives is highly recommended for logging, debugging, and testing purposes, especially since all of its variants and inner fields already implement Debug and Clone.
#[derive(Debug, Clone)]
pub enum ManifestJobDone {| } | ||
|
|
||
| /// Raw full-manifest bytes fetched by a provider before parsing. | ||
| pub(crate) enum ProviderFullManifestBytes { |
There was a problem hiding this comment.
The ProviderFullManifestBytes enum is missing Debug and Clone derives. Adding these makes the type much more versatile and easier to debug or log within the provider implementation.
| pub(crate) enum ProviderFullManifestBytes { | |
| #[derive(Debug, Clone)] | |
| pub(crate) enum ProviderFullManifestBytes { |
Wires the staged provider boundary + state machinery (#3077) into the demand-driven BFS mainloop and retires the old two-phase preload engine. - builder.rs: run_main_loop_bfs replaces run_preload_phase/run_bfs_phase; build_deps* move from the RegistryClient bound to ManifestProvider. - registry.rs: UnifiedRegistry made stateless — drops GLOBAL_MEMORY_CACHE + OnceMap inflight maps; now impls ManifestProvider. - service/cache.rs slimmed to pure data; delete resolver/preload.rs + BuildEvent::Preload* + logger handlers. - supports_semver: bool → ResolutionMode enum across scheduling fns. - removes #3077's #![allow(dead_code)] now that the machinery has a consumer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires the staged provider boundary + state machinery (#3077) into the demand-driven BFS mainloop and retires the old two-phase preload engine. - builder.rs: run_main_loop_bfs replaces run_preload_phase/run_bfs_phase; build_deps* move from the RegistryClient bound to ManifestProvider. - registry.rs: UnifiedRegistry made stateless — drops GLOBAL_MEMORY_CACHE + OnceMap inflight maps; now impls ManifestProvider. - service/cache.rs slimmed to pure data; delete resolver/preload.rs + BuildEvent::Preload* + logger handlers. - supports_semver: bool → ResolutionMode enum across scheduling fns. - removes #3077's #![allow(dead_code)] now that the machinery has a consumer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
85ef39e to
b17c314
Compare
PR 2 of 2 (stacked on #3077). Wires the staged provider + demand engine into build_deps and retires the two-phase preload engine. - builder.rs: build_deps* drive demand::run_main_loop_bfs; preload/bfs phases + gather_preload_deps deleted; RegistryClient bound -> ManifestProvider. - registry.rs: UnifiedRegistry made stateless (drops GLOBAL_MEMORY_CACHE + OnceMap inflight maps); impls ManifestProvider. - delete resolver/preload.rs + BuildEvent::Preload* + cache_version_manifest; cache.rs slimmed; supports_semver: bool -> ResolutionMode enum. - demand.rs/provider.rs: remove the #3077 allow; add the driver integration test (CountingRegistry single-flight). #3077 + this == the reorganised resolver tree; #3028 kept as compose-back ref. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR 2 of 2 (stacked on #3077). Wires the staged provider + demand engine into build_deps and retires the two-phase preload engine. - builder.rs: build_deps* drive demand::run_main_loop_bfs; preload/bfs phases + gather_preload_deps deleted; RegistryClient bound -> ManifestProvider. - registry.rs: UnifiedRegistry made stateless (drops GLOBAL_MEMORY_CACHE + OnceMap inflight maps); impls ManifestProvider. - delete resolver/preload.rs + BuildEvent::Preload* + cache_version_manifest; cache.rs slimmed; supports_semver: bool -> ResolutionMode enum. - demand.rs/provider.rs: remove the #3077 allow; add the driver integration test (CountingRegistry single-flight). #3077 + this == the reorganised resolver tree; #3028 kept as compose-back ref. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
b17c314 to
715220e
Compare
PR 2 of 2 (stacked on #3077). Wires the staged provider + demand engine into build_deps and retires the two-phase preload engine. - builder.rs: build_deps* drive demand::run_main_loop_bfs; preload/bfs phases + gather_preload_deps deleted; RegistryClient bound -> ManifestProvider. - registry.rs: UnifiedRegistry made stateless (drops GLOBAL_MEMORY_CACHE + OnceMap inflight maps); impls ManifestProvider. - demand/: remove the #3077 allow; add the driver integration test (CountingRegistry single-flight) to mod.rs. - delete resolver/preload.rs + BuildEvent::Preload* + cache_version_manifest; cache.rs slimmed; supports_semver: bool -> ResolutionMode enum. #3077 + this == the reorganised resolver tree. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
28e3e19 to
f5f9dd0
Compare
PR 2 of 2 (stacked on #3077). Adds the demand mainloop driver, wires build_deps into it, and retires the two-phase preload engine. - demand/mod.rs: run_main_loop_bfs + fetch/handle/resolve helpers (the driver consuming the #3077 state machine); removes the staging allow. - builder.rs: build_deps* drive demand::run_main_loop_bfs; preload/bfs phases + gather_preload_deps deleted; RegistryClient bound -> ManifestProvider. - registry.rs: UnifiedRegistry made stateless (drops GLOBAL_MEMORY_CACHE + OnceMap inflight maps); impls ManifestProvider. - delete resolver/preload.rs + BuildEvent::Preload* + cache_version_manifest; cache.rs slimmed; add the driver integration test (CountingRegistry single-flight). #3077 + this == the reorganised resolver tree. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR 1 of 2 (base next). Stages the demand resolver primitives behind a module-level #![allow(dead_code)] — next still resolves via the two-phase preload path, so these have no consumer until #3078 wires them in. - service/provider.rs: ManifestProvider trait (single-threaded: the demand loop polls fetch jobs on a FuturesUnordered and never spawns, so the trait needs no Send/Sync and uses #[async_trait(?Send)] uniformly) + ManifestJob/ ManifestJobDone/ManifestFullData (all Debug + Clone), with contract tests. - resolver/demand/state.rs: the manifest state machine — ManifestState holds a generic ManifestSlot<K,V> { cache, waiters, failures } per manifest kind (dedup via is_settled, wake via wake, cache_version helper). FetchFuture is a plain boxed future (no JoinHandle). The driver lands in #3078. - builder.rs: BuildDepsConfig gains project_cache. traits/registry.rs: registry_url() default + MockRegistryClient impls ManifestProvider (?Send). Renames warm_project_cache to project_cache throughout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
f5f9dd0 to
0f34f0f
Compare
PR 2 of 2 (stacked on #3077). Adds the demand mainloop driver, wires build_deps into it, retires the two-phase preload engine. Fetch jobs run single-threaded: the driver polls them on a FuturesUnordered (no tokio::spawn), so the provider stays ?Send and the spawn/spawn_local wasm cfg is gone. - demand/mod.rs: run_main_loop_bfs + helpers; fetch_registry_manifest returns a boxed future (Box::pin, no JoinHandle); driver awaits FetchDone directly. - registry.rs: UnifiedRegistry made stateless + impls ManifestProvider (?Send). - builder.rs wired to demand; delete preload.rs + BuildEvent::Preload*; cache.rs slimmed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📊 pm-bench-phases ·
|
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 9.74s | 0.94s | 10.09s | 10.27s | 741M | 334.8K |
| utoo-next | 8.22s | 0.20s | 10.35s | 12.13s | 820M | 116.7K |
| utoo-npm | 8.02s | 0.39s | 10.54s | 12.24s | 853M | 120.5K |
| utoo | 8.41s | 0.33s | 10.40s | 12.32s | 844M | 112.0K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 14.8K | 19.1K | 1.11G | 6M | 1.79G | 1.67G | 1M |
| utoo-next | 104.8K | 73.9K | 1.08G | 5M | 1.62G | 1.61G | 2M |
| utoo-npm | 118.2K | 78.7K | 1.08G | 4M | 1.62G | 1.62G | 2M |
| utoo | 123.6K | 71.5K | 1.08G | 5M | 1.62G | 1.62G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 1.95s | 0.07s | 4.17s | 1.05s | 517M | 168.7K |
| utoo-next | 2.89s | 0.08s | 5.45s | 1.69s | 622M | 83.2K |
| utoo-npm | 3.05s | 0.03s | 5.68s | 2.01s | 625M | 78.6K |
| utoo | 3.02s | 0.20s | 5.49s | 1.73s | 620M | 78.6K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 7.9K | 4.7K | 206M | 3M | 109M | - | 1M |
| utoo-next | 46.9K | 71.7K | 203M | 2M | 7M | 3M | 2M |
| utoo-npm | 71.7K | 91.7K | 203M | 2M | 7M | 3M | 2M |
| utoo | 47.6K | 71.2K | 203M | 2M | 7M | 3M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 6.85s | 0.17s | 5.87s | 9.89s | 545M | 182.0K |
| utoo-next | 5.76s | 0.08s | 4.83s | 10.86s | 451M | 59.7K |
| utoo-npm | 6.19s | 0.19s | 4.86s | 10.85s | 435M | 61.7K |
| utoo | 5.79s | 0.23s | 4.80s | 10.62s | 497M | 63.7K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 4.6K | 6.7K | 937M | 3M | 1.67G | 1.67G | 1M |
| utoo-next | 92.2K | 46.9K | 906M | 2M | 1.61G | 1.61G | 2M |
| utoo-npm | 94.5K | 48.4K | 906M | 2M | 1.61G | 1.61G | 2M |
| utoo | 87.0K | 47.0K | 906M | 2M | 1.61G | 1.61G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 3.38s | 0.09s | 0.20s | 2.41s | 136M | 33.8K |
| utoo-next | 2.40s | 0.24s | 0.51s | 3.79s | 80M | 18.6K |
| utoo-npm | 2.43s | 0.05s | 0.48s | 3.75s | 80M | 18.4K |
| utoo | 2.23s | 0.04s | 0.50s | 3.76s | 79M | 18.2K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 474 | 29 | 5M | 17K | 1.83G | 1.66G | 1M |
| utoo-next | 42.0K | 17.9K | 307K | 12K | 1.61G | 1.61G | 2M |
| utoo-npm | 37.9K | 17.0K | 306K | 20K | 1.61G | 1.61G | 2M |
| utoo | 40.7K | 18.9K | 308K | 13K | 1.62G | 1.61G | 2M |
npmmirror.com: no output captured.
PR 1 of 2 (base
next). Stages the stateless resolver primitives the demand mainloop will consume, behind a module-level#.service/provider.rs:ManifestProvidertrait (I/O boundary) +ManifestJob/ManifestJobDone/ManifestFullData.resolver/builder.rs: demand-loop state machinery —FetchQueues,ManifestState,FetchKey/FetchDone/FetchPriority,ResolverManifestCache,ResolutionMode,pump_fetches+ manifest-job helpers.service/mod.rs: re-export provider types.All additive/stateless, no behavior change. #3078 wires
run_main_loop_bfs, deletes the preload engine, and removes the allow.🤖 Generated with Claude Code