[v14.x backport] fs: improve fsPromises readFile performance

lib/fs.js

@@ -24,10 +24,6 @@
 
 'use strict';
 
-// Most platforms don't allow reads or writes >= 2 GB.
-// See https://github.com/libuv/libuv/pull/1501.
-const kIoMaxLength = 2 ** 31 - 1;
-
 // When using FSReqCallback, make sure to create the object only *after* all
 // parameter validation has happened, so that the objects are not kept in memory
 // in case they are created but never used due to an exception.
@@ -79,6 +75,10 @@ const { FSReqCallback, statValues } = binding;
 const { toPathIfFileURL } = require('internal/url');
 const internalUtil = require('internal/util');
 const {
+  constants: {
+    kIoMaxLength,
+    kMaxUserId,
+  },
   copyObject,
   Dirent,
   getDirents,
@@ -121,8 +121,6 @@ const {
   validateInteger,
   validateInt32
 } = require('internal/validators');
-// 2 ** 32 - 1
-const kMaxUserId = 4294967295;
 
 let truncateWarn = true;
 let fs;

lib/internal/fs/promises.js

@@ -1,18 +1,9 @@
 'use strict';
 
-// Most platforms don't allow reads or writes >= 2 GB.
-// See https://github.com/libuv/libuv/pull/1501.
-const kIoMaxLength = 2 ** 31 - 1;
-
-// Note: This is different from kReadFileBufferLength used for non-promisified
-// fs.readFile.
-const kReadFileMaxChunkSize = 2 ** 14;
 const kWriteFileMaxChunkSize = 2 ** 14;
 
-// 2 ** 32 - 1
-const kMaxUserId = 4294967295;
-
 const {
+  ArrayPrototypePush,
   Error,
   MathMax,
   MathMin,
@@ -44,6 +35,12 @@ const {
 const { isArrayBufferView } = require('internal/util/types');
 const { rimrafPromises } = require('internal/fs/rimraf');
 const {
+  constants: {
+    kIoMaxLength,
+    kMaxUserId,
+    kReadFileBufferLength,
+    kReadFileUnknownBufferLength,
+  },
   copyObject,
   getDirents,
   getOptions,
@@ -296,24 +293,46 @@ async function readFileHandle(filehandle, options) {
   if (size > kIoMaxLength)
     throw new ERR_FS_FILE_TOO_LARGE(size);
 
-  const chunks = [];
-  const chunkSize = size === 0 ?
-    kReadFileMaxChunkSize :
-    MathMin(size, kReadFileMaxChunkSize);
   let endOfFile = false;
+  let totalRead = 0;
+  const noSize = size === 0;
+  const buffers = [];
+  const fullBuffer = noSize ? undefined : Buffer.allocUnsafeSlow(size);
   do {
     if (signal && signal.aborted) {
       throw lazyDOMException('The operation was aborted', 'AbortError');
     }
-    const buf = Buffer.alloc(chunkSize);
-    const { bytesRead, buffer } =
-      await read(filehandle, buf, 0, chunkSize, -1);
-    endOfFile = bytesRead === 0;
-    if (bytesRead > 0)
-      chunks.push(buffer.slice(0, bytesRead));
+    let buffer;
+    let offset;
+    let length;
+    if (noSize) {
+      buffer = Buffer.allocUnsafeSlow(kReadFileUnknownBufferLength);
+      offset = 0;
+      length = kReadFileUnknownBufferLength;
+    } else {
+      buffer = fullBuffer;
+      offset = totalRead;
+      length = MathMin(size - totalRead, kReadFileBufferLength);
+    }
+
+    const bytesRead = (await binding.read(filehandle.fd, buffer, offset,
+                                          length, -1, kUsePromises)) || 0;
+    totalRead += bytesRead;
+    endOfFile = bytesRead === 0 || totalRead === size;
+    if (noSize && bytesRead > 0) {
+      const isBufferFull = bytesRead === kReadFileUnknownBufferLength;
+      const chunkBuffer = isBufferFull ? buffer : buffer.slice(0, bytesRead);
+      ArrayPrototypePush(buffers, chunkBuffer);
+    }
   } while (!endOfFile);
 
-  const result = chunks.length === 1 ? chunks[0] : Buffer.concat(chunks);
+  let result;
+  if (size > 0) {
+    result = totalRead === size ? fullBuffer : fullBuffer.slice(0, totalRead);
+  } else {
+    result = buffers.length === 1 ? buffers[0] : Buffer.concat(buffers,
+                                                               totalRead);
+  }
 
   return options.encoding ? result.toString(options.encoding) : result;
 }

lib/internal/fs/read_file_context.js

@@ -4,6 +4,13 @@ const {
   MathMin,
 } = primordials;
 
+const {
+  constants: {
+    kReadFileBufferLength,
+    kReadFileUnknownBufferLength,
+  }
+} = require('internal/fs/utils');
+
 const { Buffer } = require('buffer');
 
 const { FSReqCallback, close, read } = internalBinding('fs');
@@ -18,15 +25,6 @@ const lazyDOMException = hideStackFrames((message, name) => {
   return new DOMException(message, name);
 });
 
-// Use 64kb in case the file type is not a regular file and thus do not know the
-// actual file size. Increasing the value further results in more frequent over
-// allocation for small files and consumes CPU time and memory that should be
-// used else wise.
-// Use up to 512kb per read otherwise to partition reading big files to prevent
-// blocking other threads in case the available threads are all in use.
-const kReadFileUnknownBufferLength = 64 * 1024;
-const kReadFileBufferLength = 512 * 1024;
-
 function readFileAfterRead(err, bytesRead) {
   const context = this.context;
 

lib/internal/fs/utils.js

@@ -113,6 +113,21 @@ const kMaximumCopyMode = COPYFILE_EXCL |
                          COPYFILE_FICLONE |
                          COPYFILE_FICLONE_FORCE;
 
+// Most platforms don't allow reads or writes >= 2 GB.
+// See https://github.com/libuv/libuv/pull/1501.
+const kIoMaxLength = 2 ** 31 - 1;
+
+// Use 64kb in case the file type is not a regular file and thus do not know the
+// actual file size. Increasing the value further results in more frequent over
+// allocation for small files and consumes CPU time and memory that should be
+// used else wise.
+// Use up to 512kb per read otherwise to partition reading big files to prevent
+// blocking other threads in case the available threads are all in use.
+const kReadFileUnknownBufferLength = 64 * 1024;
+const kReadFileBufferLength = 512 * 1024;
+
+const kMaxUserId = 2 ** 32 - 1;
+
 const isWindows = process.platform === 'win32';
 
 let fs;
@@ -815,6 +830,12 @@ const validatePosition = hideStackFrames((position, name) => {
 });
 
 module.exports = {
+  constants: {
+    kIoMaxLength,
+    kMaxUserId,
+    kReadFileBufferLength,
+    kReadFileUnknownBufferLength,
+  },
   assertEncoding,
   BigIntStats,  // for testing
   copyObject,

test/parallel/test-fs-promises-file-handle-readFile.js

@@ -11,7 +11,7 @@ const {
   open,
   readFile,
   writeFile,
-  truncate
+  truncate,
 } = fs.promises;
 const path = require('path');
 const tmpdir = require('../common/tmpdir');
@@ -65,6 +65,7 @@ async function doReadAndCancel() {
     await assert.rejects(readFile(fileHandle, { signal }), {
       name: 'AbortError'
     });
+    await fileHandle.close();
   }
 
   // Signal aborted on first tick
@@ -75,10 +76,11 @@ async function doReadAndCancel() {
     fs.writeFileSync(filePathForHandle, buffer);
     const controller = new AbortController();
     const { signal } = controller;
-    tick(1, () => controller.abort());
+    process.nextTick(() => controller.abort());
     await assert.rejects(readFile(fileHandle, { signal }), {
       name: 'AbortError'
-    });
+    }, 'tick-0');
+    await fileHandle.close();
   }
 
   // Signal aborted right before buffer read
@@ -91,10 +93,12 @@ async function doReadAndCancel() {
 
     const controller = new AbortController();
     const { signal } = controller;
-    tick(2, () => controller.abort());
+    tick(1, () => controller.abort());
     await assert.rejects(fileHandle.readFile({ signal, encoding: 'utf8' }), {
       name: 'AbortError'
-    });
+    }, 'tick-1');
+
+    await fileHandle.close();
   }
 
   // Validate file size is within range for reading
@@ -112,6 +116,7 @@ async function doReadAndCancel() {
       name: 'RangeError',
       code: 'ERR_FS_FILE_TOO_LARGE'
     });
+    await fileHandle.close();
   }
 }