fs: improve cpSync performance

benchmark/fs/bench-cpSync.js

@@ -0,0 +1,23 @@
+'use strict';
+
+const common = require('../common');
+const fs = require('fs');
+const path = require('path');
+const tmpdir = require('../../test/common/tmpdir');
+tmpdir.refresh();
+
+const bench = common.createBenchmark(main, {
+  n: [1, 100, 10_000],
+});
+
+function main({ n }) {
+  tmpdir.refresh();
+  const options = { force: true, recursive: true };
+  const src = path.join(__dirname, '../../test/fixtures/copy');
+  const dest = tmpdir.resolve(`${process.pid}/subdir/cp-bench-${process.pid}`);
+  bench.start();
+  for (let i = 0; i < n; i++) {
+    fs.cpSync(src, dest, options);
+  }
+  bench.end(n);
+}

lib/internal/fs/cp/cp-sync.js

@@ -46,8 +46,41 @@ const {
   resolve,
 } = require('path');
 const { isPromise } = require('util/types');
+const internalFsBinding = internalBinding('fs');
 
+/**
+ *
+ * @param {string} src
+ * @param {string} dest
+ * @param {{
+ *   preserveTimestamps?: boolean,
+ *   filter?: (src: string, dest: string) => boolean,
+ *   dereference?: boolean,
+ *   errorOnExist?: boolean,
+ *   force?: boolean,
+ *   recursive?: boolean,
+ *   mode?: number
+ *   verbatimSymlinks?: boolean
+ * }} opts
+ */
 function cpSyncFn(src, dest, opts) {
+  // We exclude the following options deliberately:
+  // - filter option - calling a js function from C++ is costly.
+  // - `mode` option - not yet implemented
+  // - `dereference` option - it doesn't play well with std::filesystem
+  // - `verbatimSymlinks` option - it doesn't play well with std::filesystem
+  if (opts.filter == null && (opts.mode == null || opts.mode === 0) && !opts.dereference && !opts.verbatimSymlinks) {
+    return internalFsBinding.cpSync(
+      src,
+      dest,
+      opts.preserveTimestamps,
+      opts.errorOnExist,
+      opts.force,
+      opts.recursive,
+    );
+  }
+
+
   // Warn about using preserveTimestamps on 32-bit node
   if (opts.preserveTimestamps && process.arch === 'ia32') {
     const warning = 'Using the preserveTimestamps option in 32-bit ' +

src/node_errors.h

@@ -70,6 +70,14 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
   V(ERR_DLOPEN_FAILED, Error)                                                  \
   V(ERR_ENCODING_INVALID_ENCODED_DATA, TypeError)                              \
   V(ERR_EXECUTION_ENVIRONMENT_NOT_AVAILABLE, Error)                            \
+  V(ERR_FS_CP_EINVAL, Error)                                                   \
+  V(ERR_FS_CP_DIR_TO_NON_DIR, Error)                                           \
+  V(ERR_FS_CP_FIFO_PIPE, Error)                                                \
+  V(ERR_FS_CP_EEXIST, Error)                                                   \
+  V(ERR_FS_CP_NON_DIR_TO_DIR, Error)                                           \
+  V(ERR_FS_CP_SOCKET, Error)                                                   \
+  V(ERR_FS_CP_UNKNOWN, Error)                                                  \
+  V(ERR_FS_EISDIR, Error)                                                      \
   V(ERR_ILLEGAL_CONSTRUCTOR, Error)                                            \
   V(ERR_INVALID_ADDRESS, Error)                                                \
   V(ERR_INVALID_ARG_VALUE, TypeError)                                          \

src/node_file.cc

@@ -2106,6 +2106,175 @@ static void OpenFileHandle(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
+// TODO(@anonrig): Implement v8 fast APi calls for `cpSync`.
+static void CpSync(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  // src, dest, preserveTimestamps, errorOnExist, force, recursive
+  CHECK_EQ(args.Length(), 6);
+  BufferValue src(env->isolate(), args[0]);
+  CHECK_NOT_NULL(*src);
+  ToNamespacedPath(env, &src);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFileSystemRead, src.ToStringView());
+
+  BufferValue dest(env->isolate(), args[1]);
+  CHECK_NOT_NULL(*dest);
+  ToNamespacedPath(env, &dest);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView());
+
+  bool preserveTimestamps = args[2]->IsTrue();
+  bool errorOnExist = args[3]->IsTrue();
+  bool force = args[4]->IsTrue();
+  bool recursive = args[5]->IsTrue();
+
+  using copy_options = std::filesystem::copy_options;
+  using file_type = std::filesystem::file_type;
+
+  std::error_code error_code{};
+  copy_options options = copy_options::copy_symlinks;
+
+  // When true timestamps from src will be preserved.
+  if (preserveTimestamps) options |= copy_options::create_hard_links;
+  // Overwrite existing file or directory.
+  if (force) {
+    options |= copy_options::overwrite_existing;
+  } else {
+    options |= copy_options::skip_existing;
+  }
+  // Copy directories recursively.
+  if (recursive) {
+    options |= copy_options::recursive;
+  }
+
+  auto src_path = std::filesystem::path(src.ToStringView());
+  auto dest_path = std::filesystem::path(dest.ToStringView());
+
+  auto resolved_src = src_path.lexically_normal();
+  auto resolved_dest = dest_path.lexically_normal();
+
+  if (resolved_src == resolved_dest) {
+    std::string message =
+        "src and dest cannot be the same " + resolved_src.string();
+    return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
+  }
+
+  auto get_stat = [](const std::filesystem::path& path)
+      -> std::optional<std::filesystem::file_status> {
+    std::error_code error_code{};
+    auto file_status = std::filesystem::status(path, error_code);
+    if (error_code) {
+      return std::nullopt;
+    }
+    return file_status;
+  };
+
+  auto src_type = get_stat(src_path);
+  auto dest_type = get_stat(dest_path);
+
+  if (!src_type.has_value()) {
+    std::string message = "src path " + src_path.string() + " does not exist";
+    return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
+  }
+
+  const bool src_is_dir = src_type->type() == file_type::directory;
+
+  if (dest_type.has_value()) {
+    // Check if src and dest are identical.
+    if (std::filesystem::equivalent(src_path, dest_path)) {
+      std::string message =
+          "src and dest cannot be the same " + dest_path.string();
+      return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
+    }
+
+    const bool dest_is_dir = dest_type->type() == file_type::directory;
+
+    if (src_is_dir && !dest_is_dir) {
+      std::string message = "Cannot overwrite non-directory " +
+                            src_path.string() + " with directory " +
+                            dest_path.string();
+      return THROW_ERR_FS_CP_DIR_TO_NON_DIR(env, message.c_str());
+    }
+
+    if (!src_is_dir && dest_is_dir) {
+      std::string message = "Cannot overwrite directory " + dest_path.string() +
+                            " with non-directory " + src_path.string();
+      return THROW_ERR_FS_CP_NON_DIR_TO_DIR(env, message.c_str());
+    }
+  }
+
+  if (src_is_dir && dest_path.string().starts_with(src_path.string())) {
+    std::string message = "Cannot copy " + src_path.string() +
+                          " to a subdirectory of self " + dest_path.string();
+    return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
+  }
+
+  auto dest_parent = dest_path.parent_path();
+  // "/" parent is itself. Therefore, we need to check if the parent is the same
+  // as itself.
+  while (src_path.parent_path() != dest_parent &&
+         dest_parent.has_parent_path() &&
+         dest_parent.parent_path() != dest_parent) {
+    if (std::filesystem::equivalent(
+            src_path, dest_path.parent_path(), error_code)) {
+      std::string message = "Cannot copy " + src_path.string() +
+                            " to a subdirectory of self " + dest_path.string();
+      return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
+    }
+
+    // If equivalent fails, it's highly likely that dest_parent does not exist
+    if (error_code) {
+      break;
+    }
+
+    dest_parent = dest_parent.parent_path();
+  }
+
+  if (src_is_dir && !recursive) {
+    std::string message =
+        "Recursive option not enabled, cannot copy a directory: " +
+        src_path.string();
+    return THROW_ERR_FS_EISDIR(env, message.c_str());
+  }
+
+  switch (src_type->type()) {
+    case file_type::socket: {
+      std::string message = "Cannot copy a socket file: " + dest_path.string();
+      return THROW_ERR_FS_CP_SOCKET(env, message.c_str());
+    }
+    case file_type::fifo: {
+      std::string message = "Cannot copy a FIFO pipe: " + dest_path.string();
+      return THROW_ERR_FS_CP_FIFO_PIPE(env, message.c_str());
+    }
+    case file_type::unknown: {
+      std::string message =
+          "Cannot copy an unknown file type: " + dest_path.string();
+      return THROW_ERR_FS_CP_UNKNOWN(env, message.c_str());
+    }
+    default:
+      break;
+  }
+
+  if (dest_type.has_value() && errorOnExist) {
+    std::string message = dest_path.string() + " already exists";
+    return THROW_ERR_FS_CP_EEXIST(env, message.c_str());
+  }
+
+  std::filesystem::create_directories(dest_path.parent_path(), error_code);
+  std::filesystem::copy(src_path, dest_path, options, error_code);
+  if (error_code) {
+    if (error_code == std::errc::file_exists) {
+      std::string message = "File already exists";
+      return THROW_ERR_FS_CP_EEXIST(env, message.c_str());
+    }
+
+    std::string message = "Unhandled error " +
+                          std::to_string(error_code.value()) + ": " +
+                          error_code.message();
+    return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
+  }
+}
+
 static void CopyFile(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   Isolate* isolate = env->isolate();
@@ -3344,6 +3513,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
   SetMethod(isolate, target, "writeFileUtf8", WriteFileUtf8);
   SetMethod(isolate, target, "realpath", RealPath);
   SetMethod(isolate, target, "copyFile", CopyFile);
+  SetMethod(isolate, target, "cpSync", CpSync);
 
   SetMethod(isolate, target, "chmod", Chmod);
   SetMethod(isolate, target, "fchmod", FChmod);
@@ -3466,6 +3636,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(WriteFileUtf8);
   registry->Register(RealPath);
   registry->Register(CopyFile);
+  registry->Register(CpSync);
 
   registry->Register(Chmod);
   registry->Register(FChmod);

test/fixtures/permission/fs-read.js

@@ -161,23 +161,21 @@ const regularFile = __filename;
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemRead',
-    // cpSync calls statSync before reading blockedFile
-    resource: path.toNamespacedPath(blockedFolder),
+    resource: path.toNamespacedPath(blockedFile),
   }));
   assert.throws(() => {
     fs.cpSync(blockedFileURL, path.join(blockedFolder, 'any-other-file'));
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemRead',
-    // cpSync calls statSync before reading blockedFile
-    resource: path.toNamespacedPath(blockedFolder),
+    resource: path.toNamespacedPath(blockedFile),
   }));
   assert.throws(() => {
     fs.cpSync(blockedFile, path.join(__dirname, 'any-other-file'));
   }, common.expectsError({
     code: 'ERR_ACCESS_DENIED',
     permission: 'FileSystemRead',
-    resource: path.toNamespacedPath(__dirname),
+    resource: path.toNamespacedPath(blockedFile),
   }));
 }
 

