Skip to content

Commit

Permalink
src: use libuv to get env vars
Browse files Browse the repository at this point in the history
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 <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
addaleax committed Sep 18, 2019
1 parent 82ebcb3 commit 5664791
Showing 1 changed file with 28 additions and 56 deletions.
84 changes: 28 additions & 56 deletions src/node_env_var.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

Expand All @@ -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 {
Expand Down

0 comments on commit 5664791

Please sign in to comment.