From 53e3cde6527bd634d996ca993703381bf6037efe Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Fri, 22 May 2026 00:04:58 +0200 Subject: [PATCH] perf(pacquet/resolving-resolver-base): share ResolveResult.manifest via Arc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ResolveResult` propagates by clone — once into `ResolvedPackage` in the deps-resolver's `TreeCtx`, again into each `DependenciesGraphNode` in the peer-resolved graph, and a few more times through the install dispatch. The `manifest` field is a `serde_json::Value` tree (per-version body) and every clone walked it node-by-node. Wrap it in `Arc` so `ResolveResult::clone()` bumps a refcount instead of deep-cloning the JSON tree: - New `SharedDependencyManifest = Arc` alias on `pacquet-resolving-resolver-base`. - `ResolveResult::manifest` and `LatestInfo::latest_manifest` re-typed as `Option`. - All construction sites in npm / git / tarball / local / runtime resolvers and `resolve_from_workspace` wrap their built `Value` in `Arc::new`. - Read sites mostly worked unchanged via `Arc`'s `Deref` coercion. Two call sites that pass `Option<&Value>` to a helper (`manifest_unpacked_size`, `build_package_metadata`) switched to `.as_deref()`. Mirrors JS object-reference semantics — pnpm's `pkgResponse.manifest` is an object, not a deep copy, so propagating the response across resolve / install pays no copy. Tests: 295/295 in the resolver crates pass; 298/298 in `pacquet-package-manager`. Clippy clean. --- .../src/bun_resolver.rs | 6 +++--- .../src/deno_resolver.rs | 6 +++--- .../src/node_resolver.rs | 6 +++--- .../src/dependencies_graph_to_lockfile.rs | 2 +- .../dependencies_graph_to_lockfile/tests.rs | 2 +- .../src/install_package_from_registry.rs | 2 +- .../src/resolve_importer/tests.rs | 2 +- .../resolving-deps-resolver/src/tests.rs | 2 +- .../src/local_resolver.rs | 4 ++-- .../src/npm_resolver.rs | 4 +++- .../src/resolve_from_workspace.rs | 2 +- .../crates/resolving-resolver-base/src/lib.rs | 7 ++++--- .../resolving-resolver-base/src/resolve.rs | 19 ++++++++++++++++--- 13 files changed, 40 insertions(+), 24 deletions(-) diff --git a/pacquet/crates/engine-runtime-bun-resolver/src/bun_resolver.rs b/pacquet/crates/engine-runtime-bun-resolver/src/bun_resolver.rs index 80bbde6d90..1fa3e32971 100644 --- a/pacquet/crates/engine-runtime-bun-resolver/src/bun_resolver.rs +++ b/pacquet/crates/engine-runtime-bun-resolver/src/bun_resolver.rs @@ -109,7 +109,7 @@ impl BunResolver { name_ver: None, latest: None, published_at: None, - manifest: Some(manifest), + manifest: Some(std::sync::Arc::new(manifest)), resolution, resolved_via: RESOLVED_VIA.to_string(), normalized_bare_specifier: Some(format!("runtime:{version_spec}")), @@ -157,10 +157,10 @@ impl BunResolver { return Ok(Some(LatestInfo::default())); }; Ok(Some(LatestInfo { - latest_manifest: Some(serde_json::json!({ + latest_manifest: Some(std::sync::Arc::new(serde_json::json!({ "name": "bun", "version": name_ver.suffix.to_string(), - })), + }))), })) } } diff --git a/pacquet/crates/engine-runtime-deno-resolver/src/deno_resolver.rs b/pacquet/crates/engine-runtime-deno-resolver/src/deno_resolver.rs index 406e3a9874..6720b32759 100644 --- a/pacquet/crates/engine-runtime-deno-resolver/src/deno_resolver.rs +++ b/pacquet/crates/engine-runtime-deno-resolver/src/deno_resolver.rs @@ -116,7 +116,7 @@ impl DenoResolver { name_ver: None, latest: None, published_at: None, - manifest: Some(manifest), + manifest: Some(std::sync::Arc::new(manifest)), resolution, resolved_via: RESOLVED_VIA.to_string(), normalized_bare_specifier: Some(format!("runtime:{version_spec}")), @@ -164,10 +164,10 @@ impl DenoResolver { return Ok(Some(LatestInfo::default())); }; Ok(Some(LatestInfo { - latest_manifest: Some(serde_json::json!({ + latest_manifest: Some(std::sync::Arc::new(serde_json::json!({ "name": "deno", "version": name_ver.suffix.to_string(), - })), + }))), })) } } diff --git a/pacquet/crates/engine-runtime-node-resolver/src/node_resolver.rs b/pacquet/crates/engine-runtime-node-resolver/src/node_resolver.rs index 9af0f1ae74..041525b933 100644 --- a/pacquet/crates/engine-runtime-node-resolver/src/node_resolver.rs +++ b/pacquet/crates/engine-runtime-node-resolver/src/node_resolver.rs @@ -155,7 +155,7 @@ impl NodeResolver { name_ver: None, latest: None, published_at: None, - manifest: Some(manifest), + manifest: Some(std::sync::Arc::new(manifest)), resolution, resolved_via: RESOLVED_VIA.to_string(), normalized_bare_specifier: Some(format!("runtime:{range}")), @@ -193,10 +193,10 @@ impl NodeResolver { return Ok(Some(LatestInfo::default())); }; Ok(Some(LatestInfo { - latest_manifest: Some(serde_json::json!({ + latest_manifest: Some(std::sync::Arc::new(serde_json::json!({ "name": "node", "version": version, - })), + }))), })) } diff --git a/pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs b/pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs index 958fbf2597..efe25c5dc3 100644 --- a/pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs +++ b/pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs @@ -263,7 +263,7 @@ fn build_packages_and_snapshots( /// `dependencies` / `optionalDependencies` / `transitivePeerDependencies` / /// `optional` / `patched`, which go on the snapshot below). fn build_package_metadata(node: &DependenciesGraphNode) -> PackageMetadata { - let manifest = node.resolve_result.manifest.as_ref(); + let manifest = node.resolve_result.manifest.as_deref(); let engines = manifest .and_then(|m| m.get("engines")) diff --git a/pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs b/pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs index ecc2ed564e..998ed4aac1 100644 --- a/pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs +++ b/pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs @@ -33,7 +33,7 @@ fn make_resolve_result(name: &str, version: &str, manifest: serde_json::Value) - name_ver: Some(name_ver), latest: None, published_at: None, - manifest: Some(manifest), + manifest: Some(std::sync::Arc::new(manifest)), resolution: make_registry_resolution(), resolved_via: "npm-registry".to_string(), normalized_bare_specifier: None, diff --git a/pacquet/crates/package-manager/src/install_package_from_registry.rs b/pacquet/crates/package-manager/src/install_package_from_registry.rs index d6e53f5e4b..10dbed7476 100644 --- a/pacquet/crates/package-manager/src/install_package_from_registry.rs +++ b/pacquet/crates/package-manager/src/install_package_from_registry.rs @@ -148,7 +148,7 @@ impl<'a> InstallPackageFromRegistry<'a> { if first_visit { let (tarball_url, integrity) = extract_tarball(&resolution.resolution)?; - let unpacked_size = manifest_unpacked_size(resolution.manifest.as_ref()); + let unpacked_size = manifest_unpacked_size(resolution.manifest.as_deref()); // `pnpm:progress resolved` mirrors pnpm's emit at // : diff --git a/pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs b/pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs index 20fd73ea60..e4c429d85b 100644 --- a/pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs +++ b/pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs @@ -52,7 +52,7 @@ fn fake_result(name: &str, version: &str, manifest: serde_json::Value) -> Resolv name_ver: Some(name_ver), latest: Some(version.to_string()), published_at: None, - manifest: Some(manifest), + manifest: Some(std::sync::Arc::new(manifest)), resolution: LockfileResolution::Tarball(TarballResolution { tarball: format!("https://registry.example/{name}-{version}.tgz"), integrity: None, diff --git a/pacquet/crates/resolving-deps-resolver/src/tests.rs b/pacquet/crates/resolving-deps-resolver/src/tests.rs index 1b911f07e2..5770863b36 100644 --- a/pacquet/crates/resolving-deps-resolver/src/tests.rs +++ b/pacquet/crates/resolving-deps-resolver/src/tests.rs @@ -53,7 +53,7 @@ fn fake_result(name: &str, version: &str, manifest: serde_json::Value) -> Resolv name_ver: Some(name_ver), latest: Some(version.to_string()), published_at: None, - manifest: Some(manifest), + manifest: Some(std::sync::Arc::new(manifest)), resolution: LockfileResolution::Tarball(TarballResolution { tarball: format!("https://registry.example/{name}-{version}.tgz"), integrity: None, diff --git a/pacquet/crates/resolving-local-resolver/src/local_resolver.rs b/pacquet/crates/resolving-local-resolver/src/local_resolver.rs index f942b32808..4e94a260c3 100644 --- a/pacquet/crates/resolving-local-resolver/src/local_resolver.rs +++ b/pacquet/crates/resolving-local-resolver/src/local_resolver.rs @@ -76,7 +76,7 @@ pub enum LocalResolverUpdate { #[derive(Debug, Clone)] pub struct LocalResolveResult { pub id: PkgResolutionId, - pub manifest: Option, + pub manifest: Option>, pub normalized_bare_specifier: Option, pub resolution: LockfileResolution, /// `local-filesystem` — the same `resolvedVia` tag upstream uses @@ -263,7 +263,7 @@ async fn resolve_spec( Ok(Some(LocalResolveResult { id: spec.id.clone(), - manifest: Some(manifest), + manifest: Some(std::sync::Arc::new(manifest)), normalized_bare_specifier: Some(spec.normalized_bare_specifier), resolution: LockfileResolution::Directory(DirectoryResolution { directory: spec.dependency_path, diff --git a/pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs b/pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs index 06e9c14129..a8022c894e 100644 --- a/pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs +++ b/pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs @@ -405,7 +405,9 @@ pub(crate) fn build_resolve_result( path: None, }); let published_at = meta.published_at(&picked.version.to_string()).map(str::to_string); - let manifest = Some(serde_json::to_value(picked).map_err(|err| Box::new(err) as ResolveError)?); + let manifest = Some(std::sync::Arc::new( + serde_json::to_value(picked).map_err(|err| Box::new(err) as ResolveError)?, + )); let policy_violation = detect_min_release_age_violation( &pkg_name, &picked.version.to_string(), diff --git a/pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs b/pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs index 87e6cf720c..d66dd4501e 100644 --- a/pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs +++ b/pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs @@ -266,7 +266,7 @@ fn resolve_from_local_package( name_ver: None, latest: None, published_at: None, - manifest: Some(local_package.manifest.clone()), + manifest: Some(std::sync::Arc::new(local_package.manifest.clone())), resolution: LockfileResolution::Directory(DirectoryResolution { directory }), resolved_via: "workspace".to_string(), normalized_bare_specifier: None, diff --git a/pacquet/crates/resolving-resolver-base/src/lib.rs b/pacquet/crates/resolving-resolver-base/src/lib.rs index 81d580b11b..55a754e813 100644 --- a/pacquet/crates/resolving-resolver-base/src/lib.rs +++ b/pacquet/crates/resolving-resolver-base/src/lib.rs @@ -28,9 +28,10 @@ mod verifier; pub use resolve::{ DIRECT_DEP_SELECTOR_WEIGHT, DependencyManifest, EXISTING_VERSION_SELECTOR_WEIGHT, LatestInfo, LatestQuery, PkgResolutionId, PreferredVersions, ResolveError, ResolveFuture, - ResolveLatestFuture, ResolveOptions, ResolveResult, Resolver, UpdateBehavior, - VersionSelectorEntry, VersionSelectorType, VersionSelectorWithWeight, VersionSelectors, - WantedDependency, WorkspacePackage, WorkspacePackages, WorkspacePackagesByVersion, + ResolveLatestFuture, ResolveOptions, ResolveResult, Resolver, SharedDependencyManifest, + UpdateBehavior, VersionSelectorEntry, VersionSelectorType, VersionSelectorWithWeight, + VersionSelectors, WantedDependency, WorkspacePackage, WorkspacePackages, + WorkspacePackagesByVersion, }; pub use verifier::{ ResolutionPolicyViolation, ResolutionVerification, ResolutionVerifier, VerifyCtx, VerifyFuture, diff --git a/pacquet/crates/resolving-resolver-base/src/resolve.rs b/pacquet/crates/resolving-resolver-base/src/resolve.rs index 7f6ffb4a14..0c4a31ffda 100644 --- a/pacquet/crates/resolving-resolver-base/src/resolve.rs +++ b/pacquet/crates/resolving-resolver-base/src/resolve.rs @@ -8,7 +8,7 @@ //! pnpm's //! [`createResolver`](https://github.com/pnpm/pnpm/blob/3687b0e180/resolving/default-resolver/src/index.ts#L97-L173). -use std::{collections::BTreeMap, future::Future, path::PathBuf, pin::Pin}; +use std::{collections::BTreeMap, future::Future, path::PathBuf, pin::Pin, sync::Arc}; use chrono::{DateTime, Utc}; use derive_more::{Display, From}; @@ -236,6 +236,16 @@ pub struct ResolveOptions { /// in-memory manifest lands, swap this alias for it. pub type DependencyManifest = serde_json::Value; +/// `Arc`-shared variant of [`DependencyManifest`], used in +/// [`ResolveResult::manifest`]. Wrapping the manifest avoids the +/// deep-clone of the JSON tree every time a `ResolveResult` +/// propagates — the deps-resolver stores one copy in +/// `ResolvedPackage` and another in each `DependenciesGraph` node, +/// each `Clone` cost dropped from O(manifest size) to a refcount +/// bump. Mirrors JS object-reference semantics — pnpm's +/// `resolveResult.manifest` is an object, not a deep copy. +pub type SharedDependencyManifest = Arc; + /// Outcome of one [`Resolver::resolve`] call when the resolver claims /// the wanted dependency. Mirrors pnpm's /// [`ResolveResult`](https://github.com/pnpm/pnpm/blob/3687b0e180/resolving/resolver-base/src/index.ts#L212-L237). @@ -261,7 +271,10 @@ pub struct ResolveResult { pub published_at: Option, /// The manifest fragment the resolver fetched. Optional because /// some protocols defer manifest reading to the fetch step. - pub manifest: Option, + /// Held as [`SharedDependencyManifest`] (`Arc`-shared) so the + /// deps-resolver's tree walk and the per-snapshot graph copies + /// don't deep-clone the JSON tree per occurrence. + pub manifest: Option, /// Where the artifact lives. Pacquet reuses /// [`LockfileResolution`] for this — same shape as upstream's /// `Resolution`, which is the discriminated union over @@ -309,7 +322,7 @@ pub struct LatestQuery { /// (`Ok(Some(LatestInfo { latest_manifest: None }))`). #[derive(Debug, Default, Clone)] pub struct LatestInfo { - pub latest_manifest: Option, + pub latest_manifest: Option, } /// Error type the resolver seam uses. Boxed-trait-object today so each