From e74242275708c0eb5c430b44235ab4062f973c2c Mon Sep 17 00:00:00 2001
From: Trevor Norris <trev.norris@gmail.com>
Date: Tue, 10 Nov 2015 16:36:50 -0700
Subject: [PATCH] fs: use pushValueToArray for readdir(Sync)

Improve performance by pushing directory entries to returned array in
batches of 8 using pushValueToArray() in JS. Add benchmarks to
demonstrate this improvement.

PR-URL: https://github.com/nodejs/node/pull/3780
Reviewed-By: Fedor Indutny <fedor@indutny.com>
---
 benchmark/fs/bench-readdir.js     | 22 ++++++++++++++++++
 benchmark/fs/bench-readdirSync.js | 19 ++++++++++++++++
 src/node_file.cc                  | 37 ++++++++++++++++++++++++++-----
 3 files changed, 72 insertions(+), 6 deletions(-)
 create mode 100644 benchmark/fs/bench-readdir.js
 create mode 100644 benchmark/fs/bench-readdirSync.js

diff --git a/benchmark/fs/bench-readdir.js b/benchmark/fs/bench-readdir.js
new file mode 100644
index 0000000000..2f0eab6a82
--- /dev/null
+++ b/benchmark/fs/bench-readdir.js
@@ -0,0 +1,22 @@
+'use strict';
+
+const common = require('../common');
+const fs = require('fs');
+
+const bench = common.createBenchmark(main, {
+  n: [1e4],
+});
+
+
+function main(conf) {
+  const n = conf.n >>> 0;
+
+  bench.start();
+  (function r(cntr) {
+    if (--cntr <= 0)
+      return bench.end(n);
+    fs.readdir(__dirname + '/../../lib/', function() {
+      r(cntr);
+    });
+  }(n));
+}
diff --git a/benchmark/fs/bench-readdirSync.js b/benchmark/fs/bench-readdirSync.js
new file mode 100644
index 0000000000..9f89649138
--- /dev/null
+++ b/benchmark/fs/bench-readdirSync.js
@@ -0,0 +1,19 @@
+'use strict';
+
+const common = require('../common');
+const fs = require('fs');
+
+const bench = common.createBenchmark(main, {
+  n: [1e4],
+});
+
+
+function main(conf) {
+  const n = conf.n >>> 0;
+
+  bench.start();
+  for (var i = 0; i < n; i++) {
+    fs.readdirSync(__dirname + '/../../lib/');
+  }
+  bench.end(n);
+}
diff --git a/src/node_file.cc b/src/node_file.cc
index b6ef7d5b78..5c1b39864c 100644
--- a/src/node_file.cc
+++ b/src/node_file.cc
@@ -214,6 +214,9 @@ static void After(uv_fs_t *req) {
         {
           int r;
           Local<Array> names = Array::New(env->isolate(), 0);
+          Local<Function> fn = env->push_values_to_array_function();
+          Local<Value> name_argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
+          size_t name_idx = 0;
 
           for (int i = 0; ; i++) {
             uv_dirent_t ent;
@@ -229,9 +232,19 @@ static void After(uv_fs_t *req) {
               break;
             }
 
-            Local<String> name = String::NewFromUtf8(env->isolate(),
-                                                     ent.name);
-            names->Set(i, name);
+            name_argv[name_idx++] =
+                String::NewFromUtf8(env->isolate(), ent.name);
+
+            if (name_idx >= ARRAY_SIZE(name_argv)) {
+              fn->Call(env->context(), names, name_idx, name_argv)
+                  .ToLocalChecked();
+              name_idx = 0;
+            }
+          }
+
+          if (name_idx > 0) {
+            fn->Call(env->context(), names, name_idx, name_argv)
+                .ToLocalChecked();
           }
 
           argv[1] = names;
@@ -811,6 +824,9 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
     CHECK_GE(SYNC_REQ.result, 0);
     int r;
     Local<Array> names = Array::New(env->isolate(), 0);
+    Local<Function> fn = env->push_values_to_array_function();
+    Local<Value> name_v[NODE_PUSH_VAL_TO_ARRAY_MAX];
+    size_t name_idx = 0;
 
     for (int i = 0; ; i++) {
       uv_dirent_t ent;
@@ -821,9 +837,18 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
       if (r != 0)
         return env->ThrowUVException(r, "readdir", "", *path);
 
-      Local<String> name = String::NewFromUtf8(env->isolate(),
-                                               ent.name);
-      names->Set(i, name);
+
+      name_v[name_idx++] = String::NewFromUtf8(env->isolate(), ent.name);
+
+      if (name_idx >= ARRAY_SIZE(name_v)) {
+        fn->Call(env->context(), names, name_idx, name_v)
+            .ToLocalChecked();
+        name_idx = 0;
+      }
+    }
+
+    if (name_idx > 0) {
+      fn->Call(env->context(), names, name_idx, name_v).ToLocalChecked();
     }
 
     args.GetReturnValue().Set(names);