Skip to content

Commit 24865bc

Browse files
dario-piotrowiczaduh95
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 <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
1 parent 0c9efcc commit 24865bc

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
@@ -805,6 +805,13 @@ inline void Environment::ThrowError(
805805
isolate()->ThrowException(fun(OneByteString(isolate(), errmsg), {}));
806806
}
807807

808+
inline void Environment::ThrowStdErrException(std::error_code error_code,
809+
const char* syscall,
810+
const char* path) {
811+
ThrowErrnoException(
812+
error_code.value(), syscall, error_code.message().c_str(), path);
813+
}
814+
808815
inline void Environment::ThrowErrnoException(int errorno,
809816
const char* syscall,
810817
const char* message,

src/env.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -850,6 +850,9 @@ class Environment final : public MemoryRetainer {
850850
inline void ThrowError(const char* errmsg);
851851
inline void ThrowTypeError(const char* errmsg);
852852
inline void ThrowRangeError(const char* errmsg);
853+
inline void ThrowStdErrException(std::error_code error_code,
854+
const char* syscall,
855+
const char* path = nullptr);
853856
inline void ThrowErrnoException(int errorno,
854857
const char* syscall = nullptr,
855858
const char* message = nullptr,

src/node_file.cc

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

45+
#include <cstdio>
4546
#include <filesystem>
4647

4748
#if defined(__MINGW32__) || defined(_MSC_VER)
@@ -3236,6 +3237,72 @@ static void CpSyncCheckPaths(const FunctionCallbackInfo<Value>& args) {
32363237
}
32373238
}
32383239

3240+
static void CpSyncOverrideFile(const FunctionCallbackInfo<Value>& args) {
3241+
Environment* env = Environment::GetCurrent(args);
3242+
Isolate* isolate = env->isolate();
3243+
3244+
CHECK_EQ(args.Length(), 4); // src, dest, mode, preserveTimestamps
3245+
3246+
BufferValue src(isolate, args[0]);
3247+
CHECK_NOT_NULL(*src);
3248+
ToNamespacedPath(env, &src);
3249+
3250+
BufferValue dest(isolate, args[1]);
3251+
CHECK_NOT_NULL(*dest);
3252+
ToNamespacedPath(env, &dest);
3253+
3254+
int mode;
3255+
if (!GetValidFileMode(env, args[2], UV_FS_COPYFILE).To(&mode)) {
3256+
return;
3257+
}
3258+
3259+
bool preserve_timestamps = args[3]->IsTrue();
3260+
3261+
THROW_IF_INSUFFICIENT_PERMISSIONS(
3262+
env, permission::PermissionScope::kFileSystemRead, src.ToStringView());
3263+
THROW_IF_INSUFFICIENT_PERMISSIONS(
3264+
env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView());
3265+
3266+
std::error_code error;
3267+
3268+
if (!std::filesystem::remove(*dest, error)) {
3269+
return env->ThrowStdErrException(error, "unlink", *dest);
3270+
}
3271+
3272+
if (mode == 0) {
3273+
// if no mode is specified use the faster std::filesystem API
3274+
if (!std::filesystem::copy_file(*src, *dest, error)) {
3275+
return env->ThrowStdErrException(error, "cp", *dest);
3276+
}
3277+
} else {
3278+
uv_fs_t req;
3279+
auto cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
3280+
auto result = uv_fs_copyfile(nullptr, &req, *src, *dest, mode, nullptr);
3281+
if (is_uv_error(result)) {
3282+
return env->ThrowUVException(result, "cp", nullptr, *src, *dest);
3283+
}
3284+
}
3285+
3286+
if (preserve_timestamps) {
3287+
uv_fs_t req;
3288+
auto cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
3289+
int result = uv_fs_stat(nullptr, &req, *src, nullptr);
3290+
if (is_uv_error(result)) {
3291+
return env->ThrowUVException(result, "stat", nullptr, *src);
3292+
}
3293+
3294+
const uv_stat_t* const s = static_cast<const uv_stat_t*>(req.ptr);
3295+
const double source_atime = s->st_atim.tv_sec + s->st_atim.tv_nsec / 1e9;
3296+
const double source_mtime = s->st_mtim.tv_sec + s->st_mtim.tv_nsec / 1e9;
3297+
3298+
int utime_result =
3299+
uv_fs_utime(nullptr, &req, *dest, source_atime, source_mtime, nullptr);
3300+
if (is_uv_error(utime_result)) {
3301+
return env->ThrowUVException(utime_result, "utime", nullptr, *dest);
3302+
}
3303+
}
3304+
}
3305+
32393306
BindingData::FilePathIsFileReturnType BindingData::FilePathIsFile(
32403307
Environment* env, const std::string& file_path) {
32413308
THROW_IF_INSUFFICIENT_PERMISSIONS(
@@ -3574,6 +3641,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
35743641
SetMethod(isolate, target, "mkdtemp", Mkdtemp);
35753642

35763643
SetMethod(isolate, target, "cpSyncCheckPaths", CpSyncCheckPaths);
3644+
SetMethod(isolate, target, "cpSyncOverrideFile", CpSyncOverrideFile);
35773645

35783646
StatWatcher::CreatePerIsolateProperties(isolate_data, target);
35793647
BindingData::CreatePerIsolateProperties(isolate_data, target);
@@ -3685,6 +3753,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
36853753
registry->Register(CopyFile);
36863754

36873755
registry->Register(CpSyncCheckPaths);
3756+
registry->Register(CpSyncOverrideFile);
36883757

36893758
registry->Register(Chmod);
36903759
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;
@@ -258,6 +259,7 @@ export interface FsBinding {
258259
close: typeof InternalFSBinding.close;
259260
copyFile: typeof InternalFSBinding.copyFile;
260261
cpSyncCheckPaths: typeof InternalFSBinding.cpSyncCheckPaths;
262+
cpSyncOverrideFile: typeof InternalFSBinding.cpSyncOverrideFile;
261263
fchmod: typeof InternalFSBinding.fchmod;
262264
fchown: typeof InternalFSBinding.fchown;
263265
fdatasync: typeof InternalFSBinding.fdatasync;

0 commit comments

Comments
 (0)