FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: improve `readFileSync` with file descriptors · nodejs/node@07d0518 · GitHub

/ node Public

Commit 07d0518

Browse files
anonrig
authored and
ruyadorno
committed
fs: improve readFileSync with file descriptors
PR-URL: #49691 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent d2bcdcb commit 07d0518

4 files changed

Lines changed: 50 additions & 27 deletions

File tree

‎benchmark/fs/readFileSync.js‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,21 @@ const fs = require('fs');
66
const bench = common.createBenchmark(main, {
77
encoding: ['undefined', 'utf8'],
88
path: ['existing', 'non-existing'],
9-
n: [60e1],
9+
hasFileDescriptor: ['true', 'false'],
10+
n: [1e4],
1011
});
1112

12-
function main({ n, encoding, path }) {
13+
function main({ n, encoding, path, hasFileDescriptor }) {
1314
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+
}
1524
bench.start();
1625
for (let i = 0; i < n; ++i) {
1726
try {
@@ -21,4 +30,7 @@ function main({ n, encoding, path }) {
2130
}
2231
}
2332
bench.end(n);
33+
if (shouldClose) {
34+
fs.closeSync(file);
35+
}
2436
}

‎lib/fs.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -437,13 +437,11 @@ function tryReadSync(fd, isUserFd, buffer, pos, len) {
437437
function readFileSync(path, options) {
438438
options = getOptions(options, { flag: 'r' });
439439

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') {
444441
return syncFs.readFileUtf8(path, options.flag);
445442
}
446443

444+
const isUserFd = isFd(path); // File descriptor ownership
447445
const fd = isUserFd ? path : fs.openSync(path, options.flag, 0o666);
448446

449447
const stats = tryStatSync(fd, isUserFd);

‎lib/internal/fs/sync.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const {
99
getStatFsFromBinding,
1010
getValidatedFd,
1111
} = require('internal/fs/utils');
12-
const { parseFileMode } = require('internal/validators');
12+
const { parseFileMode, isInt32 } = require('internal/validators');
1313

1414
const binding = internalBinding('fs');
1515

@@ -19,7 +19,9 @@ const binding = internalBinding('fs');
1919
* @return {string}
2020
*/
2121
function readFileUtf8(path, flag) {
22-
path = pathModule.toNamespacedPath(getValidatedPath(path));
22+
if (!isInt32(path)) {
23+
path = pathModule.toNamespacedPath(getValidatedPath(path));
24+
}
2325
return binding.readFileUtf8(path, stringToFlags(flag));
2426
}
2527

‎src/node_file.cc‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2190,7 +2190,7 @@ static void OpenSync(const FunctionCallbackInfo<Value>& args) {
21902190
uv_fs_t req;
21912191
auto make = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
21922192
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);
21942194
FS_SYNC_TRACE_END(open);
21952195
if (err < 0) {
21962196
return env->ThrowUVException(err, "open", nullptr, path.out());
@@ -2581,30 +2581,41 @@ static void ReadFileUtf8(const FunctionCallbackInfo<Value>& args) {
25812581

25822582
CHECK_GE(args.Length(), 2);
25832583

2584-
BufferValue path(env->isolate(), args[0]);
2585-
CHECK_NOT_NULL(*path);
2586-
25872584
CHECK(args[1]->IsInt32());
25882585
const int flags = args[1].As<Int32>()->Value();
25892586

2590-
if (CheckOpenPermissions(env, path, flags).IsNothing()) return;
2591-
2587+
uv_file file;
25922588
uv_fs_t req;
2593-
auto defer_req_cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
25942589

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+
}
26012608
}
26022609

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);
26072617
});
2618+
26082619
std::string result{};
26092620
char buffer[8192];
26102621
uv_buf_t buf = uv_buf_init(buffer, sizeof(buffer));
@@ -2615,7 +2626,7 @@ static void ReadFileUtf8(const FunctionCallbackInfo<Value>& args) {
26152626
if (req.result < 0) {
26162627
FS_SYNC_TRACE_END(read);
26172628
// 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);
26192630
}
26202631
if (r <= 0) {
26212632
break;

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL