GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — fs: improve error performance of `readlinkSync` · nodejs/node@bc6f279

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

benchmark/fs
- bench-readlinkSync.js
lib
- fs.js
src
- node_file.cc
typings/internalBinding
- fs.d.ts

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎benchmark/fs/bench-readlinkSync.js‎`

Copy file name to clipboard

+54Lines changed: 54 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,54 @@`
	`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`	`+if (process.platform === 'win32') {`
	`9`	+ console.log('Skipping: Windows does not play well with `symlinkSync`');
	`10`	`+ process.exit(0);`
	`11`	`+}`
	`12`	`+`
	`13`	`+const bench = common.createBenchmark(main, {`
	`14`	`+ type: ['valid', 'invalid'],`
	`15`	`+ n: [1e3],`
	`16`	`+});`
	`17`	`+`
	`18`	`+function main({ n, type }) {`
	`19`	`+ switch (type) {`
	`20`	`+ case 'valid': {`
	`21`	`+ tmpdir.refresh();`
	`22`	+ const tmpfile = tmpdir.resolve(`.readlink-file-${process.pid}`);
	`23`	`+ fs.writeFileSync(tmpfile, 'data', 'utf8');`
	`24`	`+ let returnValue;`
	`25`	`+ for (let i = 0; i < n; i++) {`
	`26`	+ fs.symlinkSync(tmpfile, tmpdir.resolve(`.readlink-sync-${i}`), 'file');
	`27`	`+ }`
	`28`	`+ bench.start();`
	`29`	`+ for (let i = 0; i < n; i++) {`
	`30`	+ returnValue = fs.readlinkSync(tmpdir.resolve(`.readlink-sync-${i}`), { encoding: 'utf8' });
	`31`	`+ }`
	`32`	`+ bench.end(n);`
	`33`	`+ assert(returnValue);`
	`34`	`+ break;`
	`35`	`+ }`
	`36`	`+`
	`37`	`+ case 'invalid': {`
	`38`	`+ let hasError = false;`
	`39`	`+ bench.start();`
	`40`	`+ for (let i = 0; i < n; i++) {`
	`41`	`+ try {`
	`42`	`+ fs.readlinkSync(tmpdir.resolve('.non-existing-file-for-readlinkSync'));`
	`43`	`+ } catch {`
	`44`	`+ hasError = true;`
	`45`	`+ }`
	`46`	`+ }`
	`47`	`+ bench.end(n);`
	`48`	`+ assert(hasError);`
	`49`	`+ break;`
	`50`	`+ }`
	`51`	`+ default:`
	`52`	`+ new Error('Invalid type');`
	`53`	`+ }`
	`54`	`+}`

Collapse file

`‎lib/fs.js‎`

Copy file name to clipboardExpand all lines: lib/fs.js

+5-8Lines changed: 5 additions & 8 deletions

Original file line number	Diff line number	Diff line change
`@@ -1728,11 +1728,10 @@ function readlink(path, options, callback) {`
`1728`	`1728`	`function readlinkSync(path, options) {`
`1729`	`1729`	`options = getOptions(options);`
`1730`	`1730`	`path = getValidatedPath(path, 'oldPath');`
`1731`		`- const ctx = { path };`
`1732`		`- const result = binding.readlink(pathModule.toNamespacedPath(path),`
`1733`		`- options.encoding, undefined, ctx);`
`1734`		`- handleErrorFromBinding(ctx);`
`1735`		`- return result;`
	`1731`	`+ return binding.readlink(`
	`1732`	`+ pathModule.toNamespacedPath(path),`
	`1733`	`+ options.encoding,`
	`1734`	`+ );`
`1736`	`1735`	`}`
`1737`	`1736`
`1738`	`1737`	`/**`
`@@ -2714,10 +2713,8 @@ function realpathSync(p, options) {`
`2714`	`2713`	`}`
`2715`	`2714`	`}`
`2716`	`2715`	`if (linkTarget === null) {`
`2717`		`- const ctx = { path: base };`
`2718`	`2716`	`binding.stat(baseLong, false, undefined, true);`
`2719`		`- linkTarget = binding.readlink(baseLong, undefined, undefined, ctx);`
`2720`		`- handleErrorFromBinding(ctx);`
	`2717`	`+ linkTarget = binding.readlink(baseLong, undefined);`
`2721`	`2718`	`}`
`2722`	`2719`	`resolvedLink = pathModule.resolve(previous, linkTarget);`
`2723`	`2720`

Collapse file

`‎src/node_file.cc‎`

