fs: use sized reads for large files in readFileUtf8

fs.readFileSync(path, 'utf8') read the whole file in 8 KiB read() calls
appended to a std::string, i.e. one syscall and a potential
reallocation per 8 KiB (an 8 MiB file took ~1400 read() calls).

Keep the exact old sequence for small files (one read into the 8 KiB
stack buffer, one read reporting EOF). Once a read fills the stack
buffer, read the rest directly into one heap buffer sized from fstat()
(plus one byte so that the EOF read does not force growth), growing
geometrically only when the size is unavailable or wrong. The size is
only an allocation hint: reading continues until read() reports EOF,
so procfs/sysfs files, FIFOs, files that change while being read and
file descriptors positioned mid-file behave as before, and the bytes
handed to StringBytes::Encode() are exactly the ones read.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

Author: codebytere

src/node_file.cc

@@ -2924,10 +2924,30 @@ static void ReadFileUtf8(const FunctionCallbackInfo<Value>& args) {
     uv_fs_req_cleanup(&req);
   });
 
+  // Small files (the common case, e.g. module sources) are read exactly as
+  // before: read() into an 8 KiB stack buffer until one reports EOF. Once a
+  // read fills the whole stack buffer the file is evidently larger; the rest
+  // is then read directly into one heap buffer -- first a 64 KiB step (so
+  // files up to that size still take exactly the reads they took before, and
+  // no fstat()), then, if that fills up too, sized from fstat() (falling back
+  // to geometric growth) -- instead of appending 8 KiB per syscall to a
+  // repeatedly reallocated std::string. The fstat() size is only a hint for
+  // the allocation: reading continues until read() reports EOF, so files
+  // whose size is misreported (procfs) or that change concurrently behave as
+  // before, and the bytes handed to StringBytes::Encode() are exactly the
+  // ones read.
   std::string result{};
   char buffer[8192];
   uv_buf_t buf = uv_buf_init(buffer, sizeof(buffer));
 
