Skip to content

Commit 476e8d4

Browse files
committed
fs: improve error performance for fs.mkdtempSync
1 parent fa74158 commit 476e8d4

File tree

4 files changed

+96
-17
lines changed

4 files changed

+96
-17
lines changed

benchmark/fs/bench-mkdtempSync.js

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

lib/fs.js

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2895,23 +2895,7 @@ function mkdtemp(prefix, options, callback) {
28952895
* @returns {string}
28962896
*/
28972897
function mkdtempSync(prefix, options) {
2898-
options = getOptions(options);
2899-
2900-
prefix = getValidatedPath(prefix, 'prefix');
2901-
warnOnNonPortableTemplate(prefix);
2902-
2903-
let path;
2904-
if (typeof prefix === 'string') {
2905-
path = `${prefix}XXXXXX`;
2906-
} else {
2907-
path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);
2908-
}
2909-
2910-
const ctx = { path };
2911-
const result = binding.mkdtemp(path, options.encoding,
2912-
undefined, ctx);
2913-
handleErrorFromBinding(ctx);
2914-
return result;
2898+
return syncFs.mkdtemp(prefix, options);
29152899
}
29162900

29172901
/**

lib/internal/fs/sync.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@ const {
88
getStatsFromBinding,
99
getStatFsFromBinding,
1010
getValidatedFd,
11+
getOptions,
12+
warnOnNonPortableTemplate,
1113
} = require('internal/fs/utils');
1214
const { parseFileMode } = require('internal/validators');
15+
const { Buffer } = require('buffer');
1316

1417
const binding = internalBinding('fs');
1518

@@ -51,6 +54,25 @@ function copyFile(src, dest, mode) {
5154
);
5255
}
5356

57+
function mkdtemp(prefix, options) {
58+
options = getOptions(options);
59+
60+
prefix = getValidatedPath(prefix, 'prefix');
61+
warnOnNonPortableTemplate(prefix);
62+
63+
let path;
64+
if (typeof prefix === 'string') {
65+
path = `${prefix}XXXXXX`;
66+
} else {
67+
path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);
68+
}
69+
70+
return binding.mkdtempSync(
71+
path,
72+
options.encoding,
73+
);
74+
}
75+
5476
function stat(path, options = { bigint: false, throwIfNoEntry: true }) {
5577
path = getValidatedPath(path);
5678
const stats = binding.statSync(
@@ -91,6 +113,7 @@ module.exports = {
91113
exists,
92114
access,
93115
copyFile,
116+
mkdtemp,
94117
stat,
95118
statfs,
96119
open,

src/node_file.cc

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2947,6 +2947,38 @@ static void Mkdtemp(const FunctionCallbackInfo<Value>& args) {
29472947
}
29482948
}
29492949

2950+
static void MkdtempSync(const FunctionCallbackInfo<Value>& args) {
2951+
Environment* env = Environment::GetCurrent(args);
2952+
Isolate* isolate = env->isolate();
2953+
2954+
CHECK_GE(args.Length(), 2);
2955+
2956+
BufferValue tmpl(isolate, args[0]);
2957+
CHECK_NOT_NULL(*tmpl);
2958+
THROW_IF_INSUFFICIENT_PERMISSIONS(
2959+
env, permission::PermissionScope::kFileSystemWrite, tmpl.ToStringView());
2960+
2961+
const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8);
2962+
2963+
uv_fs_t req;
2964+
auto make = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
2965+
FS_SYNC_TRACE_BEGIN(mkdtemp);
2966+
int err = uv_fs_mkdtemp(nullptr, &req, *tmpl, nullptr);
2967+
FS_SYNC_TRACE_END(mkdtemp);
2968+
if (err < 0) {
2969+
return env->ThrowUVException(err, "mkdtemp", nullptr, *tmpl);
2970+
}
2971+
2972+
Local<Value> error;
2973+
MaybeLocal<Value> rc =
2974+
StringBytes::Encode(isolate, req.path, encoding, &error);
2975+
if (rc.IsEmpty()) {
2976+
env->isolate()->ThrowException(error);
2977+
return;
2978+
}
2979+
args.GetReturnValue().Set(rc.ToLocalChecked());
2980+
}
2981+
29502982
static bool FileURLToPath(
29512983
Environment* env,
29522984
const ada::url_aggregator& file_url,
@@ -3399,6 +3431,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
33993431
SetMethod(isolate, target, "lutimes", LUTimes);
34003432

34013433
SetMethod(isolate, target, "mkdtemp", Mkdtemp);
3434+
SetMethodNoSideEffect(isolate, target, "mkdtempSync", MkdtempSync);
34023435

34033436
StatWatcher::CreatePerIsolateProperties(isolate_data, target);
34043437
BindingData::CreatePerIsolateProperties(isolate_data, target);
@@ -3524,6 +3557,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
35243557
registry->Register(LUTimes);
35253558

35263559
registry->Register(Mkdtemp);
3560+
registry->Register(MkdtempSync);
35273561
registry->Register(NewFSReqCallback);
35283562

35293563
registry->Register(FileHandle::New);

0 commit comments

Comments
 (0)