child_process: build the default env block in one native pass · nodejs/node@a0e7141 · GitHub
Skip to content

Commit a0e7141

Browse files
codebytereaduh95
authored andcommitted
child_process: build the default env block in one native pass
When spawn()/spawnSync() are called without options.env, normalizeSpawnArguments() copied process.env with a spread and then walked the copy to build the KEY=value array uv_spawn() takes. Spreading the process.env proxy costs one enumerator callback plus a query and a getter interceptor per variable, each doing a linear getenv() scan and allocating; with a couple of hundred variables that was the single largest JS-side cost of spawning a process. Add KVStore::Pairs() (Enumerate() + Get() by default, one uv_os_environ() pass for the real environment, skipping hidden variables on Windows exactly like Enumerate() does), expose it as process_wrap.getEnvPairs(), and use it for the default-environment case. A user supplied options.env and the permission model case keep the existing code. On Windows the same sort/first-wins-case-insensitive filter is applied to the pairs. The variables copyProcessEnvToEnv() propagates are part of the real environment by definition, and its entries cannot contain null bytes, so those steps only remain on the options.env path. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65325 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 4977e51 commit a0e7141

5 files changed

Lines changed: 248 additions & 50 deletions

File tree

‎lib/child_process.js‎

Lines changed: 95 additions & 50 deletions

‎src/node_env_var.cc‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ using v8::Boolean;
1515
using v8::Context;
1616
using v8::DontDelete;
1717
using v8::DontEnum;
18+
using v8::EscapableHandleScope;
1819
using v8::FunctionTemplate;
1920
using v8::HandleScope;
2021
using v8::IndexedPropertyHandlerConfiguration;
@@ -47,6 +48,7 @@ class RealEnvStore final : public KVStore {
4748
int32_t Query(const char* key) const override;
4849
void Delete(Isolate* isolate, Local<String> key) override;
4950
MaybeLocal<Array> Enumerate(Isolate* isolate) const override;
51+
MaybeLocal<Array> Pairs(Isolate* isolate) const override;
5052
};
5153

