Skip to content

Commit 8870bb8

Browse files
dario-piotrowicztargos
authored andcommitted
fs: improve cpSync dest overriding performance
move the logic in `cpSync` that overrides existing dest files from JavaScript to C++ increasing its performance PR-URL: #58160 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent ce081bc commit 8870bb8

File tree

5 files changed

+85
-7
lines changed

5 files changed

+85
-7
lines changed

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,12 @@ function getStats(src, dest, opts) {
8181

8282
function onFile(srcStat, destStat, src, dest, opts) {
8383
if (!destStat) return copyFile(srcStat, src, dest, opts);
84-
return mayCopyFile(srcStat, src, dest, opts);
85-
}
8684

87-
// TODO(@anonrig): Move this function to C++.
88-
function mayCopyFile(srcStat, src, dest, opts) {
8985
if (opts.force) {
90-
unlinkSync(dest);
91-
return copyFile(srcStat, src, dest, opts);
92-
} else if (opts.errorOnExist) {
86+
return fsBinding.cpSyncOverrideFile(src, dest, opts.mode, opts.preserveTimestamps);
87+
}
88+
89+
if (opts.errorOnExist) {
9390
throw new ERR_FS_CP_EEXIST({
9491
message: `${dest} already exists`,
9592
path: dest,

src/env-inl.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,13 @@ inline void Environment::ThrowError(
775775
isolate()->ThrowException(fun(OneByteString(isolate(), errmsg), {}));
776776
}
777777

778+
inline void Environment::ThrowStdErrException(std::error_code error_code,
779+
const char* syscall,
780+
const char* path) {
781+
ThrowErrnoException(
782+
error_code.value(), syscall, error_code.message().c_str(), path);
783+
}
784+
778785
inline void Environment::ThrowErrnoException(int errorno,
779786
const char* syscall,
780787
const char* message,

src/env.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -830,6 +830,9 @@ class Environment final : public MemoryRetainer {
830830
inline void ThrowError(const char* errmsg);
831831
inline void ThrowTypeError(const char* errmsg);
832832
inline void ThrowRangeError(const char* errmsg);
833+
inline void ThrowStdErrException(std::error_code error_code,
834+
const char* syscall,
835+
const char* path = nullptr);
833836
inline void ThrowErrnoException(int errorno,
834837
const char* syscall = nullptr,
835838
const char* message = nullptr,

src/node_file.cc

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
#include "uv.h"
4444
#include "v8-fast-api-calls.h"
4545

46+
#include <cstdio>
4647
#include <filesystem>
4748

4849
#if defined(__MINGW32__) || defined(_MSC_VER)
@@ -3350,6 +3351,72 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
33503351
}
33513352
}
33523353

3354+
static void CpSyncOverrideFile(const FunctionCallbackInfo<Value>& args) {
3355+
Environment* env = Environment::GetCurrent(args);
3356+
Isolate* isolate = env->isolate();
3357+
3358+
CHECK_EQ(args.Length(), 4); // src, dest, mode, preserveTimestamps
3359+
3360+
BufferValue src(isolate, args[0]);
3361+
CHECK_NOT_NULL(*src);
3362+
ToNamespacedPath(env, &src);
3363+
3364+
BufferValue dest(isolate, args[1]);
3365+
CHECK_NOT_NULL(*dest);
3366+
ToNamespacedPath(env, &dest);
3367+
3368+
int mode;
3369+
if (!GetValidFileMode(env, args[2], UV_FS_COPYFILE).To(&mode)) {
3370+
return;
3371+
}
3372+
3373+
bool preserve_timestamps = args[3]->IsTrue();
3374+
3375+
THROW_IF_INSUFFICIENT_PERMISSIONS(
3376+
env, permission::PermissionScope::kFileSystemRead, src.ToStringView());
3377+
THROW_IF_INSUFFICIENT_PERMISSIONS(
3378+
env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView());
3379+
3380+
std::error_code error;
3381+
3382+
if (!std::filesystem::remove(*dest, error)) {
3383+
return env->ThrowStdErrException(error, "unlink", *dest);
3384+
}
3385+
3386+
if (mode == 0) {
3387+
// if no mode is specified use the faster std::filesystem API
3388+
if (!std::filesystem::copy_file(*src, *dest, error)) {
3389+
return env->ThrowStdErrException(error, "cp", *dest);
3390+
}
3391+
} else {
3392+
uv_fs_t req;
3393+
auto cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
3394+
auto result = uv_fs_copyfile(nullptr, &req, *src, *dest, mode, nullptr);
3395+
if (is_uv_error(result)) {
3396+
return env->ThrowUVException(result, "cp", nullptr, *src, *dest);
3397+
}
3398+
}
3399+
3400+
if (preserve_timestamps) {
3401+
uv_fs_t req;
3402+
auto cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
3403+
int result = uv_fs_stat(nullptr, &req, *src, nullptr);
3404+
if (is_uv_error(result)) {
3405+
return env->ThrowUVException(result, "stat", nullptr, *src);
3406+
}
3407+
3408+
const uv_stat_t* const s = static_cast<const uv_stat_t*>(req.ptr);
3409+
const double source_atime = s->st_atim.tv_sec + s->st_atim.tv_nsec / 1e9;
3410+
const double source_mtime = s->st_mtim.tv_sec + s->st_mtim.tv_nsec / 1e9;
3411+
3412+
int utime_result =
3413+
uv_fs_utime(nullptr, &req, *dest, source_atime, source_mtime, nullptr);
3414+
if (is_uv_error(utime_result)) {
3415+
return env->ThrowUVException(utime_result, "utime", nullptr, *dest);
3416+
}
3417+
}
3418+
}
3419+
33533420
BindingData::FilePathIsFileReturnType BindingData::FilePathIsFile(
33543421
Environment* env, const std::string& file_path) {
33553422
THROW_IF_INSUFFICIENT_PERMISSIONS(
@@ -3689,6 +3756,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
36893756
SetMethod(isolate, target, "mkdtemp", Mkdtemp);
36903757

36913758
SetMethod(isolate, target, "cpSyncCheckPaths", CpSyncCheckPaths);
3759+
SetMethod(isolate, target, "cpSyncOverrideFile", CpSyncOverrideFile);
36923760

36933761
StatWatcher::CreatePerIsolateProperties(isolate_data, target);
36943762
BindingData::CreatePerIsolateProperties(isolate_data, target);
@@ -3801,6 +3869,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
38013869
registry->Register(CopyFile);
38023870

38033871
registry->Register(CpSyncCheckPaths);
3872+
registry->Register(CpSyncOverrideFile);
38043873

38053874
registry->Register(Chmod);
38063875
registry->Register(FChmod);

typings/internalBinding/fs.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ declare namespace InternalFSBinding {
7777
function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, usePromises: typeof kUsePromises): Promise<void>;
7878

7979
function cpSyncCheckPaths(src: StringOrBuffer, dest: StringOrBuffer, dereference: boolean, recursive: boolean): void;
80+
function cpSyncOverrideFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, preserveTimestamps: boolean): void;
8081

8182
function fchmod(fd: number, mode: number, req: FSReqCallback): void;
8283
function fchmod(fd: number, mode: number): void;
@@ -260,6 +261,7 @@ export interface FsBinding {
260261
close: typeof InternalFSBinding.close;
261262
copyFile: typeof InternalFSBinding.copyFile;
262263
cpSyncCheckPaths: typeof InternalFSBinding.cpSyncCheckPaths;
264+
cpSyncOverrideFile: typeof InternalFSBinding.cpSyncOverrideFile;
263265
fchmod: typeof InternalFSBinding.fchmod;
264266
fchown: typeof InternalFSBinding.fchown;
265267
fdatasync: typeof InternalFSBinding.fdatasync;

0 commit comments

Comments
 (0)