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

Author: anonrig

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-file/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-file/context');
   const context = new ReadFileContext(callback, options.encoding);
   context.isUserFd = isFd(path); // File descriptor ownership
 
@@ -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(path);
+  }
+
   const fd = isUserFd ? path : fs.openSync(path, options.flag, 0o666);
 
   const stats = tryStatSync(fd, isUserFd);

lib/internal/fs/read_file_context.js lib/internal/fs/read-file/context.jslib/internal/fs/read_file_context.js renamed to lib/internal/fs/read-file/context.js

@@ -0,0 +1,24 @@
+'use strict';
+
+const { handleErrorFromBinding } = require('internal/fs/utils');
+const { toPathIfFileURL } = require('internal/url');
+
+const binding = internalBinding('fs');
+
+/**
+ * @param {string | URL} path
+ * @return {string}
+ */
+function readFileSyncUtf8(path) {
+  const response = binding.readFileSync(toPathIfFileURL(path));
+
+  if (typeof response === 'string') {
+    return response;
+  }
+
+  handleErrorFromBinding({ errno: response });
+}
+
+module.exports = {
+  readFileSyncUtf8,
+};

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

@@ -1963,6 +1963,63 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
+static void ReadFileSync(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+
+  CHECK_GE(args.Length(), 1);
+
+  BufferValue path(env->isolate(), args[0]);
+  CHECK_NOT_NULL(*path);
+
+  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, O_RDONLY, 0, 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[4096];
+  uv_buf_t buf = uv_buf_init(buffer, sizeof(buffer));
+  size_t read_characters;
+
+  FS_SYNC_TRACE_BEGIN(read);
+  while (true) {
+    read_characters =
+        uv_fs_read(nullptr, &req, file, &buf, 1, result.size(), 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 (read_characters <= 0) {
+      break;
+    }
+    result.append(buf.base, read_characters);
+  }
+  FS_SYNC_TRACE_END(read);
+
+  args.GetReturnValue().Set(String::NewFromUtf8(env->isolate(),
+                                                result.data(),
+                                                v8::NewStringType::kNormal,
+                                                result.size())
+                                .ToLocalChecked());
+}
+
 static inline Maybe<void> CheckOpenPermissions(Environment* env,
                                                const BufferValue& path,
                                                int flags) {
@@ -3149,6 +3206,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);
@@ -3266,6 +3324,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);

src/node_file.cc

@@ -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',