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: nodejs#3780 Reviewed-By: Fedor Indutny <[email protected]>
leak4mk0 · Dec 18, 2015 · 0fde28c · 0fde28c
1 parent d39ace1
commit 0fde28c
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 6 deletions.
diff --git a/benchmark/fs/bench-readdir.js 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
@@ -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
@@ -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);