From 9df8d9d83140beb24a53b0ac52b1eaf2ea34c095 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 17 Oct 2022 12:27:31 -0400 Subject: [PATCH] perf(npm): parallelize caching of npm specifier package infos (#16323) --- cli/npm/cache.rs | 6 ++++++ cli/npm/resolution.rs | 26 ++++++++++++++++++++++---- cli/npm/resolvers/common.rs | 7 +------ cli/npm/resolvers/local.rs | 2 +- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/cli/npm/cache.rs b/cli/npm/cache.rs index 6b841501ac..77ecf12285 100644 --- a/cli/npm/cache.rs +++ b/cli/npm/cache.rs @@ -23,6 +23,12 @@ use super::semver::NpmVersion; use super::tarball::verify_and_extract_tarball; use super::NpmPackageId; +/// For some of the tests, we want downloading of packages +/// to be deterministic so that the output is always the same +pub fn should_sync_download() -> bool { + std::env::var("DENO_UNSTABLE_NPM_SYNC_DOWNLOAD") == Ok("1".to_string()) +} + pub const NPM_PACKAGE_SYNC_LOCK_FILENAME: &str = ".deno_sync_lock"; #[derive(Clone, Debug)] diff --git a/cli/npm/resolution.rs b/cli/npm/resolution.rs index d53b677142..2ceaa86ab2 100644 --- a/cli/npm/resolution.rs +++ b/cli/npm/resolution.rs @@ -15,6 +15,7 @@ use deno_core::parking_lot::RwLock; use serde::Deserialize; use serde::Serialize; +use super::cache::should_sync_download; use super::registry::NpmPackageInfo; use super::registry::NpmPackageVersionDistInfo; use super::registry::NpmPackageVersionInfo; @@ -386,6 +387,7 @@ impl NpmResolution { // go over the top level packages first, then down the // tree one level at a time through all the branches + let mut unresolved_tasks = Vec::with_capacity(package_reqs.len()); for package_req in package_reqs { if snapshot.package_reqs.contains_key(&package_req) { // skip analyzing this package, as there's already a matching top level package @@ -400,7 +402,24 @@ impl NpmResolution { } // no existing best version, so resolve the current packages - let info = self.api.package_info(&package_req.name).await?; + let api = self.api.clone(); + let maybe_info = if should_sync_download() { + // for deterministic test output + Some(api.package_info(&package_req.name).await) + } else { + None + }; + unresolved_tasks.push(tokio::task::spawn(async move { + let info = match maybe_info { + Some(info) => info?, + None => api.package_info(&package_req.name).await?, + }; + Result::<_, AnyError>::Ok((package_req, info)) + })); + } + + for result in futures::future::join_all(unresolved_tasks).await { + let (package_req, info) = result??; let version_and_info = get_resolved_package_version_and_info( &package_req.name, &package_req, @@ -449,9 +468,8 @@ impl NpmResolution { ordering => ordering, }); - // cache all the dependencies' registry infos in parallel when this env var isn't set - if std::env::var("DENO_UNSTABLE_NPM_SYNC_DOWNLOAD") != Ok("1".to_string()) - { + // cache all the dependencies' registry infos in parallel if should + if !should_sync_download() { let handles = deps .iter() .map(|dep| { diff --git a/cli/npm/resolvers/common.rs b/cli/npm/resolvers/common.rs index 7769d2322f..b91deae9e6 100644 --- a/cli/npm/resolvers/common.rs +++ b/cli/npm/resolvers/common.rs @@ -10,6 +10,7 @@ use deno_core::futures; use deno_core::futures::future::BoxFuture; use deno_core::url::Url; +use crate::npm::cache::should_sync_download; use crate::npm::resolution::NpmResolutionSnapshot; use crate::npm::NpmCache; use crate::npm::NpmPackageReq; @@ -79,12 +80,6 @@ pub async fn cache_packages( Ok(()) } -/// For some of the tests, we want downloading of packages -/// to be deterministic so that the output is always the same -pub fn should_sync_download() -> bool { - std::env::var("DENO_UNSTABLE_NPM_SYNC_DOWNLOAD") == Ok("1".to_string()) -} - pub fn ensure_registry_read_permission( registry_path: &Path, path: &Path, diff --git a/cli/npm/resolvers/local.rs b/cli/npm/resolvers/local.rs index 10ac8abfae..cd79320b79 100644 --- a/cli/npm/resolvers/local.rs +++ b/cli/npm/resolvers/local.rs @@ -20,9 +20,9 @@ use deno_runtime::deno_core::futures; use tokio::task::JoinHandle; use crate::fs_util; +use crate::npm::cache::should_sync_download; use crate::npm::resolution::NpmResolution; use crate::npm::resolution::NpmResolutionSnapshot; -use crate::npm::resolvers::common::should_sync_download; use crate::npm::NpmCache; use crate::npm::NpmPackageId; use crate::npm::NpmPackageReq;