os: improve tmpdir performance

PR-URL: nodejs#54709
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>

Author: anonrig

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');
+  }
+}

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();
 

src/node_credentials.cc

@@ -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);
@@ -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);
@@ -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);