benchmark: add dir and withFileTypes option readdir benchmarks

PR-URL: nodejs#24125 Refs: v8/v8@0483e9a Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Yang Guo <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
kiyomizumia · Nov 15, 2018 · e73ee88 · e73ee88
1 parent 6ad8d46
commit e73ee88
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 61 deletions.
diff --git a/benchmark/fs/bench-readdir.js b/benchmark/fs/bench-readdir.js
@@ -5,16 +5,19 @@ const fs = require('fs');
 const path = require('path');
 
 const bench = common.createBenchmark(main, {
-  n: [1e4],
+  n: [10],
+  dir: [ 'lib', 'test/parallel'],
+  withFileTypes: ['true', 'false']
 });
 
-
-function main({ n }) {
+function main({ n, dir, withFileTypes }) {
+  withFileTypes = withFileTypes === 'true';
+  const fullPath = path.resolve(__dirname, '../../', dir);
   bench.start();
   (function r(cntr) {
     if (cntr-- <= 0)
       return bench.end(n);
-    fs.readdir(path.resolve(__dirname, '../../lib/'), function() {
+    fs.readdir(fullPath, { withFileTypes }, function() {
       r(cntr);
     });
   }(n));

diff --git a/benchmark/fs/bench-readdirSync.js b/benchmark/fs/bench-readdirSync.js
@@ -5,14 +5,18 @@ const fs = require('fs');
 const path = require('path');
 
 const bench = common.createBenchmark(main, {
-  n: [1e4],
+  n: [10],
+  dir: [ 'lib', 'test/parallel'],
+  withFileTypes: ['true', 'false']
 });
 
 
-function main({ n }) {
+function main({ n, dir, withFileTypes }) {
+  withFileTypes = withFileTypes === 'true';
+  const fullPath = path.resolve(__dirname, '../../', dir);
   bench.start();
   for (var i = 0; i < n; i++) {
-    fs.readdirSync(path.resolve(__dirname, '../../lib/'));
+    fs.readdirSync(fullPath, { withFileTypes });
   }
   bench.end(n);
 }
diff --git a/src/node_file.cc b/src/node_file.cc
@@ -571,7 +571,7 @@ void AfterScanDir(uv_fs_t* req) {
   Environment* env = req_wrap->env();
   Local<Value> error;
   int r;
-  std::vector<Local<Value>> name_argv;
+  std::vector<Local<Value>> name_v;
 
   for (int i = 0; ; i++) {
     uv_dirent_t ent;
@@ -593,12 +593,10 @@ void AfterScanDir(uv_fs_t* req) {
     if (filename.IsEmpty())
       return req_wrap->Reject(error);
 
-    name_argv.push_back(filename.ToLocalChecked());
+    name_v.push_back(filename.ToLocalChecked());
   }
 
-  Local<Array> names =
-      Array::New(env->isolate(), name_argv.data(), name_argv.size());
-  req_wrap->Resolve(names);
+  req_wrap->Resolve(Array::New(env->isolate(), name_v.data(), name_v.size()));
 }
 
 void AfterScanDirWithTypes(uv_fs_t* req) {
@@ -613,13 +611,9 @@ void AfterScanDirWithTypes(uv_fs_t* req) {
   Isolate* isolate = env->isolate();
   Local<Value> error;
   int r;
-  Local<Array> names = Array::New(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;
-  Local<Value> types = Array::New(isolate, 0);
-  Local<Value> type_argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
-  size_t type_idx = 0;
+
+  std::vector<Local<Value>> name_v;
+  std::vector<Local<Value>> type_v;
 
   for (int i = 0; ; i++) {
     uv_dirent_t ent;
@@ -641,48 +635,13 @@ void AfterScanDirWithTypes(uv_fs_t* req) {
     if (filename.IsEmpty())
       return req_wrap->Reject(error);
 
-    name_argv[name_idx++] = filename.ToLocalChecked();
-
-    if (name_idx >= arraysize(name_argv)) {
-      MaybeLocal<Value> ret = fn->Call(env->context(), names, name_idx,
-                                       name_argv);
-      if (ret.IsEmpty()) {
-        return;
-      }
-      name_idx = 0;
-    }
-
-    type_argv[type_idx++] = Integer::New(isolate, ent.type);
-
-    if (type_idx >= arraysize(type_argv)) {
-      MaybeLocal<Value> ret = fn->Call(env->context(), types, type_idx,
-          type_argv);
-      if (ret.IsEmpty()) {
-        return;
-      }
-      type_idx = 0;
-    }
-  }
-
-  if (name_idx > 0) {
-    MaybeLocal<Value> ret = fn->Call(env->context(), names, name_idx,
-        name_argv);
-    if (ret.IsEmpty()) {
-      return;
-    }
-  }
-
-  if (type_idx > 0) {
-    MaybeLocal<Value> ret = fn->Call(env->context(), types, type_idx,
-        type_argv);
-    if (ret.IsEmpty()) {
-      return;
-    }
+    name_v.push_back(filename.ToLocalChecked());
+    type_v.push_back(Integer::New(isolate, ent.type));
   }
 
   Local<Array> result = Array::New(isolate, 2);
-  result->Set(0, names);
-  result->Set(1, types);
+  result->Set(0, Array::New(isolate, name_v.data(), name_v.size()));
+  result->Set(1, Array::New(isolate, type_v.data(), type_v.size()));
   req_wrap->Resolve(result);
 }
 
@@ -1523,9 +1482,8 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
     Local<Array> names = Array::New(isolate, name_v.data(), name_v.size());
     if (with_types) {
       Local<Array> result = Array::New(isolate, 2);
-      Local<Value> types = Array::New(isolate, type_v.data(), type_v.size());
       result->Set(0, names);
-      result->Set(1, types);
+      result->Set(1, Array::New(isolate, type_v.data(), type_v.size()));
       args.GetReturnValue().Set(result);
     } else {
       args.GetReturnValue().Set(names);

diff --git a/test/parallel/test-benchmark-fs.js b/test/parallel/test-benchmark-fs.js
@@ -16,5 +16,7 @@ runBenchmark('fs', [
   'statType=fstat',
   'statSyncType=fstatSync',
   'encodingType=buf',
-  'filesize=1024'
+  'filesize=1024',
+  'dir=.github',
+  'withFileTypes=false'
 ], { NODE_TMPDIR: tmpdir.path, NODEJS_BENCHMARK_ZERO_ALLOWED: 1 });