fs: add a fast-path for readFileSync utf-8

PR-URL: #48658
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>

benchmark/fs/readFileSync.js

@@ -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);
 }

lib/fs.js

@@ -142,6 +142,7 @@ const {
   validateObject,
   validateString,
 } = require('internal/validators');
+const { readFileSyncUtf8 } = require('internal/fs/read/utf8');
 
 let truncateWarn = true;
 let fs;
@@ -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/context');
   const context = new ReadFileContext(callback, options.encoding);
   context.isUserFd = isFd(path); // File descriptor ownership
 
@@ -457,7 +458,15 @@ function tryReadSync(fd, isUserFd, buffer, pos, len) {
  */
 function readFileSync(path, options) {
   options = getOptions(options, { flag: 'r' });
+
   const isUserFd = isFd(path); // File descriptor ownership
+
+  // TODO(@anonrig): Do not handle file descriptor ownership for now.
+  if (!isUserFd && (options.encoding === 'utf8' || options.encoding === 'utf-8')) {
+    path = getValidatedPath(path);
+    return readFileSyncUtf8(pathModule.toNamespacedPath(path), stringToFlags(options.flag));
+  }
+
   const fd = isUserFd ? path : fs.openSync(path, options.flag, 0o666);
 
   const stats = tryStatSync(fd, isUserFd);

lib/internal/fs/read/utf8.js

@@ -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,
+};

src/node_file.cc

@@ -1959,6 +1959,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, -1, 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);
 
@@ -3114,6 +3174,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);
@@ -3231,6 +3292,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);

test/parallel/test-bootstrap-modules.js

@@ -74,6 +74,7 @@ const expectedModules = new Set([
   'NativeModule internal/webstreams/queuingstrategies',
   'NativeModule internal/blob',
   'NativeModule internal/fs/utils',
+  'NativeModule internal/fs/read/utf8',
   'NativeModule fs',
   'Internal Binding options',
   'NativeModule internal/options',