test/parallel/test-fs-cp.mjs

@@ -155,7 +155,7 @@
   const src = nextdir();
   mkdirSync(src, mustNotMutateObjectDeep({ recursive: true }));
   writeFileSync(join(src, 'foo.js'), 'foo', 'utf8');
-  symlinkSync('foo.js', join(src, 'bar.js'));
+  symlinkSync(join(src, 'foo.js'), join(src, 'bar.js'));
 
   const dest = nextdir();
   mkdirSync(dest, mustNotMutateObjectDeep({ recursive: true }));
@@ -171,7 +171,7 @@
   const src = nextdir();
   mkdirSync(src, mustNotMutateObjectDeep({ recursive: true }));
   writeFileSync(join(src, 'foo.js'), 'foo', 'utf8');
-  symlinkSync('foo.js', join(src, 'bar.js'));
+  symlinkSync(join(src, 'foo.js'), join(src, 'bar.js'));
 
   const dest = nextdir();
   mkdirSync(dest, mustNotMutateObjectDeep({ recursive: true }));
@@ -216,9 +216,9 @@
   symlinkSync(dest, join(src, 'link'));
   cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true }));
   assert.throws(
     () => cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })),
     {
-      code: 'ERR_FS_CP_EINVAL'
+      code: 'ERR_FS_CP_EEXIST'
     }
   );
 }
