GeezerVison

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

GeezerVison is showing github.com — fs: improve error performance of `renameSync` · nodejs/node@39f31a3

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

benchmark/fs
- bench-renameSync.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-renameSync.js‎`

Copy file name to clipboard

+49Lines changed: 49 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,49 @@`
	`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`	`+const bench = common.createBenchmark(main, {`
	`9`	`+ type: ['invalid', 'valid'],`
	`10`	`+ n: [2e3],`
	`11`	`+});`
	`12`	`+`
	`13`	`+function main({ n, type }) {`
	`14`	`+ switch (type) {`
	`15`	`+ case 'invalid': {`
	`16`	`+ let hasError = false;`
	`17`	`+ bench.start();`
	`18`	`+ for (let i = 0; i < n; i++) {`
	`19`	`+ try {`
	`20`	+ fs.renameSync(tmpdir.resolve(`.non-existing-file-${i}`), tmpdir.resolve(`.new-file-${i}`));
	`21`	`+ } catch {`
	`22`	`+ hasError = true;`
	`23`	`+ }`
	`24`	`+ }`
	`25`	`+ bench.end(n);`
	`26`	`+ assert(hasError);`
	`27`	`+ break;`
	`28`	`+ }`
	`29`	`+ case 'valid': {`
	`30`	`+ tmpdir.refresh();`
	`31`	`+ for (let i = 0; i < n; i++) {`
	`32`	+ fs.writeFileSync(tmpdir.resolve(`.existing-file-${i}`), 'bench', 'utf8');
	`33`	`+ }`
	`34`	`+`
	`35`	`+ bench.start();`
	`36`	`+ for (let i = 0; i < n; i++) {`
	`37`	`+ fs.renameSync(`
	`38`	+ tmpdir.resolve(`.existing-file-${i}`),
	`39`	+ tmpdir.resolve(`.new-existing-file-${i}`),
	`40`	`+ );`
	`41`	`+ }`
	`42`	`+`
	`43`	`+ bench.end(n);`
	`44`	`+ break;`
	`45`	`+ }`
	`46`	`+ default:`
	`47`	`+ throw new Error('Invalid type');`
	`48`	`+ }`
	`49`	`+}`

Collapse file

`‎lib/fs.js‎`

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

+4-4Lines changed: 4 additions & 4 deletions

Original file line number	Diff line number	Diff line change
`@@ -1032,10 +1032,10 @@ function rename(oldPath, newPath, callback) {`
`1032`	`1032`	`function renameSync(oldPath, newPath) {`
`1033`	`1033`	`oldPath = getValidatedPath(oldPath, 'oldPath');`
`1034`	`1034`	`newPath = getValidatedPath(newPath, 'newPath');`
`1035`		`- const ctx = { path: oldPath, dest: newPath };`
`1036`		`- binding.rename(pathModule.toNamespacedPath(oldPath),`
`1037`		`- pathModule.toNamespacedPath(newPath), undefined, ctx);`
`1038`		`- handleErrorFromBinding(ctx);`
	`1035`	`+ binding.rename(`
	`1036`	`+ pathModule.toNamespacedPath(oldPath),`
	`1037`	`+ pathModule.toNamespacedPath(newPath),`
	`1038`	`+ );`
`1039`	`1039`	`}`
`1040`	`1040`
`1041`	`1041`	`/**`

Collapse file

`‎src/node_file.cc‎`

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

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

Original file line number	Diff line number	Diff line change
`@@ -1467,7 +1467,7 @@ static void Rename(const FunctionCallbackInfo<Value>& args) {`
`1467`	`1467`	`Isolate* isolate = env->isolate();`
`1468`	`1468`
`1469`	`1469`	`const int argc = args.Length();`
`1470`		`- CHECK_GE(argc, 3);`
	`1470`	`+ CHECK_GE(argc, 2);`
`1471`	`1471`
`1472`	`1472`	`BufferValue old_path(isolate, args[0]);`
`1473`	`1473`	`CHECK_NOT_NULL(*old_path);`
`@@ -1484,8 +1484,8 @@ static void Rename(const FunctionCallbackInfo<Value>& args) {`
`1484`	`1484`	`permission::PermissionScope::kFileSystemWrite,`
`1485`	`1485`	`new_path.ToStringView());`
`1486`	`1486`
`1487`		`- FSReqBase* req_wrap_async = GetReqWrap(args, 2);`
`1488`		`- if (req_wrap_async != nullptr) {`
	`1487`	`+ if (argc > 2) { // rename(old_path, new_path, req)`
	`1488`	`+ FSReqBase* req_wrap_async = GetReqWrap(args, 2);`
`1489`	`1489`	`FS_ASYNC_TRACE_BEGIN2(UV_FS_RENAME,`
`1490`	`1490`	`req_wrap_async,`
`1491`	`1491`	`"old_path",`
`@@ -1495,12 +1495,11 @@ static void Rename(const FunctionCallbackInfo<Value>& args) {`
`1495`	`1495`	`AsyncDestCall(env, req_wrap_async, args, "rename", *new_path,`
`1496`	`1496`	`new_path.length(), UTF8, AfterNoArgs, uv_fs_rename,`
`1497`	`1497`	`old_path, new_path);`
`1498`		`- } else {`
`1499`		`- CHECK_EQ(argc, 4);`
`1500`		`- FSReqWrapSync req_wrap_sync;`
	`1498`	`+ } else { // rename(old_path, new_path)`
	`1499`	`+ FSReqWrapSync req_wrap_sync("rename", old_path, new_path);`
`1501`	`1500`	`FS_SYNC_TRACE_BEGIN(rename);`
`1502`		`- SyncCall(env, args[3], &req_wrap_sync, "rename", uv_fs_rename,`
`1503`		`- old_path, new_path);`
	`1501`	`+ SyncCallAndThrowOnError(`
	`1502`	`+ env, &req_wrap_sync, uv_fs_rename, old_path, new_path);`
`1504`	`1503`	`FS_SYNC_TRACE_END(rename);`
`1505`	`1504`	`}`
`1506`	`1505`	`}`

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
`@@ -183,6 +183,7 @@ declare namespace InternalFSBinding {`
`183`	`183`	`function rename(oldPath: string, newPath: string, req: FSReqCallback): void;`
`184`	`184`	`function rename(oldPath: string, newPath: string, req: undefined, ctx: FSSyncContext): void;`
`185`	`185`	`function rename(oldPath: string, newPath: string, usePromises: typeof kUsePromises): Promise<void>;`
	`186`	`+ function rename(oldPath: string, newPath: string): void;`
`186`	`187`
`187`	`188`	`function rmdir(path: string, req: FSReqCallback): void;`
`188`	`189`	`function rmdir(path: string, req: undefined, ctx: FSSyncContext): void;`

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.