5254
class MapKVStore final : public KVStore {
@@ -222,6 +224,64 @@ MaybeLocal<Array> RealEnvStore::Enumerate(Isolate* isolate) const {
222224
return env_v.ToArray();
223225
}
224226

227+
MaybeLocal<Array> RealEnvStore::Pairs(Isolate* isolate) const {
228+
Mutex::ScopedLock lock(per_process::env_var_mutex);
229+
uv_env_item_t* items;
230+
int count;
231+
232+
auto cleanup = OnScopeLeave([&]() { uv_os_free_environ(items, count); });
233+
CHECK_EQ(uv_os_environ(&items, &count), 0);
234+
235+
MaybeStackBuffer<Local<Value>, 256> pairs_v(count);
236+
int pairs_v_index = 0;
237+
std::string pair;
238+
for (int i = 0; i < count; i++) {
239+
#ifdef _WIN32
240+
// If the key starts with '=' it is a hidden environment variable.
241+
// Enumerate() skips these, so a copy of process.env never had them.
242+
if (items[i].name[0] == '=') continue;
243+
#endif
244+
pair.assign(items[i].name);
245+
pair += '=';
246+
pair += items[i].value;
247+
Local<Value> str;
248+
if (!ToV8Value(isolate->GetCurrentContext(), pair, isolate).ToLocal(&str)) {
249+
return {};
250+
}
251+
pairs_v[pairs_v_index++] = str;
252+
}
253+
254+
return Array::New(isolate, pairs_v.out(), pairs_v_index);
255+
}
256+
257+
MaybeLocal<Array> KVStore::Pairs(Isolate* isolate) const {
258+
EscapableHandleScope scope(isolate);
259+
Local<Context> context = isolate->GetCurrentContext();
260+
Local<Array> keys;
261+
if (!Enumerate(isolate).ToLocal(&keys)) return {};
262+
uint32_t keys_length = keys->Length();
263+
MaybeStackBuffer<Local<Value>, 256> pairs(keys_length);
264+
uint32_t pairs_index = 0;
265+
std::string pair;
266+
for (uint32_t i = 0; i < keys_length; i++) {
267+
Local<Value> key;
268+
if (!keys->Get(context, i).ToLocal(&key)) return {};
269+
if (!key->IsString()) continue;
270+
Utf8Value key_utf8(isolate, key);
271+
// A key that disappeared between Enumerate() and Get() is skipped, like an
272+
// undefined value is when copying process.env in JS.
273+
std::optional<std::string> value = Get(*key_utf8);
274+
if (!value.has_value()) continue;
275+
pair.assign(*key_utf8, key_utf8.length());
276+
pair += '=';
277+
pair += *value;
278+
Local<Value> str;
279+
if (!ToV8Value(context, pair, isolate).ToLocal(&str)) return {};
280+
pairs[pairs_index++] = str;
281+
}
282+
return scope.Escape(Array::New(isolate, pairs.out(), pairs_index));
283+
}
284+
225285
std::shared_ptr<KVStore> KVStore::Clone(Isolate* isolate) const {
226286
HandleScope handle_scope(isolate);
227287
Local<Context> context = isolate->GetCurrentContext();

‎src/process_wrap.cc‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ class ProcessWrap : public HandleWrap {
7979
SetProtoMethod(isolate, constructor, "kill", Kill);
8080

8181
SetConstructorFunction(context, target, "Process", constructor);
82+
SetMethodNoSideEffect(context, target, "getEnvPairs", GetEnvPairs);
8283

8384
Local<Object> constants = Object::New(isolate);
8485
NODE_DEFINE_CONSTANT(constants, kProcessFlagDetached);
@@ -91,6 +92,7 @@ class ProcessWrap : public HandleWrap {
9192
registry->Register(New);
9293
registry->Register(Spawn);
9394
registry->Register(Kill);
95+
registry->Register(GetEnvPairs);
9496
}
9597

9698
SET_NO_MEMORY_INFO()
@@ -342,6 +344,17 @@ class ProcessWrap : public HandleWrap {
342344
args.GetReturnValue().Set(err);
343345
}
344346

347+
// The current environment as ["KEY=value", ...], i.e. what a spawned
348+
// child inherits by default, produced in one pass over the environment
349+
// block instead of one interceptor round trip per variable.
350+
static void GetEnvPairs(const FunctionCallbackInfo<Value>& args) {
351+
Environment* env = Environment::GetCurrent(args);
352+
Local<Array> pairs;
353+
if (env->env_vars()->Pairs(env->isolate()).ToLocal(&pairs)) {
354+
args.GetReturnValue().Set(pairs);
355+
}
356+
}
357+
345358
static void Kill(const FunctionCallbackInfo<Value>& args) {
346359
Environment* env = Environment::GetCurrent(args);
347360
ProcessWrap* wrap;

‎src/util.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,10 @@ class KVStore {
323323
virtual int32_t Query(const char* key) const = 0;
324324
virtual void Delete(v8::Isolate* isolate, v8::Local<v8::String> key) = 0;
325325
virtual v8::MaybeLocal<v8::Array> Enumerate(v8::Isolate* isolate) const = 0;
326+
// All entries as an array of "KEY=value" strings, in enumeration order —
327+
// the form uv_spawn() consumes. The default implementation is
328+
// Enumerate() + Get(); stores that can produce it in one pass override it.
329+
virtual v8::MaybeLocal<v8::Array> Pairs(v8::Isolate* isolate) const;
326330

327331
virtual std::shared_ptr<KVStore> Clone(v8::Isolate* isolate) const;
328332
virtual v8::Maybe<void> AssignFromObject(v8::Local<v8::Context> context,
Lines changed: 76 additions & 0 deletions

0 commit comments

Comments
 (0)