perf: move jsr manifest checksum to locker (#488)

src/graph.rs

@@ -174,7 +174,7 @@ pub enum JsrLoadError {
   ContentLoadExternalSpecifier,
   #[error(transparent)]
   ContentLoad(Arc<anyhow::Error>),
-  #[error("JSR package manifest for '{}' failed to load: {:#}", .0, .1)]
+  #[error("JSR package manifest for '{}' failed to load. {:#}", .0, .1)]
   PackageManifestLoad(String, Arc<anyhow::Error>),
   #[error("JSR package not found: {}", .0)]
   PackageNotFound(String),
@@ -3057,7 +3057,7 @@ struct PendingModuleLoadItem {
 
 struct LoadedJsrPackageViaHttpsUrl {
   nv: PackageNv,
-  manifest_checksum: String,
+  manifest_checksum_for_locker: Option<LoaderChecksum>,
 }
 
 type PendingInfoFuture<'a> = LocalBoxFuture<'a, PendingInfo>;
@@ -3282,10 +3282,12 @@ impl<'a, 'graph> Builder<'a, 'graph> {
           loaded_package_via_https_url,
         }) => {
           if let Some(pkg) = loaded_package_via_https_url {
-            self
-              .graph
-              .packages
-              .ensure_package(pkg.nv, pkg.manifest_checksum);
+            if let Some(locker) = &mut self.locker {
+              if let Some(checksum) = pkg.manifest_checksum_for_locker {
+                locker.set_pkg_manifest_checksum(&pkg.nv, checksum);
+              }
+            }
+            self.graph.packages.ensure_package(pkg.nv);
           }
 
           match result {
@@ -3459,10 +3461,12 @@ impl<'a, 'graph> Builder<'a, 'graph> {
       match version_info_result {
         Ok(version_info_load_item) => {
           let version_info = version_info_load_item.info;
-          self
-            .graph
-            .packages
-            .ensure_package(nv.clone(), version_info_load_item.checksum);
+          self.graph.packages.ensure_package(nv.clone());
+          if let Some(locker) = &mut self.locker {
+            if let Some(checksum) = version_info_load_item.checksum_for_locker {
+              locker.set_pkg_manifest_checksum(nv, checksum);
+            }
+          }
           let base_url = self.jsr_url_provider.package_url(nv);
           let export_name =
             normalize_export_name(resolution_item.nv_ref.sub_path());
@@ -4277,7 +4281,7 @@ impl<'a, 'graph> Builder<'a, 'graph> {
       maybe_checksum = self
         .locker
         .as_ref()
-        .and_then(|l| l.get_checksum(&requested_specifier));
+        .and_then(|l| l.get_remote_checksum(&requested_specifier));
     }
     let fut = async move {
       #[allow(clippy::too_many_arguments)]
@@ -4328,7 +4332,7 @@ impl<'a, 'graph> Builder<'a, 'graph> {
           maybe_version_info.replace(info);
           loaded_package_via_https_url.replace(LoadedJsrPackageViaHttpsUrl {
             nv: package_nv,
-            manifest_checksum: inner.checksum,
+            manifest_checksum_for_locker: inner.checksum_for_locker,
           });
         }
 
@@ -4417,7 +4421,11 @@ impl<'a, 'graph> Builder<'a, 'graph> {
             Ok(err) => Err(ModuleError::LoadingErr(
               load_specifier.clone(),
               maybe_range.cloned(),
-              ModuleLoadError::HttpsChecksumIntegrity(err),
+              if maybe_version_info.is_some() {
+                JsrLoadError::ContentChecksumIntegrity(err).into()
+              } else {
+                ModuleLoadError::HttpsChecksumIntegrity(err)
+              },
             )),
             Err(err) => Err(ModuleError::LoadingErr(
               load_specifier.clone(),
@@ -4474,7 +4482,7 @@ impl<'a, 'graph> Builder<'a, 'graph> {
     self.state.jsr.metadata.queue_load_package_version_info(
       package_nv,
       self.fill_pass_mode.to_cache_setting(),
-      self.graph.packages.get_manifest_checksum(package_nv),
+      self.locker.as_deref(),
       JsrMetadataStoreServices {
         executor: self.executor,
         jsr_url_provider: self.jsr_url_provider,
@@ -4551,8 +4559,8 @@ impl<'a, 'graph> Builder<'a, 'graph> {
           && matches!(specifier.scheme(), "https" | "http")
         {
           if let Some(locker) = &mut self.locker {
-            if !locker.has_checksum(&specifier) {
-              locker.set_checksum(
+            if !locker.has_remote_checksum(&specifier) {
+              locker.set_remote_checksum(
                 &specifier,
                 LoaderChecksum::new(LoaderChecksum::gen(
                   module_source_and_info.source().as_bytes(),

src/jsr.rs

@@ -20,11 +20,12 @@ use crate::source::LoadOptions;
 use crate::source::LoadResponse;
 use crate::source::Loader;
 use crate::source::LoaderChecksum;
+use crate::source::Locker;
 use crate::Executor;
 
 #[derive(Clone)]
 pub struct PendingJsrPackageVersionInfoLoadItem {
-  pub checksum: String,
+  pub checksum_for_locker: Option<LoaderChecksum>,
   pub info: Arc<JsrPackageVersionInfo>,
 }
 
@@ -103,7 +104,7 @@ impl JsrMetadataStore {
     &mut self,
     package_nv: &PackageNv,
     cache_setting: CacheSetting,
-    maybe_expected_checksum: Option<&str>,
+    maybe_locker: Option<&dyn Locker>,
     services: JsrMetadataStoreServices,
   ) {
     if self
@@ -121,24 +122,27 @@ impl JsrMetadataStore {
         package_nv.name, package_nv.version
       ))
       .unwrap();
-    let maybe_expected_checksum =
-      maybe_expected_checksum.map(|c| LoaderChecksum::new(c.to_string()));
+    let maybe_expected_checksum = maybe_locker
+      .as_ref()
+      .and_then(|locker| locker.get_pkg_manifest_checksum(package_nv));
+    let should_compute_checksum =
+      maybe_expected_checksum.is_none() && maybe_locker.is_some();
     let fut = self.load_data(
       specifier,
       services,
       cache_setting,
-      // we won't have a checksum when loading this the
-      // first time or when not using a lockfile
-      maybe_expected_checksum.clone(),
-      |content| {
-        // if we have the expected checksum, then we can re-use that here
-        let checksum = maybe_expected_checksum
-          .map(|c| c.into_string())
-          .unwrap_or_else(|| LoaderChecksum::gen(content));
+      // we won't have a checksum when not using a lockfile
+      maybe_expected_checksum,
+      move |content| {
+        let checksum_for_locker = if should_compute_checksum {
+          Some(LoaderChecksum::new(LoaderChecksum::gen(content)))
+        } else {
+          None
+        };
         let version_info: JsrPackageVersionInfo =
           serde_json::from_slice(content)?;
         Ok(PendingJsrPackageVersionInfoLoadItem {
-          checksum,
+          checksum_for_locker,
           info: Arc::new(version_info),
         })
       },

src/packages.rs

@@ -105,19 +105,11 @@ impl JsrPackageVersionInfo {
 
 #[derive(Debug, Clone)]
 struct PackageNvInfo {
-  manifest_checksum: String,
   /// Collection of exports used.
   exports: BTreeMap<String, String>,
   found_dependencies: HashSet<JsrDepPackageReq>,
 }
 
-#[derive(Debug, Clone)]
-pub struct PackageManifestIntegrityError {
-  pub nv: PackageNv,
-  pub actual: String,
-  pub expected: String,
-}
-
 #[derive(Debug, Clone, Default, Serialize)]
 pub struct PackageSpecifiers {
   #[serde(flatten)]
@@ -138,10 +130,20 @@ impl PackageSpecifiers {
     self.package_reqs.is_empty()
   }
 
+  /// The total number of JSR packages found in the graph.
   pub fn packages_len(&self) -> usize {
     self.packages.len()
   }
 
+  /// The total number of dependencies of jsr packages found in the graph.
+  pub fn package_deps_sum(&self) -> usize {
+    self
+      .packages
+      .iter()
+      .map(|p| p.1.found_dependencies.len())
+      .sum()
+  }
+
   pub fn add_nv(&mut self, package_req: PackageReq, nv: PackageNv) {
     let nvs = self
       .packages_by_name
@@ -153,60 +155,21 @@ impl PackageSpecifiers {
     self.package_reqs.insert(package_req, nv.clone());
   }
 
-  pub(crate) fn ensure_package(
-    &mut self,
-    nv: PackageNv,
-    manifest_checksum: String,
-  ) {
+  pub(crate) fn ensure_package(&mut self, nv: PackageNv) {
     self.packages.entry(nv).or_insert_with(|| PackageNvInfo {
-      manifest_checksum,
       exports: Default::default(),
       found_dependencies: Default::default(),
     });
   }
 
-  pub(crate) fn get_manifest_checksum(&self, nv: &PackageNv) -> Option<&str> {
-    self.packages.get(nv).map(|p| p.manifest_checksum.as_str())
-  }
-
-  pub fn add_manifest_checksum(
-    &mut self,
-    nv: PackageNv,
-    checksum: String,
-  ) -> Result<(), Box<PackageManifestIntegrityError>> {
-    let package = self.packages.get(&nv);
-    if let Some(package) = package {
-      if package.manifest_checksum != checksum {
-        Err(Box::new(PackageManifestIntegrityError {
-          nv,
-          actual: checksum,
-          expected: package.manifest_checksum.clone(),
-        }))
-      } else {
-        Ok(())
-      }
-    } else {
-      self.packages.insert(
-        nv,
-        PackageNvInfo {
-          manifest_checksum: checksum,
-          exports: Default::default(),
-          found_dependencies: Default::default(),
-        },
-      );
-      Ok(())
-    }
-  }
-
   /// Gets the dependencies (package constraints) of JSR packages found in the graph.
-  pub fn packages_with_checksum_and_deps(
+  pub fn packages_with_deps(
     &self,
-  ) -> impl Iterator<
-    Item = (&PackageNv, &String, impl Iterator<Item = &JsrDepPackageReq>),
-  > {
+  ) -> impl Iterator<Item = (&PackageNv, impl Iterator<Item = &JsrDepPackageReq>)>
+  {
     self.packages.iter().map(|(nv, info)| {
       let deps = info.found_dependencies.iter();
-      (nv, &info.manifest_checksum, deps)
+      (nv, deps)
     })
   }
 

src/source/mod.rs

@@ -175,43 +175,77 @@ impl fmt::Display for LoaderChecksum {
 }
 
 pub trait Locker {
-  fn get_checksum(&self, specifier: &ModuleSpecifier)
-    -> Option<LoaderChecksum>;
-  fn has_checksum(&self, specifier: &ModuleSpecifier) -> bool;
-  fn set_checksum(
+  fn get_remote_checksum(
+    &self,
+    specifier: &ModuleSpecifier,
+  ) -> Option<LoaderChecksum>;
+  fn has_remote_checksum(&self, specifier: &ModuleSpecifier) -> bool;
+  fn set_remote_checksum(
     &mut self,
     specifier: &ModuleSpecifier,
     checksum: LoaderChecksum,
   );
+
+  fn get_pkg_manifest_checksum(
+    &self,
+    package_nv: &PackageNv,
+  ) -> Option<LoaderChecksum>;
+  fn set_pkg_manifest_checksum(
+    &mut self,
+    package_nv: &PackageNv,
+    checksum: LoaderChecksum,
+  );
 }
 
 #[derive(Debug, Default, Clone)]
-pub struct HashMapLocker(HashMap<ModuleSpecifier, LoaderChecksum>);
+pub struct HashMapLocker {
+  remote: HashMap<ModuleSpecifier, LoaderChecksum>,
+  pkg_manifests: HashMap<PackageNv, LoaderChecksum>,
+}
 
 impl HashMapLocker {
-  pub fn inner(&self) -> &HashMap<ModuleSpecifier, LoaderChecksum> {
-    &self.0
+  pub fn remote(&self) -> &HashMap<ModuleSpecifier, LoaderChecksum> {
+    &self.remote
+  }
+
+  pub fn pkg_manifests(&self) -> &HashMap<PackageNv, LoaderChecksum> {
+    &self.pkg_manifests
   }
 }
 
 impl Locker for HashMapLocker {
-  fn get_checksum(
+  fn get_remote_checksum(
     &self,
     specifier: &ModuleSpecifier,
   ) -> Option<LoaderChecksum> {
-    self.0.get(specifier).cloned()
+    self.remote.get(specifier).cloned()
   }
 
-  fn has_checksum(&self, specifier: &ModuleSpecifier) -> bool {
-    self.0.contains_key(specifier)
+  fn has_remote_checksum(&self, specifier: &ModuleSpecifier) -> bool {
+    self.remote.contains_key(specifier)
   }
 
-  fn set_checksum(
+  fn set_remote_checksum(
     &mut self,
     specifier: &ModuleSpecifier,
     checksum: LoaderChecksum,
   ) {
-    self.0.insert(specifier.clone(), checksum);
+    self.remote.insert(specifier.clone(), checksum);
+  }
+
+  fn get_pkg_manifest_checksum(
+    &self,
+    package_nv: &PackageNv,
+  ) -> Option<LoaderChecksum> {
+    self.pkg_manifests.get(package_nv).cloned()
+  }
+
+  fn set_pkg_manifest_checksum(
+    &mut self,
+    package_nv: &PackageNv,
+    checksum: LoaderChecksum,
+  ) {
+    self.pkg_manifests.insert(package_nv.clone(), checksum);
   }
 }