@@ -234,7 +234,7 @@
   symlinkSync(src, join(dest, 'a', 'c'));
   assert.throws(
     () => cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })),
-    { code: 'ERR_FS_CP_SYMLINK_TO_SUBDIRECTORY' }
+    { code: 'ERR_FS_CP_EEXIST' }
   );
 }
 
@@ -398,7 +398,7 @@
   writeFileSync(join(dest, 'a', 'c'), 'hello', 'utf8');
   assert.throws(
     () => cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })),
-    { code: 'EEXIST' }
+    { code: 'ERR_FS_CP_EEXIST' }
   );
 }
 
@@ -416,19 +416,6 @@
   assert.strictEqual(srcStat.mtime.getTime(), destStat.mtime.getTime());
 }
 
-// It copies link if it does not point to folder in src.
-{
-  const src = nextdir();
-  mkdirSync(join(src, 'a', 'b'), mustNotMutateObjectDeep({ recursive: true }));
-  symlinkSync(src, join(src, 'a', 'c'));
-  const dest = nextdir();
-  mkdirSync(join(dest, 'a'), mustNotMutateObjectDeep({ recursive: true }));
-  symlinkSync(dest, join(dest, 'a', 'c'));
-  cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true }));
-  const link = readlinkSync(join(dest, 'a', 'c'));
-  assert.strictEqual(link, src);
-}
-
 // It accepts file URL as src and dest.
 {
   const src = './test/fixtures/copy/kitchen-sink';

typings/internalBinding/fs.d.ts

@@ -73,9 +73,18 @@ declare namespace InternalFSBinding {
   function close(fd: number, req: undefined, ctx: FSSyncContext): void;
 
   function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, req: FSReqCallback): void;
-  function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, req: undefined, ctx: FSSyncContext): void;
+  function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number): void;
   function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, usePromises: typeof kUsePromises): Promise<void>;
 
+  function cpSync(
+    src: StringOrBuffer,
+    dest: StringOrBuffer,
+    preserveTimestamps: boolean,
+    errorOnExist?: boolean,
+    force?: boolean,
+    recursive?: boolean,
+  ): void;
+
   function fchmod(fd: number, mode: number, req: FSReqCallback): void;
   function fchmod(fd: number, mode: number): void;
   function fchmod(fd: number, mode: number, usePromises: typeof kUsePromises): Promise<void>;
@@ -253,6 +262,7 @@ export interface FsBinding {
   chown: typeof InternalFSBinding.chown;
   close: typeof InternalFSBinding.close;
   copyFile: typeof InternalFSBinding.copyFile;
+  cpSync: typeof InternalFSBinding.cpSync;
   fchmod: typeof InternalFSBinding.fchmod;
   fchown: typeof InternalFSBinding.fchown;
   fdatasync: typeof InternalFSBinding.fdatasync;