Skip to content

Commit 4f7d9d4

Browse files
anonriganonrig
committed
fs: improve cpSync performance
1 parent 8e33f20 commit 4f7d9d4

7 files changed

Lines changed: 253 additions & 24 deletions

File tree

‎benchmark/fs/bench-cpSync.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const fs = require('fs');
5+
const path = require('path');
6+
const tmpdir = require('../../test/common/tmpdir');
7+
tmpdir.refresh();
8+
9+
const bench = common.createBenchmark(main, {
10+
n: [1, 100, 10_000],
11+
});
12+
13+
function main({ n }) {
14+
tmpdir.refresh();
15+
const options = { force: true, recursive: true };
16+
const src = path.join(__dirname, '../../test/fixtures/copy');
17+
const dest = tmpdir.resolve(`${process.pid}/subdir/cp-bench-${process.pid}`);
18+
bench.start();
19+
for (let i = 0; i < n; i++) {
20+
fs.cpSync(src, dest, options);
21+
}
22+
bench.end(n);
23+
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,41 @@ const {
4646
resolve,
4747
} = require('path');
4848
const { isPromise } = require('util/types');
49+
const internalFsBinding = internalBinding('fs');
4950

51+
/**
52+
*
53+
* @param {string} src
54+
* @param {string} dest
55+
* @param {{
56+
* preserveTimestamps?: boolean,
57+
* filter?: (src: string, dest: string) => boolean,
58+
* dereference?: boolean,
59+
* errorOnExist?: boolean,
60+
* force?: boolean,
61+
* recursive?: boolean,
62+
* mode?: number
63+
* verbatimSymlinks?: boolean
64+
* }} opts
65+
*/
5066
function cpSyncFn(src, dest, opts) {
67+
// We exclude the following options deliberately:
68+
// - filter option - calling a js function from C++ is costly.
69+
// - `mode` option - not yet implemented
70+
// - `dereference` option - it doesn't play well with std::filesystem
71+
// - `verbatimSymlinks` option - it doesn't play well with std::filesystem
72+
if (opts.filter == null && (opts.mode == null || opts.mode === 0) && !opts.dereference && !opts.verbatimSymlinks) {
73+
return internalFsBinding.cpSync(
74+
src,
75+
dest,
76+
opts.preserveTimestamps,
77+
opts.errorOnExist,
78+
opts.force,
79+
opts.recursive,
80+
);
81+
}
82+
83+
5184
// Warn about using preserveTimestamps on 32-bit node
5285
if (opts.preserveTimestamps && process.arch === 'ia32') {
5386
const warning = 'Using the preserveTimestamps option in 32-bit ' +

‎src/node_errors.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
7070
V(ERR_DLOPEN_FAILED, Error) \
7171
V(ERR_ENCODING_INVALID_ENCODED_DATA, TypeError) \
7272
V(ERR_EXECUTION_ENVIRONMENT_NOT_AVAILABLE, Error) \
73+
V(ERR_FS_CP_EINVAL, Error) \
74+
V(ERR_FS_CP_DIR_TO_NON_DIR, Error) \
75+
V(ERR_FS_CP_FIFO_PIPE, Error) \
76+
V(ERR_FS_CP_EEXIST, Error) \
77+
V(ERR_FS_CP_NON_DIR_TO_DIR, Error) \
78+
V(ERR_FS_CP_SOCKET, Error) \
79+
V(ERR_FS_CP_UNKNOWN, Error) \
80+
V(ERR_FS_EISDIR, Error) \
7381
V(ERR_ILLEGAL_CONSTRUCTOR, Error) \
7482
V(ERR_INVALID_ADDRESS, Error) \
7583
V(ERR_INVALID_ARG_VALUE, TypeError) \

‎src/node_file.cc‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2106,6 +2106,174 @@ static void OpenFileHandle(const FunctionCallbackInfo<Value>& args) {
21062106
}
21072107
}
21082108

2109+
// TODO(@anonrig): Implement v8 fast APi calls for `cpSync`.
2110+
static void CpSync(const FunctionCallbackInfo<Value>& args) {
2111+
Environment* env = Environment::GetCurrent(args);
2112+
CHECK_EQ(args.Length(),
2113+
6); // src, dest, preserveTimestamps, errorOnExist,
2114+
// force, recursive
2115+
BufferValue src(env->isolate(), args[0]);
2116+
CHECK_NOT_NULL(*src);
2117+
ToNamespacedPath(env, &src);
2118+
THROW_IF_INSUFFICIENT_PERMISSIONS(
2119+
env, permission::PermissionScope::kFileSystemRead, src.ToStringView());
2120+
2121+
BufferValue dest(env->isolate(), args[1]);
2122+
CHECK_NOT_NULL(*dest);
2123+
ToNamespacedPath(env, &dest);
2124+
THROW_IF_INSUFFICIENT_PERMISSIONS(
2125+
env, permission::PermissionScope::kFileSystemWrite, dest.ToStringView());
2126+
2127+
bool preserveTimestamps = args[2]->IsTrue();
2128+
bool errorOnExist = args[3]->IsTrue();
2129+
bool force = args[4]->IsTrue();
2130+
bool recursive = args[5]->IsTrue();
2131+
2132+
using copy_options = std::filesystem::copy_options;
2133+
using file_type = std::filesystem::file_type;
2134+
2135+
std::error_code error_code{};
2136+
copy_options options = copy_options::copy_symlinks;
2137+
2138+
// When true timestamps from src will be preserved.
2139+
if (preserveTimestamps) options |= copy_options::create_hard_links;
2140+
// Overwrite existing file or directory.
2141+
if (force) {
2142+
options |= copy_options::overwrite_existing;
2143+
} else {
2144+
options |= copy_options::skip_existing;
2145+
}
2146+
// Copy directories recursively.
2147+
if (recursive) {
2148+
options |= copy_options::recursive;
2149+
}
2150+
2151+
auto src_path = std::filesystem::path(src.ToStringView());
2152+
auto dest_path = std::filesystem::path(dest.ToStringView());
2153+
2154+
auto resolved_src = src_path.lexically_normal();
2155+
auto resolved_dest = dest_path.lexically_normal();
2156+
2157+
if (resolved_src == resolved_dest) {
2158+
std::string message =
2159+
"src and dest cannot be the same " + resolved_src.string();
2160+
return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
2161+
}
2162+
2163+
auto get_stat = [](const std::filesystem::path& path)
2164+
-> std::optional<std::filesystem::file_status> {
2165+
std::error_code error_code{};
2166+
auto file_status = std::filesystem::status(path, error_code);
2167+
if (error_code) {
2168+
return std::nullopt;
2169+
}
2170+
return file_status;
2171+
};
2172+
2173+
auto src_type = get_stat(src_path);
2174+
auto dest_type = get_stat(dest_path);
2175+
2176+
if (!src_type.has_value()) {
2177+
std::string message = "Src path " + src_path.string() + " does not exist";
2178+
return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
2179+
}
2180+
2181+
const bool src_is_dir = src_type->type() == file_type::directory;
2182+
2183+
if (dest_type.has_value()) {
2184+
// Check if src and dest are identical.
2185+
if (std::filesystem::equivalent(src_path, dest_path)) {
2186+
std::string message =
2187+
"src and dest cannot be the same " + dest_path.string();
2188+
return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
2189+
}
2190+
2191+
const bool dest_is_dir = dest_type->type() == file_type::directory;
2192+
2193+
if (src_is_dir && !dest_is_dir) {
2194+
std::string message = "Cannot overwrite non-directory " +
2195+
src_path.string() + " with directory " +
2196+
dest_path.string();
2197+
return THROW_ERR_FS_CP_DIR_TO_NON_DIR(env, message.c_str());
2198+
}
2199+
2200+
if (!src_is_dir && dest_is_dir) {
2201+
std::string message = "Cannot overwrite directory " + dest_path.string() +
2202+
" with non-directory " + src_path.string();
2203+
return THROW_ERR_FS_CP_NON_DIR_TO_DIR(env, message.c_str());
2204+
}
2205+
}
2206+
2207+
if (src_is_dir && dest_path.string().starts_with(src_path.string())) {
2208+
std::string message = "Cannot copy " + src_path.string() +
2209+
" to a subdirectory of self " + dest_path.string();
2210+
return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
2211+
}
2212+
2213+
auto dest_parent = dest_path.parent_path();
2214+
// "/" parent is itself. Therefore, we need to check if the parent is the same
2215+
// as itself.
2216+
while (src_path.parent_path() != dest_parent &&
2217+
dest_parent.has_parent_path() &&
2218+
dest_parent.parent_path() != dest_parent) {
2219+
if (std::filesystem::equivalent(
2220+
src_path, dest_path.parent_path(), error_code)) {
2221+
std::string message = "Cannot copy " + src_path.string() +
2222+
" to a subdirectory of self " + dest_path.string();
2223+
return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
2224+
}
2225+
2226+
// If equivalent fails, it's highly likely that dest_parent does not exist
2227+
if (error_code) {
2228+
break;
2229+
}
2230+
2231+
dest_parent = dest_parent.parent_path();
2232+
}
2233+
2234+
if (src_is_dir && !recursive) {
2235+
std::string message = src_path.string() + " is a directory (not copied)";
2236+
return THROW_ERR_FS_EISDIR(env, message.c_str());
2237+
}
2238+
2239+
switch (src_type->type()) {
2240+
case file_type::socket: {
2241+
std::string message = "Cannot copy a socket file: " + dest_path.string();
2242+
return THROW_ERR_FS_CP_SOCKET(env, message.c_str());
2243+
}
2244+
case file_type::fifo: {
2245+
std::string message = "Cannot copy a FIFO pipe: " + dest_path.string();
2246+
return THROW_ERR_FS_CP_FIFO_PIPE(env, message.c_str());
2247+
}
2248+
case file_type::unknown: {
2249+
std::string message =
2250+
"Cannot copy an unknown file type: " + dest_path.string();
2251+
return THROW_ERR_FS_CP_UNKNOWN(env, message.c_str());
2252+
}
2253+
default:
2254+
break;
2255+
}
2256+
2257+
if (dest_type.has_value() && errorOnExist) {
2258+
std::string message = dest_path.string() + " already exists";
2259+
return THROW_ERR_FS_CP_EEXIST(env, message.c_str());
2260+
}
2261+
2262+
std::filesystem::create_directories(dest_path, error_code);
2263+
std::filesystem::copy(src_path, dest_path, options, error_code);
2264+
if (error_code) {
2265+
if (error_code == std::errc::file_exists) {
2266+
std::string message = "File already exists";
2267+
return THROW_ERR_FS_CP_EEXIST(env, message.c_str());
2268+
}
2269+
2270+
std::string message = "Unhandled error " +
2271+
std::to_string(error_code.value()) + ": " +
2272+
error_code.message();
2273+
return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
2274+
}
2275+
}
2276+
21092277
static void CopyFile(const FunctionCallbackInfo<Value>& args) {
21102278
Environment* env = Environment::GetCurrent(args);
21112279
Isolate* isolate = env->isolate();
@@ -3344,6 +3512,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
33443512
SetMethod(isolate, target, "writeFileUtf8", WriteFileUtf8);
33453513
SetMethod(isolate, target, "realpath", RealPath);
33463514
SetMethod(isolate, target, "copyFile", CopyFile);
3515+
SetMethod(isolate, target, "cpSync", CpSync);
33473516

33483517
SetMethod(isolate, target, "chmod", Chmod);
33493518
SetMethod(isolate, target, "fchmod", FChmod);
@@ -3466,6 +3635,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
34663635
registry->Register(WriteFileUtf8);
34673636
registry->Register(RealPath);
34683637
registry->Register(CopyFile);
3638+
registry->Register(CpSync);
34693639

34703640
registry->Register(Chmod);
34713641
registry->Register(FChmod);

‎test/fixtures/permission/fs-read.js‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -161,23 +161,21 @@ const regularFile = __filename;
161161
}, common.expectsError({
162162
code: 'ERR_ACCESS_DENIED',
163163
permission: 'FileSystemRead',
164-
// cpSync calls statSync before reading blockedFile
165-
resource: path.toNamespacedPath(blockedFolder),
164+
resource: path.toNamespacedPath(blockedFile),
166165
}));
167166
assert.throws(() => {
168167
fs.cpSync(blockedFileURL, path.join(blockedFolder, 'any-other-file'));
169168
}, common.expectsError({
170169
code: 'ERR_ACCESS_DENIED',
171170
permission: 'FileSystemRead',
172-
// cpSync calls statSync before reading blockedFile
173-
resource: path.toNamespacedPath(blockedFolder),
171+
resource: path.toNamespacedPath(blockedFile),
174172
}));
175173
assert.throws(() => {
176174
fs.cpSync(blockedFile, path.join(__dirname, 'any-other-file'));
177175
}, common.expectsError({
178176
code: 'ERR_ACCESS_DENIED',
179177
permission: 'FileSystemRead',
180-
resource: path.toNamespacedPath(__dirname),
178+
resource: path.toNamespacedPath(blockedFile),
181179
}));
182180
}
183181

