GeezerVison

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

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

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
`@@ -1726,11 +1726,10 @@ function readlink(path, options, callback) {`
`1726`	`1726`	`function readlinkSync(path, options) {`
`1727`	`1727`	`options = getOptions(options);`
`1728`	`1728`	`path = getValidatedPath(path, 'oldPath');`
`1729`		`- const ctx = { path };`
`1730`		`- const result = binding.readlink(pathModule.toNamespacedPath(path),`
`1731`		`- options.encoding, undefined, ctx);`
`1732`		`- handleErrorFromBinding(ctx);`
`1733`		`- return result;`
	`1729`	`+ return binding.readlink(`
	`1730`	`+ pathModule.toNamespacedPath(path),`
	`1731`	`+ options.encoding,`
	`1732`	`+ );`
`1734`	`1733`	`}`
`1735`	`1734`
`1736`	`1735`	`/**`
`@@ -2712,10 +2711,8 @@ function realpathSync(p, options) {`
`2712`	`2711`	`}`
`2713`	`2712`	`}`
`2714`	`2713`	`if (linkTarget === null) {`
`2715`		`- const ctx = { path: base };`
`2716`	`2714`	`binding.stat(baseLong, false, undefined, true);`
`2717`		`- linkTarget = binding.readlink(baseLong, undefined, undefined, ctx);`
`2718`		`- handleErrorFromBinding(ctx);`
	`2715`	`+ linkTarget = binding.readlink(baseLong, undefined);`
`2719`	`2716`	`}`
`2720`	`2717`	`resolvedLink = pathModule.resolve(previous, linkTarget);`
`2721`	`2718`

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
`@@ -1418,7 +1418,7 @@ static void ReadLink(const FunctionCallbackInfo<Value>& args) {`
`1418`	`1418`	`Isolate* isolate = env->isolate();`
`1419`	`1419`
`1420`	`1420`	`const int argc = args.Length();`
`1421`		`- CHECK_GE(argc, 3);`
	`1421`	`+ CHECK_GE(argc, 2);`
`1422`	`1422`
`1423`	`1423`	`BufferValue path(isolate, args[0]);`
`1424`	`1424`	`CHECK_NOT_NULL(*path);`
`@@ -1427,21 +1427,20 @@ static void ReadLink(const FunctionCallbackInfo<Value>& args) {`
`1427`	`1427`
`1428`	`1428`	`const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8);`
`1429`	`1429`
`1430`		`- FSReqBase* req_wrap_async = GetReqWrap(args, 2);`
`1431`		`- if (req_wrap_async != nullptr) { // readlink(path, encoding, req)`
	`1430`	`+ if (argc > 2) { // readlink(path, encoding, req)`
	`1431`	`+ FSReqBase* req_wrap_async = GetReqWrap(args, 2);`
`1432`	`1432`	`FS_ASYNC_TRACE_BEGIN1(`
`1433`	`1433`	`UV_FS_READLINK, req_wrap_async, "path", TRACE_STR_COPY(*path))`
`1434`	`1434`	`AsyncCall(env, req_wrap_async, args, "readlink", encoding, AfterStringPtr,`
`1435`	`1435`	`uv_fs_readlink, *path);`
`1436`		`- } else {`
`1437`		`- CHECK_EQ(argc, 4);`
`1438`		`- FSReqWrapSync req_wrap_sync;`
	`1436`	`+ } else { // readlink(path, encoding)`
	`1437`	`+ FSReqWrapSync req_wrap_sync("readlink", *path);`
`1439`	`1438`	`FS_SYNC_TRACE_BEGIN(readlink);`
`1440`		`- int err = SyncCall(env, args[3], &req_wrap_sync, "readlink",`
`1441`		`- uv_fs_readlink, *path);`
	`1439`	`+ int err =`
	`1440`	`+ SyncCallAndThrowOnError(env, &req_wrap_sync, uv_fs_readlink, *path);`
`1442`	`1441`	`FS_SYNC_TRACE_END(readlink);`
`1443`	`1442`	`if (err < 0) {`
`1444`		`- return; // syscall failed, no need to continue, error info is in ctx`
	`1443`	`+ return;`
`1445`	`1444`	`}`
`1446`	`1445`	`const char* link_path = static_cast<const char*>(req_wrap_sync.req.ptr);`
`1447`	`1446`
`@@ -1451,8 +1450,7 @@ static void ReadLink(const FunctionCallbackInfo<Value>& args) {`
`1451`	`1450`	`encoding,`
`1452`	`1451`	`&error);`
`1453`	`1452`	`if (rc.IsEmpty()) {`
`1454`		`- Local<Object> ctx = args[3].As<Object>();`
`1455`		`- ctx->Set(env->context(), env->error_string(), error).Check();`
	`1453`	`+ env->isolate()->ThrowException(error);`
`1456`	`1454`	`return;`
`1457`	`1455`	`}`
`1458`	`1456`

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.