Skip to content

Commit

Permalink
fs: add a fast-path for readFileSync utf-8
Browse files Browse the repository at this point in the history
  • Loading branch information
anonrig committed Jul 5, 2023
1 parent 6fda81d commit a886056
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 5 deletions.
17 changes: 13 additions & 4 deletions benchmark/fs/readFileSync.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,21 @@ const common = require('../common.js');
const fs = require('fs');

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

function main({ n }) {
function main({ n, encoding, path }) {
const enc = encoding === 'undefined' ? undefined : encoding;
const file = path === 'existing' ? __filename : '/tmp/not-found';
bench.start();
for (let i = 0; i < n; ++i)
fs.readFileSync(__filename);
for (let i = 0; i < n; ++i) {
try {
fs.readFileSync(file, enc);
} catch {
// do nothing
}
}
bench.end(n);
}
10 changes: 9 additions & 1 deletion lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ const {
validateObject,
validateString,
} = require('internal/validators');
const { readFileSyncUtf8 } = require('internal/fs/read-file/utf8');

let truncateWarn = true;
let fs;
Expand Down Expand Up @@ -380,7 +381,7 @@ function checkAborted(signal, callback) {
function readFile(path, options, callback) {
callback = maybeCallback(callback || options);
options = getOptions(options, { flag: 'r' });
const ReadFileContext = require('internal/fs/read_file_context');
const ReadFileContext = require('internal/fs/read-file/context');
const context = new ReadFileContext(callback, options.encoding);
context.isUserFd = isFd(path); // File descriptor ownership

Expand Down Expand Up @@ -457,7 +458,14 @@ function tryReadSync(fd, isUserFd, buffer, pos, len) {
*/
function readFileSync(path, options) {
options = getOptions(options, { flag: 'r' });

const isUserFd = isFd(path); // File descriptor ownership

// TODO: Do not handle file descriptor ownership for now.
if (!isUserFd && options.encoding === 'utf8') {
return readFileSyncUtf8(getValidatedPath(path), stringToFlags(options.flag));
}

const fd = isUserFd ? path : fs.openSync(path, options.flag, 0o666);

const stats = tryStatSync(fd, isUserFd);
Expand Down
File renamed without changes.
24 changes: 24 additions & 0 deletions lib/internal/fs/read-file/utf8.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict';

const { handleErrorFromBinding } = require('internal/fs/utils');

const binding = internalBinding('fs');

/**
* @param {string} path
* @param {number} flag
* @return {string}
*/
function readFileSyncUtf8(path, flag) {
const response = binding.readFileSync(path, flag);

if (typeof response === 'string') {
return response;
}

handleErrorFromBinding({ errno: response, path });
}

module.exports = {
readFileSyncUtf8,
};
62 changes: 62 additions & 0 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1994,6 +1994,66 @@ static inline Maybe<void> CheckOpenPermissions(Environment* env,
return JustVoid();
}

static void ReadFileSync(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

CHECK_GE(args.Length(), 2);

BufferValue path(env->isolate(), args[0]);
CHECK_NOT_NULL(*path);

CHECK(args[1]->IsInt32());
const int flags = args[1].As<Int32>()->Value();

if (CheckOpenPermissions(env, path, flags).IsNothing()) return;

uv_fs_t req;
auto defer_req_cleanup = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });

FS_SYNC_TRACE_BEGIN(open);
uv_file file = uv_fs_open(nullptr, &req, *path, flags, 438, nullptr);
FS_SYNC_TRACE_END(open);
if (req.result < 0) {
// req will be cleaned up by scope leave.
return args.GetReturnValue().Set(
v8::Integer::New(env->isolate(), req.result));
}
uv_fs_req_cleanup(&req);

auto defer_close = OnScopeLeave([file]() {
uv_fs_t close_req;
CHECK_EQ(0, uv_fs_close(nullptr, &close_req, file, nullptr));
uv_fs_req_cleanup(&close_req);
});

std::string result{};
char buffer[8192];
uv_buf_t buf = uv_buf_init(buffer, sizeof(buffer));

FS_SYNC_TRACE_BEGIN(read);
while (true) {
auto r = uv_fs_read(nullptr, &req, file, &buf, 1, result.length(), nullptr);
if (req.result < 0) {
FS_SYNC_TRACE_END(read);
// req will be cleaned up by scope leave.
return args.GetReturnValue().Set(
v8::Integer::New(env->isolate(), req.result));
}
uv_fs_req_cleanup(&req);
if (r <= 0) {
break;
}
result.append(buf.base, r);
}
FS_SYNC_TRACE_END(read);

args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
result.data(),
v8::NewStringType::kNormal,
result.size())
.ToLocalChecked());
}

static void Open(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Expand Down Expand Up @@ -3149,6 +3209,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
SetMethod(isolate, target, "stat", Stat);
SetMethod(isolate, target, "lstat", LStat);
SetMethod(isolate, target, "fstat", FStat);
SetMethodNoSideEffect(isolate, target, "readFileSync", ReadFileSync);
SetMethod(isolate, target, "statfs", StatFs);
SetMethod(isolate, target, "link", Link);
SetMethod(isolate, target, "symlink", Symlink);
Expand Down Expand Up @@ -3266,6 +3327,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(Stat);
registry->Register(LStat);
registry->Register(FStat);
registry->Register(ReadFileSync);
registry->Register(StatFs);
registry->Register(Link);
registry->Register(Symlink);
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-bootstrap-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ const expectedModules = new Set([
'NativeModule internal/webstreams/queuingstrategies',
'NativeModule internal/blob',
'NativeModule internal/fs/utils',
'NativeModule internal/fs/read-file/utf8',
'NativeModule fs',
'Internal Binding options',
'NativeModule internal/options',
Expand Down

0 comments on commit a886056

Please sign in to comment.