Skip to content

Commit 6284b49

Browse files
addaleaxBridgeAR
authored andcommitted
src: use libuv to get env vars
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: #29188 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
1 parent b9c7c90 commit 6284b49

File tree

1 file changed

+28
-56
lines changed

1 file changed

+28
-56
lines changed

src/node_env_var.cc

+28-56
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,6 @@
22
#include "node_errors.h"
33
#include "node_process.h"
44

5-
#ifdef __APPLE__
6-
#include <crt_externs.h>
7-
#define environ (*_NSGetEnviron())
8-
#elif !defined(_MSC_VER)
9-
extern char** environ;
10-
#endif
11-
125
namespace node {
136
using v8::Array;
147
using v8::Boolean;
@@ -107,12 +100,6 @@ int32_t RealEnvStore::Query(Isolate* isolate, Local<String> property) const {
107100
Mutex::ScopedLock lock(per_process::env_var_mutex);
108101

109102
node::Utf8Value key(isolate, property);
110-
#ifdef _WIN32
111-
if (key[0] == L'=')
112-
return static_cast<int32_t>(v8::ReadOnly) |
113-
static_cast<int32_t>(v8::DontDelete) |
114-
static_cast<int32_t>(v8::DontEnum);
115-
#endif
116103

117104
char val[2];
118105
size_t init_sz = sizeof(val);
@@ -122,6 +109,14 @@ int32_t RealEnvStore::Query(Isolate* isolate, Local<String> property) const {
122109
return -1;
123110
}
124111

112+
#ifdef _WIN32
113+
if (key[0] == L'=') {
114+
return static_cast<int32_t>(v8::ReadOnly) |
115+
static_cast<int32_t>(v8::DontDelete) |
116+
static_cast<int32_t>(v8::DontEnum);
117+
}
118+
#endif
119+
125120
return 0;
126121
}
127122

@@ -134,54 +129,31 @@ void RealEnvStore::Delete(Isolate* isolate, Local<String> property) {
134129

135130
Local<Array> RealEnvStore::Enumerate(Isolate* isolate) const {
136131
Mutex::ScopedLock lock(per_process::env_var_mutex);
137-
#ifdef __POSIX__
138-
int env_size = 0;
139-
while (environ[env_size]) {
140-
env_size++;
141-
}
142-
std::vector<Local<Value>> env_v(env_size);
143-
144-
for (int i = 0; i < env_size; ++i) {
145-
const char* var = environ[i];
146-
const char* s = strchr(var, '=');
147-
const int length = s ? s - var : strlen(var);
148-
env_v[i] = String::NewFromUtf8(isolate, var, NewStringType::kNormal, length)
149-
.ToLocalChecked();
150-
}
151-
#else // _WIN32
152-
std::vector<Local<Value>> env_v;
153-
WCHAR* environment = GetEnvironmentStringsW();
154-
if (environment == nullptr)
155-
return Array::New(isolate); // This should not happen.
156-
WCHAR* p = environment;
157-
while (*p) {
158-
WCHAR* s;
159-
if (*p == L'=') {
160-
// If the key starts with '=' it is a hidden environment variable.
161-
p += wcslen(p) + 1;
162-
continue;
163-
} else {
164-
s = wcschr(p, L'=');
165-
}
166-
if (!s) {
167-
s = p + wcslen(p);
168-
}
169-
const uint16_t* two_byte_buffer = reinterpret_cast<const uint16_t*>(p);
170-
const size_t two_byte_buffer_len = s - p;
171-
v8::MaybeLocal<String> rc = String::NewFromTwoByte(
172-
isolate, two_byte_buffer, NewStringType::kNormal, two_byte_buffer_len);
173-
if (rc.IsEmpty()) {
132+
uv_env_item_t* items;
133+
int count;
134+
135+
OnScopeLeave cleanup([&]() { uv_os_free_environ(items, count); });
136+
CHECK_EQ(uv_os_environ(&items, &count), 0);
137+
138+
MaybeStackBuffer<Local<Value>, 256> env_v(count);
139+
int env_v_index = 0;
140+
for (int i = 0; i < count; i++) {
141+
#ifdef _WIN32
142+
// If the key starts with '=' it is a hidden environment variable.
143+
// The '\0' check is a workaround for the bug behind
144+
// https://github.com/libuv/libuv/pull/2473 and can be removed later.
145+
if (items[i].name[0] == '=' || items[i].name[0] == '\0') continue;
146+
#endif
147+
MaybeLocal<String> str = String::NewFromUtf8(
148+
isolate, items[i].name, NewStringType::kNormal);
149+
if (str.IsEmpty()) {
174150
isolate->ThrowException(ERR_STRING_TOO_LONG(isolate));
175-
FreeEnvironmentStringsW(environment);
176151
return Local<Array>();
177152
}
178-
env_v.push_back(rc.ToLocalChecked());
179-
p = s + wcslen(s) + 1;
153+
env_v[env_v_index++] = str.ToLocalChecked();
180154
}
181-
FreeEnvironmentStringsW(environment);
182-
#endif
183155

184-
return Array::New(isolate, env_v.data(), env_v.size());
156+
return Array::New(isolate, env_v.out(), env_v_index);
185157
}
186158

187159
std::shared_ptr<KVStore> KVStore::Clone(v8::Isolate* isolate) const {

0 commit comments

Comments
 (0)