fs: improve error performance for fs.renameSync

Author: anonrig

benchmark/fs/bench-renameSync.js

@@ -0,0 +1,53 @@
+'use strict';
+
+const common = require('../common');
+const fs = require('fs');
+const tmpdir = require('../../test/common/tmpdir');
+tmpdir.refresh();
+
+const bench = common.createBenchmark(main, {
+  type: ['invalid', 'valid'],
+  n: [1e3],
+});
+
+function main({ n, type }) {
+  tmpdir.refresh();
+
+  switch (type) {
+    case 'invalid': {
+      bench.start();
+      for (let i = 0; i < n; i++) {
+        try {
+          fs.renameSync(tmpdir.resolve(`.non-existing-file-${i}`), tmpdir.resolve(`.new-file-${i}`));
+        } catch {
+          // do nothing
+        }
+      }
+      bench.end(n);
+
+      break;
+    }
+    case 'valid': {
+      for (let i = 0; i < n; i++) {
+        fs.writeFileSync(tmpdir.resolve(`.existing-file-${i}`), 'bench', 'utf8');
+      }
+
+      bench.start();
+      for (let i = 0; i < n; i++) {
+        try {
+          fs.renameSync(
+            tmpdir.resolve(`.existing-file-${i}`),
+            tmpdir.resolve(`.new-existing-file-${i}`),
+          );
+        } catch {
+          // do nothing
+        }
+      }
+
+      bench.end(n);
+      break;
+    }
+    default:
+      throw new Error('Invalid type');
+  }
+}

lib/fs.js

@@ -1015,12 +1015,7 @@ function rename(oldPath, newPath, callback) {
  * @returns {void}
  */
 function renameSync(oldPath, newPath) {
-  oldPath = getValidatedPath(oldPath, 'oldPath');
-  newPath = getValidatedPath(newPath, 'newPath');
-  const ctx = { path: oldPath, dest: newPath };
-  binding.rename(pathModule.toNamespacedPath(oldPath),
-                 pathModule.toNamespacedPath(newPath), undefined, ctx);
-  handleErrorFromBinding(ctx);
+  return syncFs.rename(oldPath, newPath);
 }
 
 /**

lib/internal/fs/sync.js

@@ -93,6 +93,15 @@ function unlink(path) {
   return binding.unlinkSync(path);
 }
 
+function rename(oldPath, newPath) {
+  oldPath = getValidatedPath(oldPath, 'oldPath');
+  newPath = getValidatedPath(newPath, 'newPath');
+  return binding.renameSync(
+    pathModule.toNamespacedPath(oldPath),
+    pathModule.toNamespacedPath(newPath),
+  );
+}
+
 module.exports = {
   readFileUtf8,
   exists,
@@ -103,4 +112,5 @@ module.exports = {
   open,
   close,
   unlink,
+  rename,
 };

src/node_file.cc

@@ -1568,6 +1568,38 @@ static void Rename(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
+static void RenameSync(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  Isolate* isolate = env->isolate();
+
+  CHECK_EQ(args.Length(), 2);
+
+  BufferValue old_path(isolate, args[0]);
+  CHECK_NOT_NULL(*old_path);
+  auto view_old_path = old_path.ToStringView();
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFileSystemRead, view_old_path);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env, permission::PermissionScope::kFileSystemWrite, view_old_path);
+
+  BufferValue new_path(isolate, args[1]);
+  CHECK_NOT_NULL(*new_path);
+  THROW_IF_INSUFFICIENT_PERMISSIONS(
+      env,
+      permission::PermissionScope::kFileSystemWrite,
+      new_path.ToStringView());
+
+  uv_fs_t req;
+  auto cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
+  FS_SYNC_TRACE_BEGIN(rename);
+  int err = uv_fs_rename(nullptr, &req, *old_path, *new_path, nullptr);
+  FS_SYNC_TRACE_END(rename);
+
+  if (err < 0) {
+    return env->ThrowUVException(err, "rename", nullptr, *old_path, *new_path);
+  }
+}
+
 static void FTruncate(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
@@ -3395,6 +3427,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
   SetMethod(isolate, target, "fdatasync", Fdatasync);
   SetMethod(isolate, target, "fsync", Fsync);
   SetMethod(isolate, target, "rename", Rename);
+  SetMethod(isolate, target, "renameSync", RenameSync);
   SetMethod(isolate, target, "ftruncate", FTruncate);
   SetMethod(isolate, target, "rmdir", RMDir);
   SetMethod(isolate, target, "mkdir", MKDir);
@@ -3521,6 +3554,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(Fdatasync);
   registry->Register(Fsync);
   registry->Register(Rename);
+  registry->Register(RenameSync);
   registry->Register(FTruncate);
   registry->Register(RMDir);
   registry->Register(MKDir);

typings/internalBinding/fs.d.ts

@@ -180,6 +180,7 @@ declare namespace InternalFSBinding {
   function rename(oldPath: string, newPath: string, req: FSReqCallback): void;
   function rename(oldPath: string, newPath: string, req: undefined, ctx: FSSyncContext): void;
   function rename(oldPath: string, newPath: string, usePromises: typeof kUsePromises): Promise<void>;
+  function renameSync(oldPath: string, newPath: string): void;
 
   function rmdir(path: string, req: FSReqCallback): void;
   function rmdir(path: string, req: undefined, ctx: FSSyncContext): void;
@@ -261,6 +262,7 @@ export interface FsBinding {
   readlink: typeof InternalFSBinding.readlink;
   realpath: typeof InternalFSBinding.realpath;
   rename: typeof InternalFSBinding.rename;
+  renameSync: typeof InternalFSBinding.renameSync;
   rmdir: typeof InternalFSBinding.rmdir;
   stat: typeof InternalFSBinding.stat;
   symlink: typeof InternalFSBinding.symlink;