Skip to content

Commit

Permalink
fs: improve error performance for fs.mkdtempSync
Browse files Browse the repository at this point in the history
  • Loading branch information
anonrig committed Sep 26, 2023
1 parent 31e727d commit 58a4bcd
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 17 deletions.
38 changes: 38 additions & 0 deletions benchmark/fs/bench-mkdtempSync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
'use strict';

const common = require('../common');
const fs = require('fs');
const tmpdir = require('../../test/common/tmpdir');
tmpdir.refresh();

const nonexistentDir = tmpdir.resolve('non-existent', 'foo', 'bar');
const bench = common.createBenchmark(main, {
type: ['valid', 'invalid'],
n: [1e3],
});

function main({ n, type }) {
let prefix;

switch (type) {
case 'valid':
prefix = `${Date.now()}`;
break;
case 'invalid':
prefix = nonexistentDir;
break;
default:
new Error('Invalid type');
}

bench.start();
for (let i = 0; i < n; i++) {
try {
const folderPath = fs.mkdtempSync(prefix, { encoding: 'utf8' });
fs.rmdirSync(folderPath);
} catch {
// do nothing
}
}
bench.end(n);
}
18 changes: 1 addition & 17 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -2893,23 +2893,7 @@ function mkdtemp(prefix, options, callback) {
* @returns {string}
*/
function mkdtempSync(prefix, options) {
options = getOptions(options);

prefix = getValidatedPath(prefix, 'prefix');
warnOnNonPortableTemplate(prefix);

let path;
if (typeof prefix === 'string') {
path = `${prefix}XXXXXX`;
} else {
path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);
}

const ctx = { path };
const result = binding.mkdtemp(path, options.encoding,
undefined, ctx);
handleErrorFromBinding(ctx);
return result;
return syncFs.mkdtemp(prefix, options);
}

/**
Expand Down
23 changes: 23 additions & 0 deletions lib/internal/fs/sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ const {
getStatsFromBinding,
getStatFsFromBinding,
getValidatedFd,
getOptions,
warnOnNonPortableTemplate,
} = require('internal/fs/utils');
const { parseFileMode, isInt32 } = require('internal/validators');
const { Buffer } = require('buffer');

const binding = internalBinding('fs');

Expand Down Expand Up @@ -53,6 +56,25 @@ function copyFile(src, dest, mode) {
);
}

function mkdtemp(prefix, options) {
options = getOptions(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.mkdtempSync(
path,
options.encoding,
);
}

function stat(path, options = { bigint: false, throwIfNoEntry: true }) {
path = getValidatedPath(path);
const stats = binding.statSync(
Expand Down Expand Up @@ -93,6 +115,7 @@ module.exports = {
exists,
access,
copyFile,
mkdtemp,
stat,
statfs,
open,
Expand Down
34 changes: 34 additions & 0 deletions src/node_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2957,6 +2957,38 @@ static void Mkdtemp(const FunctionCallbackInfo<Value>& args) {
}
}

static void MkdtempSync(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();

CHECK_GE(args.Length(), 2);

BufferValue tmpl(isolate, args[0]);
CHECK_NOT_NULL(*tmpl);
THROW_IF_INSUFFICIENT_PERMISSIONS(
env, permission::PermissionScope::kFileSystemWrite, tmpl.ToStringView());

const enum encoding encoding = ParseEncoding(isolate, args[1], UTF8);

uv_fs_t req;
auto make = OnScopeLeave([&req]() { uv_fs_req_cleanup(&req); });
FS_SYNC_TRACE_BEGIN(mkdtemp);
int err = uv_fs_mkdtemp(nullptr, &req, *tmpl, nullptr);
FS_SYNC_TRACE_END(mkdtemp);
if (err < 0) {
return env->ThrowUVException(err, "mkdtemp", nullptr, *tmpl);
}

Local<Value> error;
MaybeLocal<Value> rc =
StringBytes::Encode(isolate, req.path, encoding, &error);
if (rc.IsEmpty()) {
env->isolate()->ThrowException(error);
return;
}
args.GetReturnValue().Set(rc.ToLocalChecked());
}

static bool FileURLToPath(
Environment* env,
const ada::url_aggregator& file_url,
Expand Down Expand Up @@ -3409,6 +3441,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
SetMethod(isolate, target, "lutimes", LUTimes);

SetMethod(isolate, target, "mkdtemp", Mkdtemp);
SetMethodNoSideEffect(isolate, target, "mkdtempSync", MkdtempSync);

StatWatcher::CreatePerIsolateProperties(isolate_data, target);
BindingData::CreatePerIsolateProperties(isolate_data, target);
Expand Down Expand Up @@ -3534,6 +3567,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(LUTimes);

registry->Register(Mkdtemp);
registry->Register(MkdtempSync);
registry->Register(NewFSReqCallback);

registry->Register(FileHandle::New);
Expand Down

0 comments on commit 58a4bcd

Please sign in to comment.