Repository: denoland/deno
Pull request: perf: support negative caching of package.json

This is useful for single pass resolvers, like `@deno/loader`. There the probe overhead is so large that package.json probing accounts for up to 90% of readfile calls in a large project.

<!--
Before submitting a PR, please read https://docs.deno.com/runtime/manual/references/contributing

1. Give the PR a descriptive title.

  Examples of good title:
    - fix(std/http): Fix race condition in server
    - docs(console): Update docstrings
    - feat(doc): Handle nested reexports

  Examples of bad title:
    - fix #7123
    - update docs
    - fix bugs

2. Ensure there is a related issue and it is referenced in the PR text.
3. Ensure there are tests that cover the changes.
4. Ensure `cargo test` passes.
5. Ensure `./tools/format.js` passes without changing files.
6. Ensure `./tools/lint.js` passes.
7. Open as a draft PR if your work is still in progress. The CI won't run
   all steps, but you can add '[ci]' to a commit message to force it to.
8. If you would like to run the benchmarks on the CI, add the 'ci-bench' label.
-->


Changed files:
filename: cli/lsp/config.rs
patch: @@ -43,6 +43,7 @@ use deno_npm_installer::NpmInstallerFactoryOptions;
 use deno_npm_installer::graph::NpmCachingStrategy;
 use deno_npm_installer::lifecycle_scripts::NullLifecycleScriptsExecutor;
 use deno_package_json::PackageJsonCache;
+use deno_package_json::PackageJsonCacheResult;
 use deno_path_util::url_to_file_path;
 use deno_resolver::factory::ConfigDiscoveryOption;
 use deno_resolver::factory::ResolverFactory;
