From 566479179082154decc2af59dbb64e53f200e970 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 17 Aug 2019 23:01:34 +0200
Subject: [PATCH] src: use libuv to get env vars
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This allows us to remove OS-dependent code.

                                                           confidence improvement accuracy (*)    (**)   (***)
     process/bench-env.js operation='delete' n=1000000                    3.57 %      ±10.86% ±14.46% ±18.85%
     process/bench-env.js operation='enumerate' n=1000000        ***    -14.06 %       ±7.46%  ±9.94% ±12.96%
     process/bench-env.js operation='get' n=1000000                      -7.97 %      ±11.80% ±15.70% ±20.45%
     process/bench-env.js operation='query' n=1000000                    -1.32 %       ±8.38% ±11.17% ±14.58%
     process/bench-env.js operation='set' n=1000000                      -0.98 %       ±9.63% ±12.81% ±16.68%

The drop in enumeration performance is likely due to the large
number of extra allocations that libuv performs. However, enumerating
process.env should generally not be a hot path in most applications.

PR-URL: https://github.com/nodejs/node/pull/29188
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 src/node_env_var.cc | 84 +++++++++++++++------------------------------
 1 file changed, 28 insertions(+), 56 deletions(-)

diff --git a/src/node_env_var.cc b/src/node_env_var.cc
index 2c1af65b867b0b..c63cb2c37fb3e0 100644
--- a/src/node_env_var.cc
+++ b/src/node_env_var.cc
@@ -4,13 +4,6 @@
 
 #include <time.h>  // tzset(), _tzset()
 
-#ifdef __APPLE__
-#include <crt_externs.h>
-#define environ (*_NSGetEnviron())
-#elif !defined(_MSC_VER)
-extern char** environ;
-#endif
-
 namespace node {
 using v8::Array;
 using v8::Boolean;
@@ -123,12 +116,6 @@ int32_t RealEnvStore::Query(Isolate* isolate, Local<String> property) const {
   Mutex::ScopedLock lock(per_process::env_var_mutex);
 
   node::Utf8Value key(isolate, property);
-#ifdef _WIN32
-  if (key[0] == L'=')
-    return static_cast<int32_t>(v8::ReadOnly) |
-           static_cast<int32_t>(v8::DontDelete) |
-           static_cast<int32_t>(v8::DontEnum);
-#endif
 
   char val[2];
   size_t init_sz = sizeof(val);
@@ -138,6 +125,14 @@ int32_t RealEnvStore::Query(Isolate* isolate, Local<String> property) const {
     return -1;
   }
 
+#ifdef _WIN32
+  if (key[0] == L'=') {
+    return static_cast<int32_t>(v8::ReadOnly) |
+           static_cast<int32_t>(v8::DontDelete) |
+           static_cast<int32_t>(v8::DontEnum);
+  }
+#endif
+
   return 0;
 }
 
@@ -151,54 +146,31 @@ void RealEnvStore::Delete(Isolate* isolate, Local<String> property) {
 
 Local<Array> RealEnvStore::Enumerate(Isolate* isolate) const {
   Mutex::ScopedLock lock(per_process::env_var_mutex);
-#ifdef __POSIX__
-  int env_size = 0;
-  while (environ[env_size]) {
-    env_size++;
-  }
-  std::vector<Local<Value>> env_v(env_size);
-
-  for (int i = 0; i < env_size; ++i) {
-    const char* var = environ[i];
-    const char* s = strchr(var, '=');
-    const int length = s ? s - var : strlen(var);
-    env_v[i] = String::NewFromUtf8(isolate, var, NewStringType::kNormal, length)
-                   .ToLocalChecked();
-  }
-#else  // _WIN32
-  std::vector<Local<Value>> env_v;
-  WCHAR* environment = GetEnvironmentStringsW();
-  if (environment == nullptr)
-    return Array::New(isolate);  // This should not happen.
-  WCHAR* p = environment;
-  while (*p) {
-    WCHAR* s;
-    if (*p == L'=') {
-      // If the key starts with '=' it is a hidden environment variable.
-      p += wcslen(p) + 1;
-      continue;
-    } else {
-      s = wcschr(p, L'=');
-    }
-    if (!s) {
-      s = p + wcslen(p);
-    }
-    const uint16_t* two_byte_buffer = reinterpret_cast<const uint16_t*>(p);
-    const size_t two_byte_buffer_len = s - p;
-    v8::MaybeLocal<String> rc = String::NewFromTwoByte(
-        isolate, two_byte_buffer, NewStringType::kNormal, two_byte_buffer_len);
-    if (rc.IsEmpty()) {
+  uv_env_item_t* items;
+  int count;
+
+  OnScopeLeave cleanup([&]() { uv_os_free_environ(items, count); });
+  CHECK_EQ(uv_os_environ(&items, &count), 0);
+
+  MaybeStackBuffer<Local<Value>, 256> env_v(count);
+  int env_v_index = 0;
+  for (int i = 0; i < count; i++) {
+#ifdef _WIN32
+    // If the key starts with '=' it is a hidden environment variable.
+    // The '\0' check is a workaround for the bug behind
+    // https://github.com/libuv/libuv/pull/2473 and can be removed later.
+    if (items[i].name[0] == '=' || items[i].name[0] == '\0') continue;
+#endif
+    MaybeLocal<String> str = String::NewFromUtf8(
+        isolate, items[i].name, NewStringType::kNormal);
+    if (str.IsEmpty()) {
       isolate->ThrowException(ERR_STRING_TOO_LONG(isolate));
-      FreeEnvironmentStringsW(environment);
       return Local<Array>();
     }
-    env_v.push_back(rc.ToLocalChecked());
-    p = s + wcslen(s) + 1;
+    env_v[env_v_index++] = str.ToLocalChecked();
   }
-  FreeEnvironmentStringsW(environment);
-#endif
 
-  return Array::New(isolate, env_v.data(), env_v.size());
+  return Array::New(isolate, env_v.out(), env_v_index);
 }
 
 std::shared_ptr<KVStore> KVStore::Clone(v8::Isolate* isolate) const {