fs: improve mkdtemp performance for buffer prefix

PR-URL: #51078 Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
nodejs · Jan 2, 2024 · 7421467 · 7421467
1 parent 5453abd
commit 7421467
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 17 deletions.
diff --git a/benchmark/fs/bench-mkdtempSync.js b/benchmark/fs/bench-mkdtempSync.js
@@ -0,0 +1,43 @@
+'use strict';
+
+const common = require('../common');
+const fs = require('fs');
+const assert = require('assert');
+const tmpdir = require('../../test/common/tmpdir');
+
+const bench = common.createBenchmark(main, {
+  type: ['valid-string', 'valid-buffer', 'invalid'],
+  n: [1e4],
+});
+
+function main({ n, type }) {
+  tmpdir.refresh();
+  const options = { encoding: 'utf8' };
+  let prefix;
+  let out = true;
+
+  switch (type) {
+    case 'valid-string':
+      prefix = tmpdir.resolve(`${Date.now()}`);
+      break;
+    case 'valid-buffer':
+      prefix = Buffer.from(tmpdir.resolve(`${Date.now()}`));
+      break;
+    case 'invalid':
+      prefix = tmpdir.resolve('non-existent', 'foo', 'bar');
+      break;
+    default:
+      new Error('Invalid type');
+  }
+
+  bench.start();
+  for (let i = 0; i < n; i++) {
+    try {
+      out = fs.mkdtempSync(prefix, options);
+    } catch {
+      // do nothing
+    }
+  }
+  bench.end(n);
+  assert.ok(out);
+}
diff --git a/lib/fs.js b/lib/fs.js
@@ -2933,16 +2933,9 @@ function mkdtemp(prefix, options, callback) {
   prefix = getValidatedPath(prefix, 'prefix');
   warnOnNonPortableTemplate(prefix);
 
-  let path;
-  if (typeof prefix === 'string') {
-    path = `${prefix}XXXXXX`;
-  } else {
-    path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);
-  }
-
   const req = new FSReqCallback();
   req.oncomplete = callback;
-  binding.mkdtemp(path, options.encoding, req);
+  binding.mkdtemp(prefix, options.encoding, req);
 }
 
 /**
@@ -2956,15 +2949,7 @@ function mkdtempSync(prefix, options) {
 
   prefix = getValidatedPath(prefix, 'prefix');
   warnOnNonPortableTemplate(prefix);
-
-  let path;
-  if (typeof prefix === 'string') {
-    path = `${prefix}XXXXXX`;
-  } else {
-    path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);
-  }
-
-  return binding.mkdtemp(path, options.encoding);
+  return binding.mkdtemp(prefix, options.encoding);
 }
 
 /**

diff --git a/src/node_file.cc b/src/node_file.cc
@@ -2749,6 +2749,11 @@ static void Mkdtemp(const FunctionCallbackInfo<Value>& args) {
   CHECK_GE(argc, 2);
 
   BufferValue tmpl(isolate, args[0]);
+  static constexpr const char* const suffix = "XXXXXX";
+  const auto length = tmpl.length();
+  tmpl.AllocateSufficientStorage(length + strlen(suffix));
+  snprintf(tmpl.out() + length, tmpl.length(), "%s", suffix);
+
   CHECK_NOT_NULL(*tmpl);
   THROW_IF_INSUFFICIENT_PERMISSIONS(
       env, permission::PermissionScope::kFileSystemWrite, tmpl.ToStringView());