Skip to content

Commit 7421467

Browse files
anonrigRafaelGSS
authored andcommitted
fs: improve mkdtemp performance for buffer prefix
PR-URL: #51078 Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 5453abd commit 7421467

File tree

3 files changed

+50
-17
lines changed

3 files changed

+50
-17
lines changed

benchmark/fs/bench-mkdtempSync.js

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const fs = require('fs');
5+
const assert = require('assert');
6+
const tmpdir = require('../../test/common/tmpdir');
7+
8+
const bench = common.createBenchmark(main, {
9+
type: ['valid-string', 'valid-buffer', 'invalid'],
10+
n: [1e4],
11+
});
12+
13+
function main({ n, type }) {
14+
tmpdir.refresh();
15+
const options = { encoding: 'utf8' };
16+
let prefix;
17+
let out = true;
18+
19+
switch (type) {
20+
case 'valid-string':
21+
prefix = tmpdir.resolve(`${Date.now()}`);
22+
break;
23+
case 'valid-buffer':
24+
prefix = Buffer.from(tmpdir.resolve(`${Date.now()}`));
25+
break;
26+
case 'invalid':
27+
prefix = tmpdir.resolve('non-existent', 'foo', 'bar');
28+
break;
29+
default:
30+
new Error('Invalid type');
31+
}
32+
33+
bench.start();
34+
for (let i = 0; i < n; i++) {
35+
try {
36+
out = fs.mkdtempSync(prefix, options);
37+
} catch {
38+
// do nothing
39+
}
40+
}
41+
bench.end(n);
42+
assert.ok(out);
43+
}

lib/fs.js

+2-17
Original file line numberDiff line numberDiff line change
@@ -2933,16 +2933,9 @@ function mkdtemp(prefix, options, callback) {
29332933
prefix = getValidatedPath(prefix, 'prefix');
29342934
warnOnNonPortableTemplate(prefix);
29352935

2936-
let path;
2937-
if (typeof prefix === 'string') {
2938-
path = `${prefix}XXXXXX`;
2939-
} else {
2940-
path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);
2941-
}
2942-
29432936
const req = new FSReqCallback();
29442937
req.oncomplete = callback;
2945-
binding.mkdtemp(path, options.encoding, req);
2938+
binding.mkdtemp(prefix, options.encoding, req);
29462939
}
29472940

29482941
/**
@@ -2956,15 +2949,7 @@ function mkdtempSync(prefix, options) {
29562949

29572950
prefix = getValidatedPath(prefix, 'prefix');
29582951
warnOnNonPortableTemplate(prefix);
2959-
2960-
let path;
2961-
if (typeof prefix === 'string') {
2962-
path = `${prefix}XXXXXX`;
2963-
} else {
2964-
path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);
2965-
}
2966-
2967-
return binding.mkdtemp(path, options.encoding);
2952+
return binding.mkdtemp(prefix, options.encoding);
29682953
}
29692954

29702955
/**

src/node_file.cc

+5
Original file line numberDiff line numberDiff line change
@@ -2749,6 +2749,11 @@ static void Mkdtemp(const FunctionCallbackInfo<Value>& args) {
27492749
CHECK_GE(argc, 2);
27502750

27512751
BufferValue tmpl(isolate, args[0]);
2752+
static constexpr const char* const suffix = "XXXXXX";
2753+
const auto length = tmpl.length();
2754+
tmpl.AllocateSufficientStorage(length + strlen(suffix));
2755+
snprintf(tmpl.out() + length, tmpl.length(), "%s", suffix);
2756+
27522757
CHECK_NOT_NULL(*tmpl);
27532758
THROW_IF_INSUFFICIENT_PERMISSIONS(
27542759
env, permission::PermissionScope::kFileSystemWrite, tmpl.ToStringView());

0 commit comments

Comments
 (0)