Skip to content

Commit

Permalink
perf(npm): parallelize caching of npm specifier package infos (#16323)
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret authored Oct 17, 2022
1 parent 60dd84a commit 9df8d9d
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 11 deletions.
6 changes: 6 additions & 0 deletions cli/npm/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
26 changes: 22 additions & 4 deletions cli/npm/resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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| {
Expand Down
7 changes: 1 addition & 6 deletions cli/npm/resolvers/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion cli/npm/resolvers/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 9df8d9d

Please sign in to comment.