Skip to content

Commit e3e4afc

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

File tree

6 files changed

+244
-6
lines changed

6 files changed

+244
-6
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: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,40 @@ 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)) {
70+
return internalFsBinding.cpSync(
71+
src,
72+
dest,
73+
opts.preserveTimestamps,
74+
opts.dereference,
75+
opts.errorOnExist,
76+
opts.force,
77+
opts.recursive,
78+
opts.verbatimSymlinks,
79+
);
80+
}
81+
82+
5183
// Warn about using preserveTimestamps on 32-bit node
5284
if (opts.preserveTimestamps && process.arch === 'ia32') {
5385
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: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2106,6 +2106,169 @@ 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+
8); // src, dest, preserveTimestamps, dereference, 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 dereference = args[3]->IsTrue();
2129+
bool errorOnExist = args[4]->IsTrue();
2130+
bool force = args[5]->IsTrue();
2131+
bool recursive = args[6]->IsTrue();
2132+
bool verbatimSymlinks = args[7]->IsTrue();
2133+
2134+
using copy_options = std::filesystem::copy_options;
2135+
using file_type = std::filesystem::file_type;
2136+
2137+
std::error_code error_code{};
2138+
copy_options options = copy_options ::skip_existing;
2139+
2140+
// When true timestamps from src will be preserved.
2141+
if (preserveTimestamps) options |= copy_options::create_hard_links;
2142+
// Dereference symbolic links.
2143+
if (dereference) options |= copy_options::copy_symlinks;
2144+
// Overwrite existing file or directory.
2145+
if (force) options |= copy_options::overwrite_existing;
2146+
// Copy directories recursively.
2147+
if (recursive) options |= copy_options::recursive;
2148+
// When true, path resolution for symlinks will be skipped.
2149+
if (verbatimSymlinks) options |= copy_options::skip_symlinks;
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+
std::string message = "Unhandled error " +
2266+
std::to_string(error_code.value()) + ": " +
2267+
error_code.message();
2268+
return THROW_ERR_FS_CP_EINVAL(env, message.c_str());
2269+
}
2270+
}
2271+
21092272
static void CopyFile(const FunctionCallbackInfo<Value>& args) {
21102273
Environment* env = Environment::GetCurrent(args);
21112274
Isolate* isolate = env->isolate();
@@ -3344,6 +3507,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
33443507
SetMethod(isolate, target, "writeFileUtf8", WriteFileUtf8);
33453508
SetMethod(isolate, target, "realpath", RealPath);
33463509
SetMethod(isolate, target, "copyFile", CopyFile);
3510+
SetMethod(isolate, target, "cpSync", CpSync);
33473511

33483512
SetMethod(isolate, target, "chmod", Chmod);
33493513
SetMethod(isolate, target, "fchmod", FChmod);
@@ -3466,6 +3630,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
34663630
registry->Register(WriteFileUtf8);
34673631
registry->Register(RealPath);
34683632
registry->Register(CopyFile);
3633+
registry->Register(CpSync);
34693634

34703635
registry->Register(Chmod);
34713636
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

‎typings/internalBinding/fs.d.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,20 @@ 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+
dereference?: boolean,
84+
errorOnExist?: boolean,
85+
force?: boolean,
86+
recursive?: boolean,
87+
verbatimSymlinks?: boolean,
88+
): void;
89+
7990
function fchmod(fd: number, mode: number, req: FSReqCallback): void;
8091
function fchmod(fd: number, mode: number): void;
8192
function fchmod(fd: number, mode: number, usePromises: typeof kUsePromises): Promise<void>;
@@ -253,6 +264,7 @@ export interface FsBinding {
253264
chown: typeof InternalFSBinding.chown;
254265
close: typeof InternalFSBinding.close;
255266
copyFile: typeof InternalFSBinding.copyFile;
267+
cpSync: typeof InternalFSBinding.cpSync;
256268
fchmod: typeof InternalFSBinding.fchmod;
257269
fchown: typeof InternalFSBinding.fchown;
258270
fdatasync: typeof InternalFSBinding.fdatasync;

0 commit comments

Comments
 (0)