From 461e52349848066c58ace800df560502ded51a05 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Wed, 11 Sep 2024 10:46:30 -0400 Subject: [PATCH] os: improve `tmpdir` performance PR-URL: https://github.com/nodejs/node/pull/54709 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Rafael Gonzaga Reviewed-By: Trivikram Kamat --- benchmark/os/tmpdir.js | 31 +++++++++++++++++++++++++++++++ lib/os.js | 16 +++++----------- src/node_credentials.cc | 27 +++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 11 deletions(-) create mode 100644 benchmark/os/tmpdir.js diff --git a/benchmark/os/tmpdir.js b/benchmark/os/tmpdir.js new file mode 100644 index 00000000000000..ba083fc280c001 --- /dev/null +++ b/benchmark/os/tmpdir.js @@ -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'); + } +} diff --git a/lib/os.js b/lib/os.js index bef4936c7c0bbe..bbae40b2ab1046 100644 --- a/lib/os.js +++ b/lib/os.js @@ -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'; @@ -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(); diff --git a/src/node_credentials.cc b/src/node_credentials.cc index 6333c64231d30f..65fdd145167139 100644 --- a/src/node_credentials.cc +++ b/src/node_credentials.cc @@ -109,6 +109,31 @@ static void SafeGetenv(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(result); } +static void GetTempDir(const FunctionCallbackInfo& 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(-1); @@ -456,6 +481,7 @@ static void InitGroups(const FunctionCallbackInfo& args) { void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(SafeGetenv); + registry->Register(GetTempDir); #ifdef NODE_IMPLEMENTS_POSIX_CREDENTIALS registry->Register(GetUid); @@ -478,6 +504,7 @@ static void Initialize(Local target, Local context, void* priv) { SetMethod(context, target, "safeGetenv", SafeGetenv); + SetMethod(context, target, "getTempDir", GetTempDir); #ifdef NODE_IMPLEMENTS_POSIX_CREDENTIALS Environment* env = Environment::GetCurrent(context);