perf: do not clone version information (#78)

src/registry.rs

@@ -32,8 +32,8 @@ impl NpmPackageInfo {
   pub fn version_info(
     &self,
     nv: &PackageNv,
-  ) -> Result<NpmPackageVersionInfo, NpmPackageVersionNotFound> {
-    match self.versions.get(&nv.version).cloned() {
+  ) -> Result<&NpmPackageVersionInfo, NpmPackageVersionNotFound> {
+    match self.versions.get(&nv.version) {
       Some(version_info) => Ok(version_info),
       None => Err(NpmPackageVersionNotFound(nv.clone())),
     }
@@ -388,15 +388,15 @@ pub trait NpmRegistryApi {
 /// purposes. Construct everything on the same thread.
 #[derive(Clone, Default, Debug)]
 pub struct TestNpmRegistryApi {
-  package_infos: Rc<RefCell<HashMap<String, NpmPackageInfo>>>,
+  package_infos: Rc<RefCell<HashMap<String, Arc<NpmPackageInfo>>>>,
 }
 
 impl TestNpmRegistryApi {
   pub fn add_package_info(&self, name: &str, info: NpmPackageInfo) {
     let previous = self
       .package_infos
       .borrow_mut()
-      .insert(name.to_string(), info);
+      .insert(name.to_string(), Arc::new(info));
     assert!(previous.is_none());
   }
 
@@ -415,8 +415,9 @@ impl TestNpmRegistryApi {
   pub fn with_package(&self, name: &str, f: impl FnOnce(&mut NpmPackageInfo)) {
     self.ensure_package(name);
     let mut infos = self.package_infos.borrow_mut();
-    let info = infos.get_mut(name).unwrap();
-    f(info);
+    let mut info = infos.get_mut(name).unwrap().as_ref().clone();
+    f(&mut info);
+    infos.insert(name.to_string(), Arc::new(info));
   }
 
   pub fn add_dist_tag(&self, package_name: &str, tag: &str, version: &str) {
@@ -438,22 +439,22 @@ impl TestNpmRegistryApi {
     integrity: Option<&str>,
   ) {
     self.ensure_package(name);
-    let mut infos = self.package_infos.borrow_mut();
-    let info = infos.get_mut(name).unwrap();
-    let version = Version::parse_from_npm(version).unwrap();
-    if !info.versions.contains_key(&version) {
-      info.versions.insert(
-        version.clone(),
-        NpmPackageVersionInfo {
-          version,
-          dist: NpmPackageVersionDistInfo {
-            integrity: integrity.map(|s| s.to_string()),
+    self.with_package(name, |info| {
+      let version = Version::parse_from_npm(version).unwrap();
+      if !info.versions.contains_key(&version) {
+        info.versions.insert(
+          version.clone(),
+          NpmPackageVersionInfo {
+            version,
+            dist: NpmPackageVersionDistInfo {
+              integrity: integrity.map(|s| s.to_string()),
+              ..Default::default()
+            },
             ..Default::default()
           },
-          ..Default::default()
-        },
-      );
-    }
+        );
+      }
+    })
   }
 
   pub fn with_version_info(
@@ -463,11 +464,11 @@ impl TestNpmRegistryApi {
   ) {
     let (name, version) = package;
     self.ensure_package_version(name, version);
-    let mut infos = self.package_infos.borrow_mut();
-    let info = infos.get_mut(name).unwrap();
-    let version = Version::parse_from_npm(version).unwrap();
-    let version_info = info.versions.get_mut(&version).unwrap();
-    f(version_info);
+    self.with_package(name, |info| {
+      let version = Version::parse_from_npm(version).unwrap();
+      let version_info = info.versions.get_mut(&version).unwrap();
+      f(version_info);
+    });
   }
 
   pub fn add_dependency(&self, package: (&str, &str), entry: (&str, &str)) {
@@ -537,11 +538,11 @@ impl NpmRegistryApi for TestNpmRegistryApi {
     name: &str,
   ) -> Result<Arc<NpmPackageInfo>, NpmRegistryPackageInfoLoadError> {
     let infos = self.package_infos.borrow();
-    Ok(Arc::new(infos.get(name).cloned().ok_or_else(|| {
+    Ok(infos.get(name).cloned().ok_or_else(|| {
       NpmRegistryPackageInfoLoadError::PackageNotExists {
         package_name: name.into(),
       }
-    })?))
+    })?)
   }
 }
 

src/resolution/graph.rs

@@ -971,7 +971,7 @@ impl<'a, TNpmRegistryApi: NpmRegistryApi>
           let version_info = package_info
             .version_info(&pkg_nv)
             .map_err(NpmPackageVersionResolutionError::VersionNotFound)?;
-          self.dep_entry_cache.store(pkg_nv.clone(), &version_info)?
+          self.dep_entry_cache.store(pkg_nv.clone(), version_info)?
         };
 
         (pkg_nv, deps)

src/resolution/snapshot.rs

@@ -882,7 +882,7 @@ pub fn incomplete_snapshot_from_lockfile(
   })
 }
 
-#[derive(Debug, Error)]
+#[derive(Debug, Clone, Error)]
 #[error("Integrity check failed for package: \"{package_display_id}\". Unable to verify that the package
 is the same as when the lockfile was generated.
 
@@ -902,7 +902,7 @@ pub struct IntegrityCheckFailedError {
   pub filename: String,
 }
 
-#[derive(Debug, Error)]
+#[derive(Debug, Error, Clone)]
 pub enum SnapshotFromLockfileError {
   #[error(transparent)]
   PackageInfoLoad(#[from] NpmRegistryPackageInfoLoadError),
@@ -947,15 +947,21 @@ pub async fn snapshot_from_lockfile<'a>(
         .package_info(&nv.name)
         .await
         .map_err(SnapshotFromLockfileError::PackageInfoLoad)?;
-      package_info
-        .version_info(nv)
-        .map_err(|e| SnapshotFromLockfileError::VersionNotFound { source: e })
+      Ok((package_info, nv))
     }))
   };
   let mut version_infos = get_version_infos();
   let mut i = 0;
   let mut packages = Vec::with_capacity(incomplete_snapshot.packages.len());
   while let Some(result) = version_infos.next().await {
+    let result = result
+      .as_ref()
+      .map_err(|e: &SnapshotFromLockfileError| e.clone())
+      .and_then(|(package_info, nv)| {
+        package_info
+          .version_info(nv)
+          .map_err(|e| SnapshotFromLockfileError::VersionNotFound { source: e })
+      });
     match result {
       Ok(version_info) => {
         let snapshot_package = &incomplete_snapshot.packages[i];
@@ -982,14 +988,15 @@ pub async fn snapshot_from_lockfile<'a>(
         packages.push(SerializedNpmResolutionSnapshotPackage {
           id: snapshot_package.id.clone(),
           dependencies: snapshot_package.dependencies.clone(),
-          dist: version_info.dist,
+          dist: version_info.dist.clone(),
           system: NpmResolutionPackageSystemInfo {
-            cpu: version_info.cpu,
-            os: version_info.os,
+            cpu: version_info.cpu.clone(),
+            os: version_info.os.clone(),
           },
           optional_dependencies: version_info
             .optional_dependencies
-            .into_keys()
+            .keys()
+            .cloned()
             .collect(),
           bin: version_info.bin.clone(),
           scripts: version_info.scripts.clone(),