Copy file name to clipboardExpand all lines: src/node_file.cc

+9-11Lines changed: 9 additions & 11 deletions

Original file line number	Diff line number	Diff line change
`@@ -1383,7 +1383,7 @@ static void ReadLink(const FunctionCallbackInfo<Value>& args) {`
`1383`	`1383`	`Isolate* isolate = env->isolate();`
`1384`	`1384`
`1385`	`1385`	`const int argc = args.Length();`
`1386`		`- CHECK_GE(argc, 3);`
	`1386`	`+ CHECK_GE(argc, 2);`
`1387`	`1387`
`1388`	`1388`	`BufferValue path(isolate, args[0]);`
`1389`	`1389`	`CHECK_NOT_NULL(*path);`
`@@ -1392,21 +1392,20 @@ static void ReadLink(const FunctionCallbackInfo<Value>& args) {`
`1392`	`1392`
`1393`	`1393`	`const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8);`
`1394`	`1394`
`1395`		`- FSReqBase* req_wrap_async = GetReqWrap(args, 2);`
`1396`		`- if (req_wrap_async != nullptr) { // readlink(path, encoding, req)`
	`1395`	`+ if (argc > 2) { // readlink(path, encoding, req)`
	`1396`	`+ FSReqBase* req_wrap_async = GetReqWrap(args, 2);`
`1397`	`1397`	`FS_ASYNC_TRACE_BEGIN1(`
`1398`	`1398`	`UV_FS_READLINK, req_wrap_async, "path", TRACE_STR_COPY(*path))`
`1399`	`1399`	`AsyncCall(env, req_wrap_async, args, "readlink", encoding, AfterStringPtr,`
`1400`	`1400`	`uv_fs_readlink, *path);`
`1401`		`- } else {`
`1402`		`- CHECK_EQ(argc, 4);`
`1403`		`- FSReqWrapSync req_wrap_sync;`
	`1401`	`+ } else { // readlink(path, encoding)`
	`1402`	`+ FSReqWrapSync req_wrap_sync("readlink", *path);`
`1404`	`1403`	`FS_SYNC_TRACE_BEGIN(readlink);`
`1405`		`- int err = SyncCall(env, args[3], &req_wrap_sync, "readlink",`
`1406`		`- uv_fs_readlink, *path);`
	`1404`	`+ int err =`
	`1405`	`+ SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_readlink, *path);`
`1407`	`1406`	`FS_SYNC_TRACE_END(readlink);`
`1408`	`1407`	`if (err < 0) {`
`1409`		`- return; // syscall failed, no need to continue, error info is in ctx`
	`1408`	`+ return;`
`1410`	`1409`	`}`
`1411`	`1410`	`const char* link_path = static_cast<const char*>(req_wrap_sync.req.ptr);`
`1412`	`1411`
`@@ -1416,8 +1415,7 @@ static void ReadLink(const FunctionCallbackInfo<Value>& args) {`
`1416`	`1415`	`encoding,`
`1417`	`1416`	`&error);`
`1418`	`1417`	`if (rc.IsEmpty()) {`
`1419`		`- Local<Object> ctx = args[3].As<Object>();`
`1420`		`- ctx->Set(env->context(), env->error_string(), error).Check();`
	`1418`	`+ env->isolate()->ThrowException(error);`
`1421`	`1419`	`return;`
`1422`	`1420`	`}`
`1423`	`1421`

Collapse file

`‎typings/internalBinding/fs.d.ts‎`

Copy file name to clipboardExpand all lines: typings/internalBinding/fs.d.ts

+1Lines changed: 1 addition & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -178,6 +178,7 @@ declare namespace InternalFSBinding {`
`178`	`178`	`function readlink(path: StringOrBuffer, encoding: unknown, req: FSReqCallback<string \| Buffer>): void;`
`179`	`179`	`function readlink(path: StringOrBuffer, encoding: unknown, req: undefined, ctx: FSSyncContext): string \| Buffer;`
`180`	`180`	`function readlink(path: StringOrBuffer, encoding: unknown, usePromises: typeof kUsePromises): Promise<string \| Buffer>;`
	`181`	`+ function readlink(path: StringOrBuffer, encoding: unknown): StringOrBuffer;`
`181`	`182`
`182`	`183`	`function realpath(path: StringOrBuffer, encoding: unknown, req: FSReqCallback<string \| Buffer>): void;`
`183`	`184`	`function realpath(path: StringOrBuffer, encoding: unknown, req: undefined, ctx: FSSyncContext): string \| Buffer;`

0 commit comments

Comments

0 (0)

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.