Skip to content

Commit c82c250

Browse files
anonrigH4ad
authored andcommitted
fs: add a fast-path for readFileSync utf-8
Backport-PR-URL: nodejs#48658
1 parent 490fc00 commit c82c250

File tree

6 files changed

+108
-7
lines changed

6 files changed

+108
-7
lines changed

benchmark/fs/readFileSync.js

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,21 @@ const common = require('../common.js');
44
const fs = require('fs');
55

66
const bench = common.createBenchmark(main, {
7-
n: [60e4],
7+
encoding: ['undefined', 'utf8'],
8+
path: ['existing', 'non-existing'],
9+
n: [60e1],
810
});
911

10-
function main({ n }) {
12+
function main({ n, encoding, path }) {
13+
const enc = encoding === 'undefined' ? undefined : encoding;
14+
const file = path === 'existing' ? __filename : '/tmp/not-found';
1115
bench.start();
12-
for (let i = 0; i < n; ++i)
13-
fs.readFileSync(__filename);
16+
for (let i = 0; i < n; ++i) {
17+
try {
18+
fs.readFileSync(file, enc);
19+
} catch {
20+
// do nothing
21+
}
22+
}
1423
bench.end(n);
1524
}

lib/fs.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,9 @@ const {
145145
validateObject,
146146
validateString,
147147
} = require('internal/validators');
148+
const { readFileSyncUtf8 } = require('internal/fs/read-file/utf8');
148149

149150
const watchers = require('internal/fs/watchers');
150-
const ReadFileContext = require('internal/fs/read_file_context');
151151

152152
let truncateWarn = true;
153153
let fs;
@@ -391,6 +391,7 @@ function checkAborted(signal, callback) {
391391
function readFile(path, options, callback) {
392392
callback = maybeCallback(callback || options);
393393
options = getOptions(options, { flag: 'r' });
394+
const ReadFileContext = require('internal/fs/read-file/context');
394395
const context = new ReadFileContext(callback, options.encoding);
395396
context.isUserFd = isFd(path); // File descriptor ownership
396397

@@ -467,7 +468,14 @@ function tryReadSync(fd, isUserFd, buffer, pos, len) {
467468
*/
468469
function readFileSync(path, options) {
469470
options = getOptions(options, { flag: 'r' });
471+
470472
const isUserFd = isFd(path); // File descriptor ownership
473+
474+
// TODO: Do not handle file descriptor ownership for now.
475+
if (!isUserFd && options.encoding === 'utf8') {
476+
return readFileSyncUtf8(getValidatedPath(path), stringToFlags(options.flag));
477+
}
478+
471479
const fd = isUserFd ? path : fs.openSync(path, options.flag, 0o666);
472480

473481
const stats = tryStatSync(fd, isUserFd);

lib/internal/fs/read-file/utf8.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
'use strict';
2+
3+
const { handleErrorFromBinding } = require('internal/fs/utils');
4+
5+
const binding = internalBinding('fs');
6+
7+
/**
8+
* @param {string} path
9+
* @param {number} flag
10+
* @return {string}
11+
*/
12+
function readFileSyncUtf8(path, flag) {
13+
const response = binding.readFileSync(path, flag);
14+
15+
if (typeof response === 'string') {
16+
return response;
17+
}
18+
19+
handleErrorFromBinding({ errno: response, path });
20+
}
21+
22+
module.exports = {
23+
readFileSyncUtf8,
24+
};

src/node_file.cc

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1894,6 +1894,64 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
18941894
}
18951895
}
18961896

1897+
static void ReadFileSync(const FunctionCallbackInfo<Value>& args) {
1898+
Environment* env = Environment::GetCurrent(args);
1899+
1900+
CHECK_GE(args.Length(), 2);
1901+
1902+
BufferValue path(env->isolate(), args[0]);
1903+
CHECK_NOT_NULL(*path);
1904+
1905+
CHECK(args[1]->IsInt32());
1906+
const int flags = args[1].As<Int32>()->Value();
1907+
1908+
uv_fs_t req;
1909+
auto defer_req_cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
1910+
1911+
FS_SYNC_TRACE_BEGIN(open);
1912+
uv_file file = uv_fs_open(nullptr, &req, *path, flags, 438, nullptr);
1913+
FS_SYNC_TRACE_END(open);
1914+
if (req.result < 0) {
1915+
// req will be cleaned up by scope leave.
1916+
return args.GetReturnValue().Set(
1917+
v8::Integer::New(env->isolate(), req.result));
1918+
}
1919+
uv_fs_req_cleanup(&req);
1920+
1921+
auto defer_close = OnScopeLeave([file]() {
1922+
uv_fs_t close_req;
1923+
CHECK_EQ(0, uv_fs_close(nullptr, &close_req, file, nullptr));
1924+
uv_fs_req_cleanup(&close_req);
1925+
});
1926+
1927+
std::string result{};
1928+
char buffer[8192];
1929+
uv_buf_t buf = uv_buf_init(buffer, sizeof(buffer));
1930+
1931+
FS_SYNC_TRACE_BEGIN(read);
1932+
while (true) {
1933+
auto r = uv_fs_read(nullptr, &req, file, &buf, 1, result.length(), nullptr);
1934+
if (req.result < 0) {
1935+
FS_SYNC_TRACE_END(read);
1936+
// req will be cleaned up by scope leave.
1937+
return args.GetReturnValue().Set(
1938+
v8::Integer::New(env->isolate(), req.result));
1939+
}
1940+
uv_fs_req_cleanup(&req);
1941+
if (r <= 0) {
1942+
break;
1943+
}
1944+
result.append(buf.base, r);
1945+
}
1946+
FS_SYNC_TRACE_END(read);
1947+
1948+
args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
1949+
result.data(),
1950+
v8::NewStringType::kNormal,
1951+
result.size())
1952+
.ToLocalChecked());
1953+
}
1954+
18971955
static void Open(const FunctionCallbackInfo<Value>& args) {
18981956
Environment* env = Environment::GetCurrent(args);
18991957

@@ -2720,6 +2778,8 @@ void Initialize(Local<Object> target,
27202778
SetMethod(context, target, "realpath", RealPath);
27212779
SetMethod(context, target, "copyFile", CopyFile);
27222780

2781+
SetMethodNoSideEffect(context, target, "readFileSync", ReadFileSync);
2782+
27232783
SetMethod(context, target, "chmod", Chmod);
27242784
SetMethod(context, target, "fchmod", FChmod);
27252785

@@ -2730,7 +2790,6 @@ void Initialize(Local<Object> target,
27302790
SetMethod(context, target, "utimes", UTimes);
27312791
SetMethod(context, target, "futimes", FUTimes);
27322792
SetMethod(context, target, "lutimes", LUTimes);
2733-
27342793
SetMethod(context, target, "mkdtemp", Mkdtemp);
27352794

27362795
target
@@ -2826,6 +2885,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
28262885
registry->Register(Stat);
28272886
registry->Register(LStat);
28282887
registry->Register(FStat);
2888+
registry->Register(ReadFileSync);
28292889
registry->Register(StatFs);
28302890
registry->Register(Link);
28312891
registry->Register(Symlink);

test/parallel/test-bootstrap-modules.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ const expectedModules = new Set([
6464
'NativeModule internal/fixed_queue',
6565
'NativeModule internal/fs/dir',
6666
'NativeModule internal/fs/promises',
67-
'NativeModule internal/fs/read_file_context',
6867
'NativeModule internal/fs/rimraf',
6968
'NativeModule internal/fs/utils',
69+
'NativeModule internal/fs/read/utf8',
7070
'NativeModule internal/fs/watchers',
7171
'NativeModule internal/heap_utils',
7272
'NativeModule internal/histogram',

0 commit comments

Comments
 (0)