perf(node): cache realpath_sync calls in read permission check (#19379)

cli/npm/resolvers/common.rs

@@ -1,9 +1,11 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
+use std::collections::HashMap;
 use std::io::ErrorKind;
 use std::path::Path;
 use std::path::PathBuf;
 use std::sync::Arc;
+use std::sync::Mutex;
 
 use async_trait::async_trait;
 use deno_ast::ModuleSpecifier;
@@ -54,6 +56,69 @@ pub trait NpmPackageFsResolver: Send + Sync {
   ) -> Result<(), AnyError>;
 }
 
+#[derive(Debug)]
+pub struct RegistryReadPermissionChecker {
+  fs: Arc<dyn FileSystem>,
+  cache: Mutex<HashMap<PathBuf, PathBuf>>,
+  registry_path: PathBuf,
+}
+
+impl RegistryReadPermissionChecker {
+  pub fn new(fs: Arc<dyn FileSystem>, registry_path: PathBuf) -> Self {
+    Self {
+      fs,
+      registry_path,
+      cache: Default::default(),
+    }
+  }
+
+  pub fn ensure_registry_read_permission(
+    &self,
+    permissions: &dyn NodePermissions,
+    path: &Path,
+  ) -> Result<(), AnyError> {
+    // allow reading if it's in the node_modules
+    let is_path_in_node_modules = path.starts_with(&self.registry_path)
+      && path
+        .components()
+        .all(|c| !matches!(c, std::path::Component::ParentDir));
+
+    if is_path_in_node_modules {
+      let mut cache = self.cache.lock().unwrap();
+      let registry_path_canon = match cache.get(&self.registry_path) {
+        Some(canon) => canon.clone(),
+        None => {
+          let canon = self.fs.realpath_sync(&self.registry_path)?;
+          cache.insert(self.registry_path.to_path_buf(), canon.clone());
+          canon
+        }
+      };
+
+      let path_canon = match cache.get(path) {
+        Some(canon) => canon.clone(),
+        None => {
+          let canon = self.fs.realpath_sync(path);
+          if let Err(e) = &canon {
+            if e.kind() == ErrorKind::NotFound {
+              return Ok(());
+            }
+          }
+
+          let canon = canon?;
+          cache.insert(path.to_path_buf(), canon.clone());
+          canon
+        }
+      };
+
+      if path_canon.starts_with(registry_path_canon) {
+        return Ok(());
+      }
+    }
+
+    permissions.check_read(path)
+  }
+}
+
 /// Caches all the packages in parallel.
 pub async fn cache_packages(
   mut packages: Vec<NpmResolutionPackage>,
@@ -90,35 +155,6 @@ pub async fn cache_packages(
   Ok(())
 }
 
-pub fn ensure_registry_read_permission(
-  fs: &Arc<dyn FileSystem>,
-  permissions: &dyn NodePermissions,
-  registry_path: &Path,
-  path: &Path,
-) -> Result<(), AnyError> {
-  // allow reading if it's in the node_modules
-  if path.starts_with(registry_path)
-    && path
-      .components()
-      .all(|c| !matches!(c, std::path::Component::ParentDir))
-  {
-    // todo(dsherret): cache this?
-    if let Ok(registry_path) = fs.realpath_sync(registry_path) {
-      match fs.realpath_sync(path) {
-        Ok(path) if path.starts_with(registry_path) => {
-          return Ok(());
-        }
-        Err(e) if e.kind() == ErrorKind::NotFound => {
-          return Ok(());
-        }
-        _ => {} // ignore
-      }
-    }
-  }
-
-  permissions.check_read(path)
-}
-
 /// Gets the corresponding @types package for the provided package name.
 pub fn types_package_name(package_name: &str) -> String {
   debug_assert!(!package_name.starts_with("@types/"));

cli/npm/resolvers/global.rs

@@ -23,18 +23,18 @@ use crate::npm::resolution::NpmResolution;
 use crate::npm::resolvers::common::cache_packages;
 use crate::npm::NpmCache;
 
-use super::common::ensure_registry_read_permission;
 use super::common::types_package_name;
 use super::common::NpmPackageFsResolver;
+use super::common::RegistryReadPermissionChecker;
 
 /// Resolves packages from the global npm cache.
 #[derive(Debug)]
 pub struct GlobalNpmPackageResolver {
-  fs: Arc<dyn FileSystem>,
   cache: Arc<NpmCache>,
   resolution: Arc<NpmResolution>,
   registry_url: Url,
   system_info: NpmSystemInfo,
+  registry_read_permission_checker: RegistryReadPermissionChecker,
 }
 
 impl GlobalNpmPackageResolver {
@@ -46,11 +46,14 @@ impl GlobalNpmPackageResolver {
     system_info: NpmSystemInfo,
   ) -> Self {
     Self {
-      fs,
+      cache: cache.clone(),
-      cache,
       resolution,
-      registry_url,
+      registry_url: registry_url.clone(),
       system_info,
+      registry_read_permission_checker: RegistryReadPermissionChecker::new(
+        fs,
+        cache.registry_folder(&registry_url),
+      ),
     }
   }
 
@@ -156,7 +159,8 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
     permissions: &dyn NodePermissions,
     path: &Path,
   ) -> Result<(), AnyError> {
-    let registry_path = self.cache.registry_folder(&self.registry_url);
+    self
-    ensure_registry_read_permission(&self.fs, permissions, &registry_path, path)
+      .registry_read_permission_checker
+      .ensure_registry_read_permission(permissions, path)
   }
 }

cli/npm/resolvers/local.rs

@@ -41,9 +41,9 @@ use crate::npm::NpmCache;
 use crate::util::fs::copy_dir_recursive;
 use crate::util::fs::hard_link_dir_recursive;
 
-use super::common::ensure_registry_read_permission;
 use super::common::types_package_name;
 use super::common::NpmPackageFsResolver;
+use super::common::RegistryReadPermissionChecker;
 
 /// Resolver that creates a local node_modules directory
 /// and resolves packages from it.
@@ -57,6 +57,7 @@ pub struct LocalNpmPackageResolver {
   root_node_modules_path: PathBuf,
   root_node_modules_url: Url,
   system_info: NpmSystemInfo,
+  registry_read_permission_checker: RegistryReadPermissionChecker,
 }
 
 impl LocalNpmPackageResolver {
@@ -70,15 +71,19 @@ impl LocalNpmPackageResolver {
     system_info: NpmSystemInfo,
   ) -> Self {
     Self {
-      fs,
+      fs: fs.clone(),
       cache,
       progress_bar,
       resolution,
       registry_url,
       root_node_modules_url: Url::from_directory_path(&node_modules_folder)
         .unwrap(),
-      root_node_modules_path: node_modules_folder,
+      root_node_modules_path: node_modules_folder.clone(),
       system_info,
+      registry_read_permission_checker: RegistryReadPermissionChecker::new(
+        fs,
+        node_modules_folder,
+      ),
     }
   }
 
@@ -227,12 +232,9 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
     permissions: &dyn NodePermissions,
     path: &Path,
   ) -> Result<(), AnyError> {
-    ensure_registry_read_permission(
+    self
-      &self.fs,
+      .registry_read_permission_checker
-      permissions,
+      .ensure_registry_read_permission(permissions, path)
-      &self.root_node_modules_path,
-      path,
-    )
   }
 }