From 3e8f29ae4123abaddd9544a87e16448219fdd5f7 Mon Sep 17 00:00:00 2001
From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com>
Date: Tue, 28 May 2024 11:59:17 -0700
Subject: [PATCH] perf(cli): Optimize setting up `node_modules` on macOS
 (#23980)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Hard linking (`linkat`) is ridiculously slow on mac. `copyfile` is
better, but what's even faster is `clonefile`. It doesn't have the space
savings that comes with hardlinking, but the performance difference is
worth it imo.

```
❯ hyperfine -i -p 'rm -rf node_modules/' '../../d7/target/release/deno cache npm:@11ty/eleventy' 'deno cache npm:@11ty/eleventy'
Benchmark 1: ../../d7/target/release/deno cache npm:@11ty/eleventy
  Time (mean ± σ):     115.4 ms ±   1.2 ms    [User: 27.2 ms, System: 87.3 ms]
  Range (min … max):   113.7 ms … 117.5 ms    10 runs

Benchmark 2: deno cache npm:@11ty/eleventy
  Time (mean ± σ):     619.3 ms ±   6.4 ms    [User: 34.3 ms, System: 575.6 ms]
  Range (min … max):   612.2 ms … 633.3 ms    10 runs

Summary
  ../../d7/target/release/deno cache npm:@11ty/eleventy ran
    5.37 ± 0.08 times faster than deno cache npm:@11ty/eleventy
```
---
 cli/npm/managed/resolvers/local.rs | 18 ++------
 cli/util/fs.rs                     | 68 ++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 14 deletions(-)

diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs
index 055fdfb23a..5362d2f611 100644
--- a/cli/npm/managed/resolvers/local.rs
+++ b/cli/npm/managed/resolvers/local.rs
@@ -17,6 +17,7 @@ use crate::cache::CACHE_PERM;
 use crate::npm::cache_dir::mixed_case_package_name_decode;
 use crate::util::fs::atomic_write_file;
 use crate::util::fs::canonicalize_path_maybe_not_exists_with_fs;
+use crate::util::fs::clone_dir_recursive;
 use crate::util::fs::symlink_dir;
 use crate::util::fs::LaxSingleProcessFsFlag;
 use crate::util::progress_bar::ProgressBar;
@@ -44,8 +45,6 @@ use serde::Deserialize;
 use serde::Serialize;
 
 use crate::npm::cache_dir::mixed_case_package_name_encode;
-use crate::util::fs::copy_dir_recursive;
-use crate::util::fs::hard_link_dir_recursive;
 
 use super::super::super::common::types_package_name;
 use super::super::cache::NpmCache;
@@ -331,16 +330,9 @@ async fn sync_resolution_with_fs(
         let sub_node_modules = folder_path.join("node_modules");
         let package_path =
           join_package_name(&sub_node_modules, &package.id.nv.name);
-        fs::create_dir_all(&package_path)
-          .with_context(|| format!("Creating '{}'", folder_path.display()))?;
         let cache_folder =
           cache.package_folder_for_name_and_version(&package.id.nv);
-        if hard_link_dir_recursive(&cache_folder, &package_path).is_err() {
-          // Fallback to copying the directory.
-          //
-          // Also handles EXDEV when when trying to hard link across volumes.
-          copy_dir_recursive(&cache_folder, &package_path)?;
-        }
+        clone_dir_recursive(&cache_folder, &package_path)?;
         // write out a file that indicates this folder has been initialized
         fs::write(initialized_file, "")?;
 
@@ -373,9 +365,7 @@ async fn sync_resolution_with_fs(
       let sub_node_modules = destination_path.join("node_modules");
       let package_path =
         join_package_name(&sub_node_modules, &package.id.nv.name);
-      fs::create_dir_all(&package_path).with_context(|| {
-        format!("Creating '{}'", destination_path.display())
-      })?;
+
       let source_path = join_package_name(
         &deno_local_registry_dir
           .join(get_package_folder_id_folder_name(
@@ -384,7 +374,7 @@ async fn sync_resolution_with_fs(
           .join("node_modules"),
         &package.id.nv.name,
       );
-      hard_link_dir_recursive(&source_path, &package_path)?;
+      clone_dir_recursive(&source_path, &package_path)?;
       // write out a file that indicates this folder has been initialized
       fs::write(initialized_file, "")?;
     }
diff --git a/cli/util/fs.rs b/cli/util/fs.rs
index fdc7855e62..9bdb1d014e 100644
--- a/cli/util/fs.rs
+++ b/cli/util/fs.rs
@@ -492,6 +492,74 @@ pub async fn remove_dir_all_if_exists(path: &Path) -> std::io::Result<()> {
   }
 }
 
+mod clone_dir_imp {
+
+  #[cfg(target_vendor = "apple")]
+  mod apple {
+    use super::super::copy_dir_recursive;
+    use deno_core::error::AnyError;
+    use std::os::unix::ffi::OsStrExt;
+    use std::path::Path;
+    fn clonefile(from: &Path, to: &Path) -> std::io::Result<()> {
+      let from = std::ffi::CString::new(from.as_os_str().as_bytes())?;
+      let to = std::ffi::CString::new(to.as_os_str().as_bytes())?;
+      // SAFETY: `from` and `to` are valid C strings.
+      let ret = unsafe { libc::clonefile(from.as_ptr(), to.as_ptr(), 0) };
+      if ret != 0 {
+        return Err(std::io::Error::last_os_error());
+      }
+      Ok(())
+    }
+
+    pub fn clone_dir_recursive(from: &Path, to: &Path) -> Result<(), AnyError> {
+      if let Some(parent) = to.parent() {
+        std::fs::create_dir_all(parent)?;
+      }
+      // Try to clone the whole directory
+      if let Err(err) = clonefile(from, to) {
+        if err.kind() != std::io::ErrorKind::AlreadyExists {
+          log::warn!(
+            "Failed to clone dir {:?} to {:?} via clonefile: {}",
+            from,
+            to,
+            err
+          );
+        }
+        // clonefile won't overwrite existing files, so if the dir exists
+        // we need to handle it recursively.
+        copy_dir_recursive(from, to)?;
+      }
+
+      Ok(())
+    }
+  }
+
+  #[cfg(target_vendor = "apple")]
+  pub(super) use apple::clone_dir_recursive;
+
+  #[cfg(not(target_vendor = "apple"))]
+  pub(super) fn clone_dir_recursive(
+    from: &std::path::Path,
+    to: &std::path::Path,
+  ) -> Result<(), deno_core::error::AnyError> {
+    if let Err(e) = super::hard_link_dir_recursive(from, to) {
+      log::debug!("Failed to hard link dir {:?} to {:?}: {}", from, to, e);
+      super::copy_dir_recursive(from, to)?;
+    }
+
+    Ok(())
+  }
+}
+
+/// Clones a directory to another directory. The exact method
+/// is not guaranteed - it may be a hardlink, copy, or other platform-specific
+/// operation.
+///
+/// Note: Does not handle symlinks.
+pub fn clone_dir_recursive(from: &Path, to: &Path) -> Result<(), AnyError> {
+  clone_dir_imp::clone_dir_recursive(from, to)
+}
+
 /// Copies a directory to another directory.
 ///
 /// Note: Does not handle symlinks.