Skip to content

Commit

Permalink
fs: improve error performance of linkSync
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
anonrig authored and targos committed Nov 11, 2023
1 parent ea7902d commit 9dba771
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 13 deletions.
50 changes: 50 additions & 0 deletions benchmark/fs/bench-linkSync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
'use strict';

const common = require('../common');
const fs = require('fs');
const assert = require('assert');
const tmpdir = require('../../test/common/tmpdir');

tmpdir.refresh();
const tmpfile = tmpdir.resolve(`.bench-file-data-${Date.now()}`);
fs.writeFileSync(tmpfile, 'bench-file', 'utf-8');

const bench = common.createBenchmark(main, {
type: ['valid', 'invalid'],
n: [1e3],
});

function main({ n, type }) {
switch (type) {
case 'valid': {
bench.start();
for (let i = 0; i < n; i++) {
fs.linkSync(tmpfile, tmpdir.resolve(`.valid-${i}`), 'file');
}
bench.end(n);

break;
}

case 'invalid': {
let hasError = false;
bench.start();
for (let i = 0; i < n; i++) {
try {
fs.linkSync(
tmpdir.resolve(`.non-existing-file-for-linkSync-${i}`),
__filename,
'file',
);
} catch {
hasError = true;
}
}
bench.end(n);
assert(hasError);
break;
}
default:
new Error('Invalid type');
}
}
10 changes: 4 additions & 6 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1845,12 +1845,10 @@ function linkSync(existingPath, newPath) {
existingPath = getValidatedPath(existingPath, 'existingPath');
newPath = getValidatedPath(newPath, 'newPath');

const ctx = { path: existingPath, dest: newPath };
const result = binding.link(pathModule.toNamespacedPath(existingPath),
pathModule.toNamespacedPath(newPath),
undefined, ctx);
handleErrorFromBinding(ctx);
return result;
binding.link(
pathModule.toNamespacedPath(existingPath),
pathModule.toNamespacedPath(newPath),
);
}

/**
Expand Down
12 changes: 5 additions & 7 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1377,7 +1377,7 @@ static void Link(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = env->isolate();

const int argc = args.Length();
CHECK_GE(argc, 3);
CHECK_GE(argc, 2);

BufferValue src(isolate, args[0]);
CHECK_NOT_NULL(*src);
Expand All @@ -1395,8 +1395,8 @@ static void Link(const FunctionCallbackInfo<Value>& args) {
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kFileSystemWrite, dest_view);

FSReqBase* req_wrap_async = GetReqWrap(args, 2);
if (req_wrap_async != nullptr) { // link(src, dest, req)
if (argc > 2) { // link(src, dest, req)
FSReqBase* req_wrap_async = GetReqWrap(args, 2);
FS_ASYNC_TRACE_BEGIN2(UV_FS_LINK,
req_wrap_async,
"src",
Expand All @@ -1406,11 +1406,9 @@ static void Link(const FunctionCallbackInfo<Value>& args) {
AsyncDestCall(env, req_wrap_async, args, "link", *dest, dest.length(), UTF8,
AfterNoArgs, uv_fs_link, *src, *dest);
} else { // link(src, dest)
CHECK_EQ(argc, 4);
FSReqWrapSync req_wrap_sync;
FSReqWrapSync req_wrap_sync("link", *src, *dest);
FS_SYNC_TRACE_BEGIN(link);
SyncCall(env, args[3], &req_wrap_sync, "link",
uv_fs_link, *src, *dest);
SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_link, *src, *dest);
FS_SYNC_TRACE_END(link);
}
}
Expand Down
1 change: 1 addition & 0 deletions typings/internalBinding/fs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ declare namespace InternalFSBinding {
function link(existingPath: string, newPath: string, req: FSReqCallback): void;
function link(existingPath: string, newPath: string, req: undefined, ctx: FSSyncContext): void;
function link(existingPath: string, newPath: string, usePromises: typeof kUsePromises): Promise<void>;
function link(existingPath: string, newPath: string): void;

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

0 comments on commit 9dba771

Please sign in to comment.