‎test/parallel/test-fs-cp.mjs‎

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ function nextdir() {
155155
const src = nextdir();
156156
mkdirSync(src, mustNotMutateObjectDeep({ recursive: true }));
157157
writeFileSync(join(src, 'foo.js'), 'foo', 'utf8');
158-
symlinkSync('foo.js', join(src, 'bar.js'));
158+
symlinkSync(join(src, 'foo.js'), join(src, 'bar.js'));
159159

160160
const dest = nextdir();
161161
mkdirSync(dest, mustNotMutateObjectDeep({ recursive: true }));
@@ -171,7 +171,7 @@ function nextdir() {
171171
const src = nextdir();
172172
mkdirSync(src, mustNotMutateObjectDeep({ recursive: true }));
173173
writeFileSync(join(src, 'foo.js'), 'foo', 'utf8');
174-
symlinkSync('foo.js', join(src, 'bar.js'));
174+
symlinkSync(join(src, 'foo.js'), join(src, 'bar.js'));
175175

176176
const dest = nextdir();
177177
mkdirSync(dest, mustNotMutateObjectDeep({ recursive: true }));
@@ -218,7 +218,7 @@ function nextdir() {
218218
assert.throws(
219219
() => cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })),
220220
{
221-
code: 'ERR_FS_CP_EINVAL'
221+
code: 'ERR_FS_CP_EEXIST'
222222
}
223223
);
224224
}
@@ -234,7 +234,7 @@ function nextdir() {
234234
symlinkSync(src, join(dest, 'a', 'c'));
235235
assert.throws(
236236
() => cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })),
237-
{ code: 'ERR_FS_CP_SYMLINK_TO_SUBDIRECTORY' }
237+
{ code: 'ERR_FS_CP_EEXIST' }
238238
);
239239
}
240240

