GeezerVison

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

GeezerVison is showing github.com — fs: improve `readFileSync` with file descriptors · nodejs/node@07d0518

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

benchmark/fs
- readFileSync.js
lib
- fs.js
- internal/fs
  - sync.js
src
- node_file.cc

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎benchmark/fs/readFileSync.js‎`

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

+15-3Lines changed: 15 additions & 3 deletions

Original file line number	Diff line number	Diff line change
`@@ -6,12 +6,21 @@ const fs = require('fs');`
`6`	`6`	`const bench = common.createBenchmark(main, {`
`7`	`7`	`encoding: ['undefined', 'utf8'],`
`8`	`8`	`path: ['existing', 'non-existing'],`
`9`		`- n: [60e1],`
	`9`	`+ hasFileDescriptor: ['true', 'false'],`
	`10`	`+ n: [1e4],`
`10`	`11`	`});`
`11`	`12`
`12`		`-function main({ n, encoding, path }) {`
	`13`	`+function main({ n, encoding, path, hasFileDescriptor }) {`
`13`	`14`	`const enc = encoding === 'undefined' ? undefined : encoding;`
`14`		`- const file = path === 'existing' ? __filename : '/tmp/not-found';`
	`15`	`+ let file;`
	`16`	`+ let shouldClose = false;`
	`17`	`+`
	`18`	`+ if (hasFileDescriptor === 'true') {`
	`19`	`+ shouldClose = path === 'existing';`
	`20`	`+ file = path === 'existing' ? fs.openSync(__filename) : -1;`
	`21`	`+ } else {`
	`22`	`+ file = path === 'existing' ? __filename : '/tmp/not-found';`
	`23`	`+ }`
`15`	`24`	`bench.start();`
`16`	`25`	`for (let i = 0; i < n; ++i) {`
`17`	`26`	`try {`
`@@ -21,4 +30,7 @@ function main({ n, encoding, path }) {`
`21`	`30`	`}`
`22`	`31`	`}`
`23`	`32`	`bench.end(n);`
	`33`	`+ if (shouldClose) {`
	`34`	`+ fs.closeSync(file);`
	`35`	`+ }`
`24`	`36`	`}`

Collapse file

`‎lib/fs.js‎`

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

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

Original file line number	Diff line number	Diff line change
`@@ -437,13 +437,11 @@ function tryReadSync(fd, isUserFd, buffer, pos, len) {`
`437`	`437`	`function readFileSync(path, options) {`
`438`	`438`	`options = getOptions(options, { flag: 'r' });`
`439`	`439`
`440`		`- const isUserFd = isFd(path); // File descriptor ownership`
`441`		`-`
`442`		`- // TODO(@anonrig): Do not handle file descriptor ownership for now.`
`443`		`- if (!isUserFd && (options.encoding === 'utf8' \|\| options.encoding === 'utf-8')) {`
	`440`	`+ if (options.encoding === 'utf8' \|\| options.encoding === 'utf-8') {`
`444`	`441`	`return syncFs.readFileUtf8(path, options.flag);`
`445`	`442`	`}`
`446`	`443`
	`444`	`+ const isUserFd = isFd(path); // File descriptor ownership`
`447`	`445`	`const fd = isUserFd ? path : fs.openSync(path, options.flag, 0o666);`
`448`	`446`
`449`	`447`	`const stats = tryStatSync(fd, isUserFd);`

Collapse file

`‎lib/internal/fs/sync.js‎`

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

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

Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ const {`
`9`	`9`	`getStatFsFromBinding,`
`10`	`10`	`getValidatedFd,`
`11`	`11`	`} = require('internal/fs/utils');`
`12`		`-const { parseFileMode } = require('internal/validators');`
	`12`	`+const { parseFileMode, isInt32 } = require('internal/validators');`
`13`	`13`
`14`	`14`	`const binding = internalBinding('fs');`
`15`	`15`
`@@ -19,7 +19,9 @@ const binding = internalBinding('fs');`
`19`	`19`	`* @return {string}`
`20`	`20`	`*/`
`21`	`21`	`function readFileUtf8(path, flag) {`
`22`		`- path = pathModule.toNamespacedPath(getValidatedPath(path));`
	`22`	`+ if (!isInt32(path)) {`
	`23`	`+ path = pathModule.toNamespacedPath(getValidatedPath(path));`
	`24`	`+ }`
`23`	`25`	`return binding.readFileUtf8(path, stringToFlags(flag));`
`24`	`26`	`}`
`25`	`27`

Collapse file

`‎src/node_file.cc‎`

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

+29-18Lines changed: 29 additions & 18 deletions

Original file line number	Diff line number	Diff line change
`@@ -2190,7 +2190,7 @@ static void OpenSync(const FunctionCallbackInfo<Value>& args) {`
`2190`	`2190`	`uv_fs_t req;`
`2191`	`2191`	`auto make = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });`
`2192`	`2192`	`FS_SYNC_TRACE_BEGIN(open);`
`2193`		`- int err = uv_fs_open(nullptr, &req, *path, flags, mode, nullptr);`
	`2193`	`+ auto err = uv_fs_open(nullptr, &req, *path, flags, mode, nullptr);`
`2194`	`2194`	`FS_SYNC_TRACE_END(open);`
`2195`	`2195`	`if (err < 0) {`
`2196`	`2196`	`return env->ThrowUVException(err, "open", nullptr, path.out());`
`@@ -2581,30 +2581,41 @@ static void ReadFileUtf8(const FunctionCallbackInfo<Value>& args) {`
`2581`	`2581`
`2582`	`2582`	`CHECK_GE(args.Length(), 2);`
`2583`	`2583`
`2584`		`- BufferValue path(env->isolate(), args[0]);`
`2585`		`- CHECK_NOT_NULL(*path);`
`2586`		`-`
`2587`	`2584`	`CHECK(args[1]->IsInt32());`
`2588`	`2585`	`const int flags = args[1].As<Int32>()->Value();`
`2589`	`2586`
`2590`		`- if (CheckOpenPermissions(env, path, flags).IsNothing()) return;`
`2591`		`-`
	`2587`	`+ uv_file file;`
`2592`	`2588`	`uv_fs_t req;`
`2593`		`- auto defer_req_cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });`
`2594`	`2589`
`2595`		`- FS_SYNC_TRACE_BEGIN(open);`
`2596`		`- uv_file file = uv_fs_open(nullptr, &req, *path, flags, 438, nullptr);`
`2597`		`- FS_SYNC_TRACE_END(open);`
`2598`		`- if (req.result < 0) {`
`2599`		`- // req will be cleaned up by scope leave.`
`2600`		`- return env->ThrowUVException(req.result, "open", nullptr, path.out());`
	`2590`	`+ bool is_fd = args[0]->IsInt32();`
	`2591`	`+`
	`2592`	`+ // Check for file descriptor`
	`2593`	`+ if (is_fd) {`
	`2594`	`+ file = args[0].As<Int32>()->Value();`
	`2595`	`+ } else {`
	`2596`	`+ BufferValue path(env->isolate(), args[0]);`
	`2597`	`+ CHECK_NOT_NULL(*path);`
	`2598`	`+ if (CheckOpenPermissions(env, path, flags).IsNothing()) return;`
	`2599`	`+`
	`2600`	`+ FS_SYNC_TRACE_BEGIN(open);`
	`2601`	`+ file = uv_fs_open(nullptr, &req, *path, flags, O_RDONLY, nullptr);`
	`2602`	`+ FS_SYNC_TRACE_END(open);`
	`2603`	`+ if (req.result < 0) {`
	`2604`	`+ uv_fs_req_cleanup(&req);`
	`2605`	`+ // req will be cleaned up by scope leave.`
	`2606`	`+ return env->ThrowUVException(req.result, "open", nullptr, path.out());`
	`2607`	`+ }`
`2601`	`2608`	`}`
`2602`	`2609`
`2603`		`- auto defer_close = OnScopeLeave([file]() {`
`2604`		`- uv_fs_t close_req;`
`2605`		`- CHECK_EQ(0, uv_fs_close(nullptr, &close_req, file, nullptr));`
`2606`		`- uv_fs_req_cleanup(&close_req);`
	`2610`	`+ auto defer_close = OnScopeLeave([file, is_fd, &req]() {`
	`2611`	`+ if (!is_fd) {`
	`2612`	`+ FS_SYNC_TRACE_BEGIN(close);`
	`2613`	`+ CHECK_EQ(0, uv_fs_close(nullptr, &req, file, nullptr));`
	`2614`	`+ FS_SYNC_TRACE_END(close);`
	`2615`	`+ }`
	`2616`	`+ uv_fs_req_cleanup(&req);`
`2607`	`2617`	`});`
	`2618`	`+`
`2608`	`2619`	`std::string result{};`
`2609`	`2620`	`char buffer[8192];`
`2610`	`2621`	`uv_buf_t buf = uv_buf_init(buffer, sizeof(buffer));`
`@@ -2615,7 +2626,7 @@ static void ReadFileUtf8(const FunctionCallbackInfo<Value>& args) {`
`2615`	`2626`	`if (req.result < 0) {`
`2616`	`2627`	`FS_SYNC_TRACE_END(read);`
`2617`	`2628`	`// req will be cleaned up by scope leave.`
`2618`		`- return env->ThrowUVException(req.result, "read", nullptr, path.out());`
	`2629`	`+ return env->ThrowUVException(req.result, "read", nullptr);`
`2619`	`2630`	`}`
`2620`	`2631`	`if (r <= 0) {`
`2621`	`2632`	`break;`

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.