Skip to content

Commit 81f1527

Browse files
anonrigtargos
authored andcommitted
fs: improve error performance of linkSync
PR-URL: #49962 Refs: nodejs/performance#106 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
1 parent f766c04 commit 81f1527

File tree

4 files changed

+60
-13
lines changed

4 files changed

+60
-13
lines changed

benchmark/fs/bench-linkSync.js

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const fs = require('fs');
5+
const assert = require('assert');
6+
const tmpdir = require('../../test/common/tmpdir');
7+
8+
tmpdir.refresh();
9+
const tmpfile = tmpdir.resolve(`.bench-file-data-${Date.now()}`);
10+
fs.writeFileSync(tmpfile, 'bench-file', 'utf-8');
11+
12+
const bench = common.createBenchmark(main, {
13+
type: ['valid', 'invalid'],
14+
n: [1e3],
15+
});
16+
17+
function main({ n, type }) {
18+
switch (type) {
19+
case 'valid': {
20+
bench.start();
21+
for (let i = 0; i < n; i++) {
22+
fs.linkSync(tmpfile, tmpdir.resolve(`.valid-${i}`), 'file');
23+
}
24+
bench.end(n);
25+
26+
break;
27+
}
28+
29+
case 'invalid': {
30+
let hasError = false;
31+
bench.start();
32+
for (let i = 0; i < n; i++) {
33+
try {
34+
fs.linkSync(
35+
tmpdir.resolve(`.non-existing-file-for-linkSync-${i}`),
36+
__filename,
37+
'file',
38+
);
39+
} catch {
40+
hasError = true;
41+
}
42+
}
43+
bench.end(n);
44+
assert(hasError);
45+
break;
46+
}
47+
default:
48+
new Error('Invalid type');
49+
}
50+
}

lib/fs.js

+4-6
Original file line numberDiff line numberDiff line change
@@ -1847,12 +1847,10 @@ function linkSync(existingPath, newPath) {
18471847
existingPath = getValidatedPath(existingPath, 'existingPath');
18481848
newPath = getValidatedPath(newPath, 'newPath');
18491849

1850-
const ctx = { path: existingPath, dest: newPath };
1851-
const result = binding.link(pathModule.toNamespacedPath(existingPath),
1852-
pathModule.toNamespacedPath(newPath),
1853-
undefined, ctx);
1854-
handleErrorFromBinding(ctx);
1855-
return result;
1850+
binding.link(
1851+
pathModule.toNamespacedPath(existingPath),
1852+
pathModule.toNamespacedPath(newPath),
1853+
);
18561854
}
18571855

18581856
/**

src/node_file.cc

+5-7
Original file line numberDiff line numberDiff line change
@@ -1342,7 +1342,7 @@ static void Link(const FunctionCallbackInfo<Value>& args) {
13421342
Isolate* isolate = env->isolate();
13431343

13441344
const int argc = args.Length();
1345-
CHECK_GE(argc, 3);
1345+
CHECK_GE(argc, 2);
13461346

13471347
BufferValue src(isolate, args[0]);
13481348
CHECK_NOT_NULL(*src);
@@ -1360,8 +1360,8 @@ static void Link(const FunctionCallbackInfo<Value>& args) {
13601360
THROW_IF_INSUFFICIENT_PERMISSIONS(
13611361
env, permission::PermissionScope::kFileSystemWrite, dest_view);
13621362

1363-
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
1364-
if (req_wrap_async != nullptr) { // link(src, dest, req)
1363+
if (argc > 2) { // link(src, dest, req)
1364+
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
13651365
FS_ASYNC_TRACE_BEGIN2(UV_FS_LINK,
13661366
req_wrap_async,
13671367
"src",
@@ -1371,11 +1371,9 @@ static void Link(const FunctionCallbackInfo<Value>& args) {
13711371
AsyncDestCall(env, req_wrap_async, args, "link", *dest, dest.length(), UTF8,
13721372
AfterNoArgs, uv_fs_link, *src, *dest);
13731373
} else { // link(src, dest)
1374-
CHECK_EQ(argc, 4);
1375-
FSReqWrapSync req_wrap_sync;
1374+
FSReqWrapSync req_wrap_sync("link", *src, *dest);
13761375
FS_SYNC_TRACE_BEGIN(link);
1377-
SyncCall(env, args[3], &req_wrap_sync, "link",
1378-
uv_fs_link, *src, *dest);
1376+
SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_link, *src, *dest);
13791377
FS_SYNC_TRACE_END(link);
13801378
}
13811379
}

typings/internalBinding/fs.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ declare namespace InternalFSBinding {
121121
function link(existingPath: string, newPath: string, req: FSReqCallback): void;
122122
function link(existingPath: string, newPath: string, req: undefined, ctx: FSSyncContext): void;
123123
function link(existingPath: string, newPath: string, usePromises: typeof kUsePromises): Promise<void>;
124+
function link(existingPath: string, newPath: string): void;
124125

125126
function lstat(path: StringOrBuffer, useBigint: boolean, req: FSReqCallback<Float64Array | BigUint64Array>): void;
126127
function lstat(path: StringOrBuffer, useBigint: true, req: FSReqCallback<BigUint64Array>): void;

0 commit comments

Comments
 (0)