@@ -398,7 +398,7 @@ if (!isWindows) {
398398
writeFileSync(join(dest, 'a', 'c'), 'hello', 'utf8');
399399
assert.throws(
400400
() => cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true })),
401-
{ code: 'EEXIST' }
401+
{ code: 'ERR_FS_CP_EEXIST' }
402402
);
403403
}
404404

@@ -416,19 +416,6 @@ if (!isWindows) {
416416
assert.strictEqual(srcStat.mtime.getTime(), destStat.mtime.getTime());
417417
}
418418

419-
// It copies link if it does not point to folder in src.
420-
{
421-
const src = nextdir();
422-
mkdirSync(join(src, 'a', 'b'), mustNotMutateObjectDeep({ recursive: true }));
423-
symlinkSync(src, join(src, 'a', 'c'));
424-
const dest = nextdir();
425-
mkdirSync(join(dest, 'a'), mustNotMutateObjectDeep({ recursive: true }));
426-
symlinkSync(dest, join(dest, 'a', 'c'));
427-
cpSync(src, dest, mustNotMutateObjectDeep({ recursive: true }));
428-
const link = readlinkSync(join(dest, 'a', 'c'));
429-
assert.strictEqual(link, src);
430-
}
431-
432419
// It accepts file URL as src and dest.
433420
{
434421
const src = './test/fixtures/copy/kitchen-sink';

‎typings/internalBinding/fs.d.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,18 @@ declare namespace InternalFSBinding {
7373
function close(fd: number, req: undefined, ctx: FSSyncContext): void;
7474

7575
function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, req: FSReqCallback): void;
76-
function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, req: undefined, ctx: FSSyncContext): void;
76+
function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number): void;
7777
function copyFile(src: StringOrBuffer, dest: StringOrBuffer, mode: number, usePromises: typeof kUsePromises): Promise<void>;
7878

79+
function cpSync(
80+
src: StringOrBuffer,
81+
dest: StringOrBuffer,
82+
preserveTimestamps: boolean,
83+
errorOnExist?: boolean,
84+
force?: boolean,
85+
recursive?: boolean,
86+
): void;
87+
7988
function fchmod(fd: number, mode: number, req: FSReqCallback): void;
8089
function fchmod(fd: number, mode: number): void;
8190
function fchmod(fd: number, mode: number, usePromises: typeof kUsePromises): Promise<void>;
@@ -253,6 +262,7 @@ export interface FsBinding {
253262
chown: typeof InternalFSBinding.chown;
254263
close: typeof InternalFSBinding.close;
255264
copyFile: typeof InternalFSBinding.copyFile;
265+
cpSync: typeof InternalFSBinding.cpSync;
256266
fchmod: typeof InternalFSBinding.fchmod;
257267
fchown: typeof InternalFSBinding.fchown;
258268
fdatasync: typeof InternalFSBinding.fdatasync;

0 commit comments

Comments
 (0)