Skip to content

Commit ea737f7

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

File tree

7 files changed

+255
-24
lines changed

7 files changed

+255
-24
lines changed

‎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: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,39 @@ 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+
// Calling a JavaScript function from C++ is costly. Therefore, we don't support it.
68+
// TODO(@anonrig): Support `mode` option.
69+
if (opts.filter == null && (opts.mode == null || opts.mode === 0) && !opts.dereference) {
70+
return internalFsBinding.cpSync(
71+
src,
72+
dest,
73+
opts.preserveTimestamps,
74+
opts.errorOnExist,
75+
opts.force,
76+
opts.recursive,
77+
opts.verbatimSymlinks,
78+
);
79+
}
80+
81+
5182
// Warn about using preserveTimestamps on 32-bit node
5283
if (opts.preserveTimestamps && process.arch === 'ia32') {
5384
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: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2106,6 +2106,177 @@ 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+
7); // src, dest, preserveTimestamps, errorOnExist,
2114+
// force, recursive, verbatimSymlinks
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+
bool verbatimSymlinks = args[6]->IsTrue();
2132+
2133+
using copy_options = std::filesystem::copy_options;
2134+
using file_type = std::filesystem::file_type;
2135+
2136+
std::error_code error_code{};
2137+
copy_options options = copy_options::copy_symlinks;
2138+
2139+
// When true timestamps from src will be preserved.
2140+
if (preserveTimestamps) options |= copy_options::create_hard_links;
2141+
// Overwrite existing file or directory.
2142+
if (force) {
2143+
options |= copy_options::overwrite_existing;
2144+
} else {
2145+
options |= copy_options::skip_existing;
2146+
}
2147+
// Copy directories recursively.
2148+
if (recursive) {
2149+
options |= copy_options::recursive;
2150+
}
2151+
// When true, path resolution for symlinks will be skipped.
2152+
if (verbatimSymlinks) options |= copy_options::copy_symlinks;
2153+
2154+
auto src_path = std::filesystem::path(src.ToStringView());
2155+
auto dest_path = std::filesystem::path(dest.ToStringView());
2156+
2157+
auto resolved_src = src_path.lexically_normal();
2158+
auto resolved_dest = dest_path.lexically_normal();
2159+
2160+
if (resolved_src == resolved_dest) {
2161+
std::string message =
2162+
"src and dest cannot be the same " + resolved_src.string();
2163+
return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
2164+
}
2165+
2166+
auto get_stat = [](const std::filesystem::path& path)
2167+
-> std::optional<std::filesystem::file_status> {
2168+
std::error_code error_code{};
2169+
auto file_status = std::filesystem::status(path, error_code);
2170+
if (error_code) {
2171+
return std::nullopt;
2172+
}
2173+
return file_status;
2174+
};
2175+
2176+
auto src_type = get_stat(src_path);
2177+
auto dest_type = get_stat(dest_path);
2178+
2179+
if (!src_type.has_value()) {
2180+
std::string message = "Src path " + src_path.string() + " does not exist";
2181+
return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
2182+
}
2183+
2184+
const bool src_is_dir = src_type->type() == file_type::directory;
2185+
2186+
if (dest_type.has_value()) {
2187+
// Check if src and dest are identical.
2188+
if (std::filesystem::equivalent(src_path, dest_path)) {
2189+
std::string message =
2190+
"src and dest cannot be the same " + dest_path.string();
2191+
return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
2192+
}
2193+
2194+
const bool dest_is_dir = dest_type->type() == file_type::directory;
2195+
2196+
if (src_is_dir && !dest_is_dir) {
2197+
std::string message = "Cannot overwrite non-directory " +
2198+
src_path.string() + " with directory " +
2199+
dest_path.string();
2200+
return THROW_ERR_FS_CP_DIR_TO_NON_DIR(env, message.c_str());
2201+
}
2202+
2203+
if (!src_is_dir && dest_is_dir) {
2204+
std::string message = "Cannot overwrite directory " + dest_path.string() +
2205+
" with non-directory " + src_path.string();
2206+
return THROW_ERR_FS_CP_NON_DIR_TO_DIR(env, message.c_str());
2207+
}
2208+
}
2209+
2210+
if (src_is_dir && dest_path.string().starts_with(src_path.string())) {
2211+
std::string message = "Cannot copy " + src_path.string() +
2212+
" to a subdirectory of self " + dest_path.string();
2213+
return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
2214+
}
2215+
2216+
auto dest_parent = dest_path.parent_path();
2217+
// "/" parent is itself. Therefore, we need to check if the parent is the same
2218+
// as itself.
2219+
while (src_path.parent_path() != dest_parent &&
2220+
dest_parent.has_parent_path() &&
2221+
dest_parent.parent_path() != dest_parent) {
2222+
if (std::filesystem::equivalent(
2223+
src_path, dest_path.parent_path(), error_code)) {
2224+
std::string message = "Cannot copy " + src_path.string() +
2225+
" to a subdirectory of self " + dest_path.string();
2226+
return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
2227+
}
2228+
2229+
// If equivalent fails, it's highly likely that dest_parent does not exist
2230+
if (error_code) {
2231+
break;
2232+
}
2233+
2234+
dest_parent = dest_parent.parent_path();
2235+
}
2236+
2237+
if (src_is_dir && !recursive) {
2238+
std::string message = src_path.string() + " is a directory (not copied)";
2239+
return THROW_ERR_FS_EISDIR(env, message.c_str());
2240+
}
2241+
2242+
switch (src_type->type()) {
2243+
case file_type::socket: {
2244+
std::string message = "Cannot copy a socket file: " + dest_path.string();
2245+
return THROW_ERR_FS_CP_SOCKET(env, message.c_str());
2246+
}
2247+
case file_type::fifo: {
2248+
std::string message = "Cannot copy a FIFO pipe: " + dest_path.string();
2249+
return THROW_ERR_FS_CP_FIFO_PIPE(env, message.c_str());
2250+
}
2251+
case file_type::unknown: {
2252+
std::string message =
2253+
"Cannot copy an unknown file type: " + dest_path.string();
2254+
return THROW_ERR_FS_CP_UNKNOWN(env, message.c_str());
2255+
}
2256+
default:
2257+
break;
2258+
}
2259+
2260+
if (dest_type.has_value() && errorOnExist) {
2261+
std::string message = dest_path.string() + " already exists";
2262+
return THROW_ERR_FS_CP_EEXIST(env, message.c_str());
2263+
}
2264+
2265+
std::filesystem::create_directories(dest_path, error_code);
2266+
std::filesystem::copy(src_path, dest_path, options, error_code);
2267+
if (error_code) {
2268+
if (error_code == std::errc::file_exists) {
2269+
std::string message = "File already exists";
2270+
return THROW_ERR_FS_CP_EEXIST(env, message.c_str());
2271+
}
2272+
2273+
std::string message = "Unhandled error " +
2274+
std::to_string(error_code.value()) + ": " +
2275+
error_code.message();
2276+
return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
2277+
}
2278+
}
2279+
21092280
static void CopyFile(const FunctionCallbackInfo<Value>& args) {
21102281
Environment* env = Environment::GetCurrent(args);
21112282
Isolate* isolate = env->isolate();
@@ -3344,6 +3515,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
33443515
SetMethod(isolate, target, "writeFileUtf8", WriteFileUtf8);
33453516
SetMethod(isolate, target, "realpath", RealPath);
33463517
SetMethod(isolate, target, "copyFile", CopyFile);
3518+
SetMethod(isolate, target, "cpSync", CpSync);
33473519

33483520
SetMethod(isolate, target, "chmod", Chmod);
33493521
SetMethod(isolate, target, "fchmod", FChmod);
@@ -3466,6 +3638,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
34663638
registry->Register(WriteFileUtf8);
34673639
registry->Register(RealPath);
34683640
registry->Register(CopyFile);
3641+
registry->Register(CpSync);
34693642

34703643
registry->Register(Chmod);
34713644
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: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,19 @@ 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+
verbatimSymlinks?: boolean,
87+
): void;
88+
7989
function fchmod(fd: number, mode: number, req: FSReqCallback): void;
8090
function fchmod(fd: number, mode: number): void;
8191
function fchmod(fd: number, mode: number, usePromises: typeof kUsePromises): Promise<void>;
@@ -253,6 +263,7 @@ export interface FsBinding {
253263
chown: typeof InternalFSBinding.chown;
254264
close: typeof InternalFSBinding.close;
255265
copyFile: typeof InternalFSBinding.copyFile;
266+
cpSync: typeof InternalFSBinding.cpSync;
256267
fchmod: typeof InternalFSBinding.fchmod;
257268
fchown: typeof InternalFSBinding.fchown;
258269
fdatasync: typeof InternalFSBinding.fdatasync;

0 commit comments

Comments
 (0)