Skip to content

Commit

Permalink
os: improve tmpdir performance
Browse files Browse the repository at this point in the history
PR-URL: #54709
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
  • Loading branch information
anonrig authored and aduh95 committed Sep 13, 2024
1 parent 5e72bd3 commit 461e523
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 11 deletions.
31 changes: 31 additions & 0 deletions benchmark/os/tmpdir.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
'use strict';

const common = require('../common.js');
const { tmpdir } = require('os');
const assert = require('assert');

const bench = common.createBenchmark(main, {
n: [1e6],
});

function main({ n }) {
// Warm up.
const length = 1024;
const array = [];
for (let i = 0; i < length; ++i) {
array.push(tmpdir());
}

bench.start();
for (let i = 0; i < n; ++i) {
const index = i % length;
array[index] = tmpdir();
}
bench.end(n);

// Verify the entries to prevent dead code elimination from making
// the benchmark invalid.
for (let i = 0; i < length; ++i) {
assert.strictEqual(typeof array[i], 'string');
}
}
16 changes: 5 additions & 11 deletions lib/os.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const {
SymbolToPrimitive,
} = primordials;

const { safeGetenv } = internalBinding('credentials');
const { getTempDir } = internalBinding('credentials');
const constants = internalBinding('constants').os;
const isWindows = process.platform === 'win32';

Expand Down Expand Up @@ -179,24 +179,18 @@ platform[SymbolToPrimitive] = () => process.platform;
* @returns {string}
*/
function tmpdir() {
let path;
if (isWindows) {
path = process.env.TEMP ||
let path = process.env.TEMP ||
process.env.TMP ||
(process.env.SystemRoot || process.env.windir) + '\\temp';
if (path.length > 1 && StringPrototypeEndsWith(path, '\\') &&
!StringPrototypeEndsWith(path, ':\\'))
path = StringPrototypeSlice(path, 0, -1);
} else {
path = safeGetenv('TMPDIR') ||
safeGetenv('TMP') ||
safeGetenv('TEMP') ||
'/tmp';
if (path.length > 1 && StringPrototypeEndsWith(path, '/'))
path = StringPrototypeSlice(path, 0, -1);

return path;
}

return path;
return getTempDir() || '/tmp';
}
tmpdir[SymbolToPrimitive] = () => tmpdir();

Expand Down
27 changes: 27 additions & 0 deletions src/node_credentials.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,31 @@ static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(result);
}

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

std::string dir;

// Let's wrap SafeGetEnv since it returns true for empty string.
auto get_env = [&dir, &env](std::string_view key) {
USE(SafeGetenv(key.data(), &dir, env->env_vars()));
return !dir.empty();
};

// Try TMPDIR, TMP, and TEMP in that order.
if (!get_env("TMPDIR") && !get_env("TMP") && !get_env("TEMP")) {
return;
}

if (dir.size() > 1 && dir.ends_with("/")) {
dir.pop_back();
}

args.GetReturnValue().Set(
ToV8Value(isolate->GetCurrentContext(), dir).ToLocalChecked());
}

#ifdef NODE_IMPLEMENTS_POSIX_CREDENTIALS

static const uid_t uid_not_found = static_cast<uid_t>(-1);
Expand Down Expand Up @@ -456,6 +481,7 @@ static void InitGroups(const FunctionCallbackInfo<Value>& args) {

void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(SafeGetenv);
registry->Register(GetTempDir);

#ifdef NODE_IMPLEMENTS_POSIX_CREDENTIALS
registry->Register(GetUid);
Expand All @@ -478,6 +504,7 @@ static void Initialize(Local<Object> target,
Local<Context> context,
void* priv) {
SetMethod(context, target, "safeGetenv", SafeGetenv);
SetMethod(context, target, "getTempDir", GetTempDir);

#ifdef NODE_IMPLEMENTS_POSIX_CREDENTIALS
Environment* env = Environment::GetCurrent(context);
Expand Down

0 comments on commit 461e523

Please sign in to comment.