+  char* big = nullptr;
+  size_t big_len = 0;
+  size_t big_cap = 0;
+  bool sized = false;
+  auto free_big = OnScopeLeave([&big]() { free(big); });
+  constexpr size_t kMinChunk = 64 * 1024;
+  constexpr size_t kMaxChunk = 8 * 1024 * 1024;
+
   FS_SYNC_TRACE_BEGIN(read);
   while (true) {
     auto r = uv_fs_read(nullptr, &req, file, &buf, 1, -1, nullptr);
@@ -2940,12 +2960,63 @@ static void ReadFileUtf8(const FunctionCallbackInfo<Value>& args) {
     if (r <= 0) {
       break;
     }
-    result.append(buf.base, r);
+    if (big == nullptr) {
+      result.append(buf.base, r);
+      if (static_cast<size_t>(r) < sizeof(buffer)) {
+        continue;
+      }
+      // Switch to the heap buffer.
+      uv_fs_req_cleanup(&req);
+      big_cap = kMinChunk;
+      big = UncheckedMalloc<char>(big_cap);
+      if (big == nullptr) {
+        FS_SYNC_TRACE_END(read);
+        return THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
+      }
+      memcpy(big, result.data(), result.size());
+      big_len = result.size();
+      result = std::string();
+    } else {
+      big_len += static_cast<size_t>(r);
+    }
+    if (big_len == big_cap) {
+      // Full: grow. The first time, size the buffer from fstat() when that
+      // looks trustworthy (+1 leaves room for the EOF-reporting read()).
+      size_t new_cap =
+          big_cap + std::min(kMaxChunk, std::max(kMinChunk, big_cap));
+      if (!sized) {
+        sized = true;
+        uv_fs_req_cleanup(&req);
+        uv_fs_t stat_req;
+        if (uv_fs_fstat(nullptr, &stat_req, file, nullptr) == 0) {
+          const uv_stat_t* const st =
+              static_cast<const uv_stat_t*>(stat_req.ptr);
+          if ((st->st_mode & S_IFMT) == S_IFREG &&
+              static_cast<uint64_t>(st->st_size) > big_len &&
+              static_cast<uint64_t>(st->st_size) <
+                  static_cast<uint64_t>(v8::String::kMaxLength)) {
+            new_cap = static_cast<size_t>(st->st_size) + 1;
+          }
+        }
+        uv_fs_req_cleanup(&stat_req);
+      }
+      char* const grown = UncheckedRealloc<char>(big, new_cap);
+      if (grown == nullptr) {
+        FS_SYNC_TRACE_END(read);
+        return THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
+      }
+      big = grown;
+      big_cap = new_cap;
+    }
+    buf = uv_buf_init(big + big_len, std::min(kMaxChunk, big_cap - big_len));
   }
   FS_SYNC_TRACE_END(read);
 
   Local<Value> val;
-  if (!ToV8Value(env->context(), result, isolate).ToLocal(&val)) {
+  const std::string_view content = big != nullptr
+                                       ? std::string_view(big, big_len)
+                                       : std::string_view(result);
+  if (!ToV8Value(env->context(), content, isolate).ToLocal(&val)) {
     return;
   }
 

test/parallel/test-fs-readfilesync-utf8-sizes.js

@@ -0,0 +1,88 @@
+'use strict';
+// fs.readFileSync(path, 'utf8') takes a dedicated native path. Its result must
+// equal fs.readFileSync(path).toString('utf8') for every file size (in
+// particular around its internal 8 KiB stack buffer and for multi-megabyte
+// files), for file descriptors positioned mid-file, and for files whose
+// reported size is wrong (procfs reports 0, sysfs reports a page).
+const common = require('../common');
+const tmpdir = require('../common/tmpdir');
+const assert = require('assert');
+const fs = require('fs');
+
+tmpdir.refresh();
+
+function content(size) {
+  // Multi-byte characters straddling every possible chunk boundary.
+  const unit = 'abcdé€\u{1F600}\n';
+  let s = unit.repeat(Math.ceil(size / unit.length));
+  s = s.slice(0, size);
+  // Avoid ending on a lone surrogate produced by slice().
+  if (/[\ud800-\udbff]$/.test(s)) s = s.slice(0, -1) + 'x';
+  return s;
+}
+
+const sizes = [0, 1, 8190, 8191, 8192, 8193, 8194, 16383, 16384, 16385,
+               65535, 65536, 65537, 100000, (1 << 20) - 1, 1 << 20, (1 << 20) + 1,
+               (8 << 20) + 5];
+for (const size of sizes) {
+  const file = tmpdir.resolve(`f-${size}.txt`);
+  const str = content(size);
+  fs.writeFileSync(file, str);
+  const expected = fs.readFileSync(file).toString('utf8');
+  assert.strictEqual(fs.readFileSync(file, 'utf8'), expected, `size ${size} by path`);
+  assert.strictEqual(fs.readFileSync(file, { encoding: 'utf-8' }), expected, `size ${size} utf-8 alias`);
+  // By fd: from the start (leaves the fd at EOF), then at EOF, then from a
+  // mid-file position on a fresh fd.
+  let fd = fs.openSync(file, 'r');
+  try {
+    assert.strictEqual(fs.readFileSync(fd, 'utf8'), expected, `size ${size} by fd`);
+    assert.strictEqual(fs.readFileSync(fd, 'utf8'), '', `size ${size} by fd at EOF`);
+  } finally {
+    fs.closeSync(fd);
+  }
+  if (size > 10) {
+    fd = fs.openSync(file, 'r');
+    try {
+      // Advance the fd 3 bytes (inside the ASCII prefix, so still valid UTF-8).
+      assert.strictEqual(fs.readSync(fd, Buffer.alloc(3), 0, 3, null), 3);
+      assert.strictEqual(fs.readFileSync(fd, 'utf8'), Buffer.from(expected).subarray(3).toString('utf8'),
+                         `size ${size} by fd at offset 3`);
+    } finally {
+      fs.closeSync(fd);
+    }
+  }
+}
+
+// Binary garbage is decoded with replacement characters identically.
+{
+  const file = tmpdir.resolve('binary.bin');
+  const buf = Buffer.alloc(20000);
+  for (let i = 0; i < buf.length; i++) buf[i] = (i * 7919) & 0xff;
+  fs.writeFileSync(file, buf);
+  assert.strictEqual(fs.readFileSync(file, 'utf8'), buf.toString('utf8'));
+}
+
+// Files whose st_size does not describe their content.
+if (common.isLinux) {
+  for (const file of ['/proc/self/status', '/proc/self/maps', '/proc/cpuinfo',
+                      '/proc/version', '/sys/kernel/mm/transparent_hugepage/enabled']) {
+    let viaBuffer;
+    try {
+      viaBuffer = fs.readFileSync(file);
+    } catch {
+      continue;  // Not available in this environment.
+    }
+    const viaUtf8 = fs.readFileSync(file, 'utf8');
+    if (file !== '/proc/version' && file.startsWith('/proc/')) {
+      // Content legitimately differs between two reads; compare shape instead.
+      assert.ok(viaUtf8.length > 0);
+      assert.strictEqual(viaUtf8.split('\n').length > 5, true, file);
+      if (file === '/proc/self/maps') assert.ok(viaUtf8.length > 8192, 'maps should exceed one stack buffer');
+    } else {
+      assert.strictEqual(viaUtf8, viaBuffer.toString('utf8'), file);
+    }
+  }
+}
+
+// Directory: same error either way.
+assert.throws(() => fs.readFileSync(tmpdir.path, 'utf8'), { code: 'EISDIR' });