@@ -2036,11 +2037,20 @@ impl deno_config::deno_json::DenoJsonCache for DenoJsonMemCache {
 struct PackageJsonMemCache(Mutex<HashMap<PathBuf, Arc<PackageJson>>>);
 
 impl deno_package_json::PackageJsonCache for PackageJsonMemCache {
-  fn get(&self, path: &Path) -> Option<Arc<PackageJson>> {
-    self.0.lock().get(path).cloned()
+  fn get(&self, path: &Path) -> PackageJsonCacheResult {
+    self
+      .0
+      .lock()
+      .get(path)
+      .cloned()
+      .map(|value| PackageJsonCacheResult::Hit(Some(value)))
+      .unwrap_or_else(|| PackageJsonCacheResult::NotCached)
   }
 
-  fn set(&self, path: PathBuf, data: Arc<PackageJson>) {
+  fn set(&self, path: PathBuf, data: Option<Arc<PackageJson>>) {
+    let Some(data) = data else {
+      return;
+    };
     self.0.lock().insert(path, data);
   }
 }

filename: libs/config/workspace/discovery.rs
patch: @@ -278,7 +278,7 @@ fn discover_workspace_config_files_for_single_dir<
             "package.json file found at '{}'",
             pkg_json_path.display()
           );
-          Ok(Some(pkg_json))
+          Ok(pkg_json)
         }
         Err(PackageJsonLoadError::Io { source, .. })
           if is_skippable_io_error(&source) =>

filename: libs/config/workspace/mod.rs
patch: @@ -2633,6 +2633,7 @@ pub mod test {
   use std::cell::RefCell;
   use std::collections::HashMap;
 
+  use deno_package_json::PackageJsonCacheResult;
   use deno_path_util::normalize_path;
   use deno_path_util::url_from_directory_path;
   use deno_path_util::url_from_file_path;
@@ -5836,11 +5837,18 @@ pub mod test {
   struct PkgJsonMemCache(RefCell<HashMap<PathBuf, PackageJsonRc>>);
 
   impl deno_package_json::PackageJsonCache for PkgJsonMemCache {
-    fn get(&self, path: &Path) -> Option<PackageJsonRc> {
-      self.0.borrow().get(path).cloned()
+    fn get(&self, path: &Path) -> PackageJsonCacheResult {
+      match self.0.borrow().get(path).cloned() {
+        Some(value) => PackageJsonCacheResult::Hit(Some(value)),
+        None => PackageJsonCacheResult::NotCached,
+      }
     }
 
-    fn set(&self, path: PathBuf, value: PackageJsonRc) {
+    fn set(&self, path: PathBuf, value: Option<PackageJsonRc>) {
+      let Some(value) = value else {
+        // Don't cache misses (no negative cache).
+        return;
+      };
       self.0.borrow_mut().insert(path, value);
     }
   }

filename: libs/node_resolver/package_json.rs
patch: @@ -2,11 +2,11 @@
 
 use std::cell::RefCell;
 use std::collections::HashMap;
-use std::io::ErrorKind;
 use std::path::Path;
 use std::path::PathBuf;
 
 use deno_package_json::PackageJson;
+use deno_package_json::PackageJsonCacheResult;
 use deno_package_json::PackageJsonRc;
 use sys_traits::FsRead;
 
@@ -55,11 +55,18 @@ impl PackageJsonThreadLocalCache {
 }
 
 impl deno_package_json::PackageJsonCache for PackageJsonThreadLocalCache {
-  fn get(&self, path: &Path) -> Option<PackageJsonRc> {
-    CACHE.with_borrow(|cache| cache.get(path).cloned())
+  fn get(&self, path: &Path) -> PackageJsonCacheResult {
+    CACHE.with_borrow(|cache| match cache.get(path).cloned() {
+      Some(value) => PackageJsonCacheResult::Hit(Some(value)),
+      None => PackageJsonCacheResult::NotCached,
+    })
   }
 
-  fn set(&self, path: PathBuf, package_json: PackageJsonRc) {
+  fn set(&self, path: PathBuf, package_json: Option<PackageJsonRc>) {
+    let Some(package_json) = package_json else {
+      // We don't cache misses.
+      return;
+    };
     CACHE.with_borrow_mut(|cache| cache.insert(path, package_json));
   }
 }
@@ -111,12 +118,7 @@ impl<TSys: FsRead> PackageJsonResolver<TSys> {
       path,
     );
     match result {
-      Ok(pkg_json) => Ok(Some(pkg_json)),
-      Err(deno_package_json::PackageJsonLoadError::Io { source, .. })
-        if source.kind() == ErrorKind::NotFound =>
-      {
-        Ok(None)
-      }
+      Ok(pkg_json) => Ok(pkg_json),
       Err(err) => Err(PackageJsonLoadError(err)),
     }
   }

filename: libs/package_json/lib.rs
patch: @@ -5,6 +5,7 @@
 #![deny(clippy::unused_async)]
 #![deny(clippy::unnecessary_wraps)]
 
+use std::io::ErrorKind;
 use std::path::Path;
 use std::path::PathBuf;
 
@@ -30,9 +31,14 @@ pub type PackageJsonDepsRc = deno_maybe_sync::MaybeArc<PackageJsonDeps>;
 #[allow(clippy::disallowed_types)]
 type PackageJsonDepsRcCell = deno_maybe_sync::MaybeOnceLock<PackageJsonDepsRc>;
 
+pub enum PackageJsonCacheResult {
+  Hit(Option<PackageJsonRc>),
+  NotCached,
+}
+
 pub trait PackageJsonCache {
-  fn get(&self, path: &Path) -> Option<PackageJsonRc>;
-  fn set(&self, path: PathBuf, package_json: PackageJsonRc);
+  fn get(&self, path: &Path) -> PackageJsonCacheResult;
+  fn set(&self, path: PathBuf, package_json: Option<PackageJsonRc>);
 }
 
 #[derive(Debug, Clone, JsError, PartialEq, Eq, Boxed)]
@@ -240,24 +246,36 @@ impl PackageJson {
     sys: &impl FsRead,
     maybe_cache: Option<&dyn PackageJsonCache>,
     path: &Path,
-  ) -> Result<PackageJsonRc, PackageJsonLoadError> {
-    match maybe_cache.and_then(|c| c.get(path)) {
-      Some(item) => Ok(item),
-      _ => match sys.fs_read_to_string_lossy(path) {
-        Ok(file_text) => {
-          let pkg_json =
-            PackageJson::load_from_string(path.to_path_buf(), &file_text)?;
-          let pkg_json = deno_maybe_sync::new_rc(pkg_json);
-          if let Some(cache) = maybe_cache {
-            cache.set(path.to_path_buf(), pkg_json.clone());
+  ) -> Result<Option<PackageJsonRc>, PackageJsonLoadError> {
+    let cache_entry = maybe_cache
+      .map(|c| c.get(path))
+      .unwrap_or(PackageJsonCacheResult::NotCached);
+
+    match cache_entry {
+      PackageJsonCacheResult::Hit(item) => Ok(item),
+      PackageJsonCacheResult::NotCached => {
+        match sys.fs_read_to_string_lossy(path) {
+          Ok(file_text) => {
+            let pkg_json =
+              PackageJson::load_from_string(path.to_path_buf(), &file_text)?;
+            let pkg_json = deno_maybe_sync::new_rc(pkg_json);
+            if let Some(cache) = maybe_cache {
+              cache.set(path.to_path_buf(), Some(pkg_json.clone()));
+            }
+            Ok(Some(pkg_json))
+          }
+          Err(err) if err.kind() == ErrorKind::NotFound => {
+            if let Some(cache) = maybe_cache {
+              cache.set(path.to_path_buf(), None);
+            }
+            Ok(None)
           }
-          Ok(pkg_json)
+          Err(err) => Err(PackageJsonLoadError::Io {
+            path: path.to_path_buf(),
+            source: err,
+          }),
         }
-        Err(err) => Err(PackageJsonLoadError::Io {
-          path: path.to_path_buf(),
-          source: err,
-        }),
-      },
+      }
     }
   }