fs: use sized reads for large files in readFileUtf8

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