-
Notifications
You must be signed in to change notification settings - Fork 116
perf(pm): add resolver manifest provider boundary #3030
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
elrrrrrrr
wants to merge
4
commits into
next
Choose a base branch
from
perf/pm-split-resolver-provider-boundary
base: next
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+151
−15
Draft
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
7071a09
perf(pm): add resolver manifest provider boundary
elrrrrrrr c3c7d7a
fix(ci): stabilize pm view and web worker checks
elrrrrrrr 14d38c9
ci(pm): make defender setup best effort
elrrrrrrr 21ea59b
ci(pm): make bench cargo cache best effort
elrrrrrrr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| //! Manifest provider boundary for resolver drivers. | ||
| //! | ||
| //! The demand BFS loop owns per-run cache, waiters, and inflight de-duplication. | ||
| //! A provider only executes one manifest job and hides whether it satisfied the | ||
| //! job from memory, persistent storage, or the network. | ||
|
|
||
| use std::sync::Arc; | ||
|
|
||
| use async_trait::async_trait; | ||
|
|
||
| use super::cache::VersionsInfo; | ||
| use super::manifest::MetadataFormat; | ||
| use crate::model::manifest::{CoreVersionManifest, FullManifest}; | ||
| use crate::traits::registry::RegistryClient; | ||
|
|
||
| /// Full-manifest data returned by a provider job. | ||
| #[derive(Clone)] | ||
| pub enum ManifestFullData { | ||
| /// A parsed full manifest. When the original job carried a spec, the | ||
| /// provider may also return the matching version manifest extracted in the | ||
| /// same worker task so the main loop can avoid an extra extract hop. | ||
| Full { | ||
| manifest: Arc<FullManifest>, | ||
| speculative: Option<(String, Arc<CoreVersionManifest>)>, | ||
| }, | ||
| /// A validated versions list, usually from a 304 path. The main loop can | ||
| /// resolve a concrete version and schedule a version-manifest job. | ||
| Versions(Arc<VersionsInfo>), | ||
| } | ||
|
|
||
| /// Unit of work spawned by the demand BFS loop. | ||
| #[derive(Clone)] | ||
| pub enum ManifestJob { | ||
| Full { | ||
| name: String, | ||
| /// Optional range/tag from the BFS edge that caused this full-manifest | ||
| /// fetch. The provider can use it to speculatively extract the current | ||
| /// version while the full manifest bytes are already on a CPU worker. | ||
| spec: Option<String>, | ||
| }, | ||
| Version { | ||
| name: String, | ||
| /// Cache/waiter key owned by the main loop. | ||
| spec: String, | ||
| /// Registry request spec. For npmjs 304 flows this is the resolved | ||
| /// exact version, while `spec` remains the original range key. | ||
| fetch_spec: String, | ||
| /// Metadata format for the version endpoint. Semver-capable registries | ||
| /// accept install-v1 for range/tag queries; npmjs exact-version | ||
| /// fallback requires the complete metadata format. | ||
| format: MetadataFormat, | ||
| }, | ||
| ExtractVersion { | ||
| name: String, | ||
| spec: String, | ||
| version: String, | ||
| full: Arc<FullManifest>, | ||
| }, | ||
| } | ||
|
|
||
| /// Result of one provider job. | ||
| pub enum ManifestJobDone { | ||
| Full { | ||
| name: String, | ||
| data: ManifestFullData, | ||
| }, | ||
| Version { | ||
| name: String, | ||
| spec: String, | ||
| manifest: Arc<CoreVersionManifest>, | ||
| }, | ||
| } | ||
|
|
||
| /// Lower-level manifest provider used by the demand BFS loop. | ||
| #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] | ||
| #[cfg_attr(not(target_arch = "wasm32"), async_trait)] | ||
| pub trait ManifestProvider: RegistryClient + Clone + Send + Sync + 'static { | ||
| /// Execute one manifest job. The provider owns I/O, persistence, and | ||
| /// parse/extract offloading; scheduling, waiters, and inflight | ||
| /// de-duplication stay in the BFS loop. | ||
| async fn execute_manifest_job(&self, job: ManifestJob) -> Result<ManifestJobDone, Self::Error>; | ||
| } | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a tuple for
speculativeresults can reduce code clarity and make it harder to extend in the future. Using a named struct explicitly defines the relationship between thespecand themanifest, improving maintainability and readability for other developers.