diff --git a/pacquet/crates/package-manager/src/install_package_from_registry/tests.rs b/pacquet/crates/package-manager/src/install_package_from_registry/tests.rs index 810e6f1c7f..806a8f78cf 100644 --- a/pacquet/crates/package-manager/src/install_package_from_registry/tests.rs +++ b/pacquet/crates/package-manager/src/install_package_from_registry/tests.rs @@ -6,6 +6,7 @@ use pacquet_registry_mock::AutoMockInstance; use pacquet_reporter::{LogEvent, ProgressMessage, Reporter, SilentReporter}; use pacquet_resolving_npm_resolver::{ InMemoryPackageMetaCache, NpmResolver, shared_packument_fetch_locker, + shared_picked_manifest_cache, }; use pacquet_resolving_resolver_base::{ResolveOptions, ResolveResult, Resolver, WantedDependency}; use pacquet_store_dir::{SharedVerifiedFilesCache, StoreDir}; @@ -100,6 +101,7 @@ async fn resolve_via_mock( auth_headers: Default::default(), meta_cache: Arc::new(InMemoryPackageMetaCache::default()), fetch_locker: shared_packument_fetch_locker(), + picked_manifest_cache: shared_picked_manifest_cache(), cache_dir: Some(cache_dir.to_path_buf()), offline: false, prefer_offline: false, diff --git a/pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs b/pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs index f99950c5e4..4e8485b665 100644 --- a/pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs +++ b/pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs @@ -30,7 +30,7 @@ use pacquet_resolving_local_resolver::{ }; use pacquet_resolving_npm_resolver::{ InMemoryPackageMetaCache, MergeNamedRegistriesError, NamedRegistryResolver, NpmResolver, - merge_named_registries, shared_packument_fetch_locker, + merge_named_registries, shared_packument_fetch_locker, shared_picked_manifest_cache, }; use pacquet_resolving_resolver_base::{ LatestQuery, ResolveFuture, ResolveLatestFuture, ResolveOptions, Resolver, WantedDependency, @@ -295,6 +295,13 @@ impl<'a, DependencyGroupList> InstallWithFreshLockfile<'a, DependencyGroupList> // HTTP GETs queued behind the `ThrottledClient` semaphore. let fetch_locker = shared_packument_fetch_locker(); + // One per-`(name, version)` JSON manifest cache shared between + // the npm and named-registry resolvers, so duplicate picks of + // the same package version reuse the already-serialised + // `Arc` instead of re-running `serde_json::to_value` for + // every occurrence of a shared dep in the tree. + let picked_manifest_cache = shared_picked_manifest_cache(); + let npm_resolver: Arc = Arc::new(NpmResolver { registries, named_registries: merged_named_registries.clone(), @@ -302,6 +309,7 @@ impl<'a, DependencyGroupList> InstallWithFreshLockfile<'a, DependencyGroupList> auth_headers: Arc::clone(&config.auth_headers), meta_cache: Arc::clone(&meta_cache), fetch_locker: Arc::clone(&fetch_locker), + picked_manifest_cache: Arc::clone(&picked_manifest_cache), cache_dir: Some(config.cache_dir.clone()), offline: config.offline, prefer_offline: config.prefer_offline, @@ -342,6 +350,7 @@ impl<'a, DependencyGroupList> InstallWithFreshLockfile<'a, DependencyGroupList> auth_headers: Arc::clone(&config.auth_headers), meta_cache: Arc::clone(&meta_cache), fetch_locker: Arc::clone(&fetch_locker), + picked_manifest_cache: Arc::clone(&picked_manifest_cache), cache_dir: Some(config.cache_dir.clone()), offline: config.offline, prefer_offline: config.prefer_offline, diff --git a/pacquet/crates/resolving-npm-resolver/src/lib.rs b/pacquet/crates/resolving-npm-resolver/src/lib.rs index c81cddb6d4..ac1bd93dbc 100644 --- a/pacquet/crates/resolving-npm-resolver/src/lib.rs +++ b/pacquet/crates/resolving-npm-resolver/src/lib.rs @@ -61,7 +61,8 @@ pub use parse_bare_specifier::{ pub use pick_package::{ InMemoryPackageMetaCache, MirrorPersistError, PackageMetaCache, PackumentFetchLocker, PickPackageContext, PickPackageError, PickPackageOptions, PickPackageResult, - persist_meta_to_mirror, pick_package, shared_in_memory_cache, shared_packument_fetch_locker, + PickedManifestCache, persist_meta_to_mirror, pick_package, shared_in_memory_cache, + shared_packument_fetch_locker, shared_picked_manifest_cache, }; pub use pick_package_from_meta::{ PickPackageFromMetaError, PickPackageFromMetaOptions, PickVersionByVersionRangeOptions, diff --git a/pacquet/crates/resolving-npm-resolver/src/named_registry_resolver.rs b/pacquet/crates/resolving-npm-resolver/src/named_registry_resolver.rs index ecec5470ef..4dcb527d74 100644 --- a/pacquet/crates/resolving-npm-resolver/src/named_registry_resolver.rs +++ b/pacquet/crates/resolving-npm-resolver/src/named_registry_resolver.rs @@ -74,6 +74,10 @@ pub struct NamedRegistryResolver { /// [`crate::NpmResolver`] so concurrent picks for the same /// `(registry, name)` across resolvers coalesce. pub fetch_locker: crate::PackumentFetchLocker, + /// Shared per-`(pkg_name, version)` manifest JSON cache. See + /// [`crate::PickedManifestCache`]. Same handle as the sibling + /// [`crate::NpmResolver`]. + pub picked_manifest_cache: crate::PickedManifestCache, pub cache_dir: Option, pub offline: bool, pub prefer_offline: bool, @@ -151,6 +155,7 @@ impl NamedRegistryResolver { NAMED_REGISTRY_RESOLVED_VIA, opts.published_by, opts.published_by_exclude.as_ref(), + &self.picked_manifest_cache, )?; Ok(Some(result)) diff --git a/pacquet/crates/resolving-npm-resolver/src/named_registry_resolver/tests.rs b/pacquet/crates/resolving-npm-resolver/src/named_registry_resolver/tests.rs index 5c2d0129b5..6046f0235d 100644 --- a/pacquet/crates/resolving-npm-resolver/src/named_registry_resolver/tests.rs +++ b/pacquet/crates/resolving-npm-resolver/src/named_registry_resolver/tests.rs @@ -11,7 +11,9 @@ use tempfile::TempDir; use crate::{ NamedRegistryResolver, merge_named_registries, - pick_package::{InMemoryPackageMetaCache, shared_packument_fetch_locker}, + pick_package::{ + InMemoryPackageMetaCache, shared_packument_fetch_locker, shared_picked_manifest_cache, + }, }; /// Packument for `@acme/private` served under a named registry — @@ -70,6 +72,7 @@ fn build_resolver( auth_headers: Arc::new(AuthHeaders::default()), meta_cache: Arc::new(InMemoryPackageMetaCache::default()), fetch_locker: shared_packument_fetch_locker(), + picked_manifest_cache: shared_picked_manifest_cache(), cache_dir: Some(cache_dir.path().to_path_buf()), offline: false, prefer_offline: false, diff --git a/pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs b/pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs index a8022c894e..3d5ec27dfd 100644 --- a/pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs +++ b/pacquet/crates/resolving-npm-resolver/src/npm_resolver.rs @@ -99,6 +99,12 @@ pub struct NpmResolver { /// into one network fetch. Construct via /// [`crate::shared_packument_fetch_locker`] once per install. pub fetch_locker: crate::PackumentFetchLocker, + /// Per-`(pkg_name, version)` cache for the JSON manifest the + /// resolver builds from the picker output. Shared across this + /// resolver and [`crate::NamedRegistryResolver`] so picks of the + /// same package version across registries coalesce. Construct + /// via [`crate::shared_picked_manifest_cache`] once per install. + pub picked_manifest_cache: crate::PickedManifestCache, /// Root of the on-disk metadata mirror. `None` disables every /// disk read/write — the picker goes straight to the network on /// each cache miss. @@ -222,6 +228,7 @@ impl NpmResolver { NPM_REGISTRY_RESOLVED_VIA, opts.published_by, opts.published_by_exclude.as_ref(), + &self.picked_manifest_cache, )?; Ok(Some(result)) @@ -268,6 +275,7 @@ impl NpmResolver { JSR_REGISTRY_RESOLVED_VIA, opts.published_by, opts.published_by_exclude.as_ref(), + &self.picked_manifest_cache, )?; Ok(Some(result)) @@ -377,6 +385,7 @@ pub(crate) struct PickedFromRegistry { pub(crate) version: PackageVersion, } +#[allow(clippy::too_many_arguments)] pub(crate) fn build_resolve_result( meta: &Package, picked: &PackageVersion, @@ -385,9 +394,11 @@ pub(crate) fn build_resolve_result( resolved_via: &str, published_by: Option>, published_by_exclude: Option<&PackageVersionPolicy>, + picked_manifest_cache: &crate::PickedManifestCache, ) -> Result { let pkg_name = PkgName::parse(picked.name.as_str()).map_err(|err| Box::new(err) as ResolveError)?; + let version_str = picked.version.to_string(); let name_ver = PkgNameVer::new(pkg_name.clone(), picked.version.clone()); let id = (&name_ver).into(); // The picker always carries a tarball URL on its `dist` payload — @@ -404,13 +415,24 @@ pub(crate) fn build_resolve_result( git_hosted: None, path: None, }); - let published_at = meta.published_at(&picked.version.to_string()).map(str::to_string); - let manifest = Some(std::sync::Arc::new( - serde_json::to_value(picked).map_err(|err| Box::new(err) as ResolveError)?, - )); + let published_at = meta.published_at(&version_str).map(str::to_string); + // Dedupe `serde_json::to_value(picked)` across picks of the + // same `(pkg_name, version)` pair — see [`PickedManifestCache`] + // for the rationale. + let manifest_cache_key = format!("{}@{version_str}", picked.name); + let manifest = match picked_manifest_cache.get(&manifest_cache_key) { + Some(cached) => Some(Arc::clone(cached.value())), + None => { + let arc = Arc::new( + serde_json::to_value(picked).map_err(|err| Box::new(err) as ResolveError)?, + ); + picked_manifest_cache.insert(manifest_cache_key, Arc::clone(&arc)); + Some(arc) + } + }; let policy_violation = detect_min_release_age_violation( &pkg_name, - &picked.version.to_string(), + &version_str, published_at.as_deref(), &resolution, published_by, diff --git a/pacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rs b/pacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rs index cccad904d5..59273946ff 100644 --- a/pacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rs +++ b/pacquet/crates/resolving-npm-resolver/src/npm_resolver/tests.rs @@ -11,7 +11,9 @@ use tempfile::TempDir; use crate::{ npm_resolver::NpmResolver, - pick_package::{InMemoryPackageMetaCache, shared_packument_fetch_locker}, + pick_package::{ + InMemoryPackageMetaCache, shared_packument_fetch_locker, shared_picked_manifest_cache, + }, violation_codes::MINIMUM_RELEASE_AGE_VIOLATION_CODE, }; @@ -62,6 +64,7 @@ fn build_resolver_with_registries( auth_headers: Arc::new(AuthHeaders::default()), meta_cache: Arc::new(InMemoryPackageMetaCache::default()), fetch_locker: shared_packument_fetch_locker(), + picked_manifest_cache: shared_picked_manifest_cache(), cache_dir: Some(cache_dir.path().to_path_buf()), offline: false, prefer_offline: false, diff --git a/pacquet/crates/resolving-npm-resolver/src/pick_package.rs b/pacquet/crates/resolving-npm-resolver/src/pick_package.rs index e5aeab0f6f..fd11375026 100644 --- a/pacquet/crates/resolving-npm-resolver/src/pick_package.rs +++ b/pacquet/crates/resolving-npm-resolver/src/pick_package.rs @@ -137,6 +137,27 @@ pub fn shared_packument_fetch_locker() -> PackumentFetchLocker { Arc::new(DashMap::new()) } +/// Per-`(pkg_name, version)` cache for the resolver's serialized +/// `manifest` JSON. The npm resolver builds +/// [`ResolveResult::manifest`] via `serde_json::to_value(picked)`; +/// when many resolves pick the same version of the same package +/// (the common case for shared deps like `react`, `lodash`, …) +/// every duplicate would otherwise re-walk and re-allocate the +/// same JSON tree. Cache the `Arc` once per +/// `(pkg_name, version)` pair so the second pick onwards is an +/// `Arc::clone` instead of a full reserialise. +/// +/// Shared across [`crate::NpmResolver`] and +/// [`crate::NamedRegistryResolver`] for the same reasons +/// [`PackumentFetchLocker`] is — both resolvers can pick the same +/// `(pkg_name, version)` pair in one install. +pub type PickedManifestCache = Arc>>; + +/// Construct a fresh [`PickedManifestCache`] for a new install. +pub fn shared_picked_manifest_cache() -> PickedManifestCache { + Arc::new(DashMap::new()) +} + /// Default thread-safe [`PackageMetaCache`] backed by a [`Mutex`] /// guarding a [`HashMap`]. A consumer that already has its own /// shared map can implement the trait directly instead of using diff --git a/pacquet/crates/resolving-npm-resolver/tests/chain.rs b/pacquet/crates/resolving-npm-resolver/tests/chain.rs index 0ddfdc1bf2..3eba2d9640 100644 --- a/pacquet/crates/resolving-npm-resolver/tests/chain.rs +++ b/pacquet/crates/resolving-npm-resolver/tests/chain.rs @@ -22,7 +22,7 @@ use pacquet_resolving_local_resolver::{ }; use pacquet_resolving_npm_resolver::{ InMemoryPackageMetaCache, NamedRegistryResolver, merge_named_registries, - shared_packument_fetch_locker, + shared_packument_fetch_locker, shared_picked_manifest_cache, }; use pacquet_resolving_resolver_base::{ResolveOptions, WantedDependency}; use tempfile::TempDir; @@ -39,6 +39,7 @@ fn named_registry_resolver( auth_headers: Arc::new(AuthHeaders::default()), meta_cache: Arc::new(InMemoryPackageMetaCache::default()), fetch_locker: shared_packument_fetch_locker(), + picked_manifest_cache: shared_picked_manifest_cache(), // No cache_dir means no on-disk mirror — every fetch goes // through the network. The link / workspace / file tests never // hit named-registry, so this is fine without mocks.