From 65df2c678743dbe8614c99522d3929646029482f Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Wed, 20 Dec 2023 05:44:12 -0500 Subject: [PATCH] fs: improve mkdtemp performance for buffer prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/51078 Reviewed-By: Vinícius Lourenço Claro Cardoso Reviewed-By: Matteo Collina --- benchmark/fs/bench-mkdtempSync.js | 43 +++++++++++++++++++++++++++++++ lib/fs.js | 19 ++------------ src/node_file.cc | 5 ++++ 3 files changed, 50 insertions(+), 17 deletions(-) create mode 100644 benchmark/fs/bench-mkdtempSync.js diff --git a/benchmark/fs/bench-mkdtempSync.js b/benchmark/fs/bench-mkdtempSync.js new file mode 100644 index 00000000000000..afb342366bbfdf --- /dev/null +++ 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 index ecfc7c560a1a74..b2d00de6637304 100644 --- 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 index 3f3de19dcd704a..1a14b30e00778b 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -2846,6 +2846,11 @@ static void Mkdtemp(const FunctionCallbackInfo& 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());