Skip to content

Commit 3642b0d

Browse files
dario-piotrowiczaduh95
authored andcommitted
fs: improve cpSync no-filter copyDir performance
move the logic in `cpSync` that copies a directory from src to dest from JavaScript to C++ increasing its performance Note: this improvement is not applied if the filter option is provided, such optimization will be looked into separately PR-URL: #58461 Reviewed-By: Yagiz Nizipli <[email protected]>
1 parent 375a641 commit 3642b0d

File tree

5 files changed

+249
-8
lines changed

5 files changed

+249
-8
lines changed

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

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,18 +133,27 @@ function setDestTimestamps(src, dest) {
133133

134134
// TODO(@anonrig): Move this function to C++.
135135
function onDir(srcStat, destStat, src, dest, opts) {
136-
if (!destStat) return mkDirAndCopy(srcStat.mode, src, dest, opts);
136+
if (!destStat) return copyDir(src, dest, opts, true, srcStat.mode);
137137
return copyDir(src, dest, opts);
138138
}
139139

140-
function mkDirAndCopy(srcMode, src, dest, opts) {
141-
mkdirSync(dest);
142-
copyDir(src, dest, opts);
143-
return setDestMode(dest, srcMode);
144-
}
140+
function copyDir(src, dest, opts, mkDir, srcMode) {
141+
if (!opts.filter) {
142+
// The caller didn't provide a js filter function, in this case
143+
// we can run the whole function faster in C++
144+
// TODO(dario-piotrowicz): look into making cpSyncCopyDir also accept the potential filter function
145+
return fsBinding.cpSyncCopyDir(src, dest,
146+
opts.force,
147+
opts.dereference,
148+
opts.errorOnExist,
149+
opts.verbatimSymlinks,
150+
opts.preserveTimestamps);
151+
}
152+
153+
if (mkDir) {
154+
mkdirSync(dest);
155+
}
145156

146-
// TODO(@anonrig): Move this function to C++.
147-
function copyDir(src, dest, opts) {
148157
const dir = opendirSync(src);
149158

150159
try {
@@ -169,6 +178,10 @@ function copyDir(src, dest, opts) {
169178
}
170179
} finally {
171180
dir.closeSync();
181+
182+
if (srcMode !== undefined) {
183+
setDestMode(dest, srcMode);
184+
}
172185
}
173186
}
174187

src/node_errors.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,9 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
7878
V(ERR_FS_CP_EINVAL, Error) \
7979
V(ERR_FS_CP_DIR_TO_NON_DIR, Error) \
8080
V(ERR_FS_CP_NON_DIR_TO_DIR, Error) \
81+
V(ERR_FS_CP_SYMLINK_TO_SUBDIRECTORY, Error) \
8182
V(ERR_FS_EISDIR, Error) \
83+
V(ERR_FS_CP_EEXIST, Error) \
8284
V(ERR_FS_CP_SOCKET, Error) \
8385
V(ERR_FS_CP_FIFO_PIPE, Error) \
8486
V(ERR_FS_CP_UNKNOWN, Error) \

src/node_file.cc

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

45+
#include <errno.h>
46+
#include <cerrno>
4547
#include <cstdio>
4648
#include <filesystem>
4749

@@ -3303,6 +3305,223 @@ static void CpSyncOverrideFile(const FunctionCallbackInfo<Value>& args) {
33033305
}
33043306
}
33053307

3308+
std::vector<std::string> normalizePathToArray(
3309+
const std::filesystem::path& path) {
3310+
std::vector<std::string> parts;
3311+
std::filesystem::path absPath = std::filesystem::absolute(path);
3312+
for (const auto& part : absPath) {
3313+
if (!part.empty()) parts.push_back(part.string());
3314+
}
3315+
return parts;
3316+
}
3317+
3318+
bool isInsideDir(const std::filesystem::path& src,
3319+
const std::filesystem::path& dest) {
3320+
auto srcArr = normalizePathToArray(src);
3321+
auto destArr = normalizePathToArray(dest);
3322+
if (srcArr.size() > destArr.size()) return false;
3323+
return std::equal(srcArr.begin(), srcArr.end(), destArr.begin());
3324+
}
3325+
3326+
static void CpSyncCopyDir(const FunctionCallbackInfo<Value>& args) {
3327+
CHECK_EQ(args.Length(), 7); // src, dest, force, dereference, errorOnExist,
3328+
// verbatimSymlinks, preserveTimestamps
3329+
3330+
Environment* env = Environment::GetCurrent(args);
3331+
Isolate* isolate = env->isolate();
3332+
3333+
BufferValue src(isolate, args[0]);
3334+
CHECK_NOT_NULL(*src);
3335+
ToNamespacedPath(env, &src);
3336+
3337+
BufferValue dest(isolate, args[1]);
3338+
CHECK_NOT_NULL(*dest);
3339+
ToNamespacedPath(env, &dest);
3340+
3341+
bool force = args[2]->IsTrue();
3342+
bool dereference = args[3]->IsTrue();
3343+
bool error_on_exist = args[4]->IsTrue();
3344+
bool verbatim_symlinks = args[5]->IsTrue();
3345+
bool preserve_timestamps = args[6]->IsTrue();
3346+
3347+
std::error_code error;
3348+
std::filesystem::create_directories(*dest, error);
3349+
if (error) {
3350+
return env->ThrowStdErrException(error, "cp", *dest);
3351+
}
3352+
3353+
auto file_copy_opts = std::filesystem::copy_options::recursive;
3354+
if (force) {
3355+
file_copy_opts |= std::filesystem::copy_options::overwrite_existing;
3356+
} else if (error_on_exist) {
3357+
file_copy_opts |= std::filesystem::copy_options::none;
3358+
} else {
3359+
file_copy_opts |= std::filesystem::copy_options::skip_existing;
3360+
}
3361+
3362+
std::function<bool(std::filesystem::path, std::filesystem::path)>
3363+
copy_dir_contents;
3364+
copy_dir_contents = [verbatim_symlinks,
3365+
&copy_dir_contents,
3366+
&env,
3367+
file_copy_opts,
3368+
preserve_timestamps,
3369+
force,
3370+
error_on_exist,
3371+
dereference,
3372+
&isolate](std::filesystem::path src,
3373+
std::filesystem::path dest) {
3374+
std::error_code error;
3375+
for (auto dir_entry : std::filesystem::directory_iterator(src)) {
3376+
auto dest_file_path = dest / dir_entry.path().filename();
3377+
auto dest_str = PathToString(dest);
3378+
3379+
if (dir_entry.is_symlink()) {
3380+
if (verbatim_symlinks) {
3381+
std::filesystem::copy_symlink(
3382+
dir_entry.path(), dest_file_path, error);
3383+
if (error) {
3384+
env->ThrowStdErrException(error, "cp", dest_str.c_str());
3385+
return false;
3386+
}
3387+
} else {
3388+
auto symlink_target =
3389+
std::filesystem::read_symlink(dir_entry.path().c_str(), error);
3390+
if (error) {
3391+
env->ThrowStdErrException(error, "cp", dest_str.c_str());
3392+
return false;
3393+
}
3394+
3395+
if (std::filesystem::exists(dest_file_path)) {
3396+
if (std::filesystem::is_symlink((dest_file_path.c_str()))) {
3397+
auto current_dest_symlink_target =
3398+
std::filesystem::read_symlink(dest_file_path.c_str(), error);
3399+
if (error) {
3400+
env->ThrowStdErrException(error, "cp", dest_str.c_str());
3401+
return false;
3402+
}
3403+
3404+
if (!dereference &&
3405+
std::filesystem::is_directory(symlink_target) &&
3406+
isInsideDir(symlink_target, current_dest_symlink_target)) {
3407+
std::string message =
3408+
"Cannot copy %s to a subdirectory of self %s";
3409+
THROW_ERR_FS_CP_EINVAL(env,
3410+
message.c_str(),
3411+
symlink_target.c_str(),
3412+
current_dest_symlink_target.c_str());
3413+
return false;
3414+
}
3415+
3416+
// Prevent copy if src is a subdir of dest since unlinking
3417+
// dest in this case would result in removing src contents
3418+
// and therefore a broken symlink would be created.
3419+
if (std::filesystem::is_directory(dest_file_path) &&
3420+
isInsideDir(current_dest_symlink_target, symlink_target)) {
3421+
std::string message = "cannot overwrite %s with %s";
3422+
THROW_ERR_FS_CP_SYMLINK_TO_SUBDIRECTORY(
3423+
env,
3424+
message.c_str(),
3425+
current_dest_symlink_target.c_str(),
3426+
symlink_target.c_str());
3427+
return false;
3428+
}
3429+
3430+
// symlinks get overridden by cp even if force: false, this is
3431+
// being applied here for backward compatibility, but is it
3432+
// correct? or is it a bug?
3433+
std::filesystem::remove(dest_file_path, error);
3434+
if (error) {
3435+
env->ThrowStdErrException(error, "cp", dest_str.c_str());
3436+
return false;
3437+
}
3438+
} else if (std::filesystem::is_regular_file(dest_file_path)) {
3439+
if (!dereference || (!force && error_on_exist)) {
3440+
auto dest_file_path_str = PathToString(dest_file_path);
3441+
env->ThrowStdErrException(
3442+
std::make_error_code(std::errc::file_exists),
3443+
"cp",
3444+
dest_file_path_str.c_str());
3445+
return false;
3446+
}
3447+
}
3448+
}
3449+
auto symlink_target_absolute = std::filesystem::weakly_canonical(
3450+
std::filesystem::absolute(src / symlink_target));
3451+
if (dir_entry.is_directory()) {
3452+
std::filesystem::create_directory_symlink(
3453+
symlink_target_absolute, dest_file_path, error);
3454+
} else {
3455+
std::filesystem::create_symlink(
3456+
symlink_target_absolute, dest_file_path, error);
3457+
}
3458+
if (error) {
3459+
env->ThrowStdErrException(error, "cp", dest_str.c_str());
3460+
return false;
3461+
}
3462+
}
3463+
} else if (dir_entry.is_directory()) {
3464+
auto entry_dir_path = src / dir_entry.path().filename();
3465+
std::filesystem::create_directory(dest_file_path);
3466+
auto success = copy_dir_contents(entry_dir_path, dest_file_path);
3467+
if (!success) {
3468+
return false;
3469+
}
3470+
} else if (dir_entry.is_regular_file()) {
3471+
std::filesystem::copy_file(
3472+
dir_entry.path(), dest_file_path, file_copy_opts, error);
3473+
if (error) {
3474+
if (error.value() == EEXIST) {
3475+
THROW_ERR_FS_CP_EEXIST(isolate,
3476+
"[ERR_FS_CP_EEXIST]: Target already exists: "
3477+
"cp returned EEXIST (%s already exists)",
3478+
dest_file_path.c_str());
3479+
return false;
3480+
}
3481+
env->ThrowStdErrException(error, "cp", dest_str.c_str());
3482+
return false;
3483+
}
3484+
3485+
if (preserve_timestamps) {
3486+
uv_fs_t req;
3487+
auto cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
3488+
3489+
auto dir_entry_path_str = PathToString(dir_entry.path());
3490+
int result =
3491+
uv_fs_stat(nullptr, &req, dir_entry_path_str.c_str(), nullptr);
3492+
if (is_uv_error(result)) {
3493+
env->ThrowUVException(
3494+
result, "stat", nullptr, dir_entry_path_str.c_str());
3495+
return false;
3496+
}
3497+
3498+
const uv_stat_t* const s = static_cast<const uv_stat_t*>(req.ptr);
3499+
const double source_atime =
3500+
s->st_atim.tv_sec + s->st_atim.tv_nsec / 1e9;
3501+
const double source_mtime =
3502+
s->st_mtim.tv_sec + s->st_mtim.tv_nsec / 1e9;
3503+
3504+
auto dest_file_path_str = PathToString(dest_file_path);
3505+
int utime_result = uv_fs_utime(nullptr,
3506+
&req,
3507+
dest_file_path_str.c_str(),
3508+
source_atime,
3509+
source_mtime,
3510+
nullptr);
3511+
if (is_uv_error(utime_result)) {
3512+
env->ThrowUVException(
3513+
utime_result, "utime", nullptr, dest_file_path_str.c_str());
3514+
return false;
3515+
}
3516+
}
3517+
}
3518+
}
3519+
return true;
3520+
};
3521+
3522+
copy_dir_contents(std::filesystem::path(*src), std::filesystem::path(*dest));
3523+
}
3524+
33063525
BindingData::FilePathIsFileReturnType BindingData::FilePathIsFile(
33073526
Environment* env, const std::string& file_path) {
33083527
THROW_IF_INSUFFICIENT_PERMISSIONS(
@@ -3642,6 +3861,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
36423861

36433862
SetMethod(isolate, target, "cpSyncCheckPaths", CpSyncCheckPaths);
36443863
SetMethod(isolate, target, "cpSyncOverrideFile", CpSyncOverrideFile);
3864+
SetMethod(isolate, target, "cpSyncCopyDir", CpSyncCopyDir);
36453865

36463866
StatWatcher::CreatePerIsolateProperties(isolate_data, target);
36473867
BindingData::CreatePerIsolateProperties(isolate_data, target);
@@ -3754,6 +3974,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
37543974

37553975
registry->Register(CpSyncCheckPaths);
37563976
registry->Register(CpSyncOverrideFile);
3977+
registry->Register(CpSyncCopyDir);
37573978

37583979
registry->Register(Chmod);
37593980
registry->Register(FChmod);

test/parallel/test-fs-cp.mjs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,9 @@ if (!isWindows && !isInsideDirWithUnusualChars) {
493493
const dest = nextdir();
494494
mkdirSync(dest, mustNotMutateObjectDeep({ recursive: true }));
495495
writeFileSync(join(src, 'foo.txt'), 'foo', mustNotMutateObjectDeep({ mode: 0o444 }));
496+
// Small wait to make sure that destStat.mtime.getTime() would produce a time
497+
// different from srcStat.mtime.getTime() if preserveTimestamps wasn't set to true
498+
await setTimeout(5);
496499
cpSync(src, dest, mustNotMutateObjectDeep({ preserveTimestamps: true, recursive: true }));
497500
assertDirEquivalent(src, dest);
498501
const srcStat = lstatSync(join(src, 'foo.txt'));

typings/internalBinding/fs.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ declare namespace InternalFSBinding {
7878

7979
function cpSyncCheckPaths(src: StringOrBuffer, dest: StringOrBuffer, dereference: boolean, recursive: boolean): void;
8080
function cpSyncOverrideFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, preserveTimestamps: boolean): void;
81+
function cpSyncCopyDir(src: StringOrBuffer, dest: StringOrBuffer, force: boolean, errorOnExist: boolean, verbatimSymlinks: boolean, dereference: boolean): void;
8182

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

0 commit comments

Comments
 (0)
close