Skip to content

Commit 91a67db

Browse files
committed
child_process: pass spawn options to the binding positionally
ChildProcess.prototype.spawn() handed a single options object to the ProcessWrap::Spawn() binding, which then read about a dozen properties back out individually with Object::Get(). Each of those is a property lookup across the JS/C++ boundary on every spawn. Pass file, args, cwd, envPairs, stdio, uid and gid as positional arguments and pack the boolean flags (detached, windowsHide, windowsVerbatimArguments) into a single integer whose bit values are exported from the binding as `constants`. The native side then reads each value directly from the call arguments. Add internal typings for the process_wrap binding (previously untyped) describing the new positional spawn() signature and the exported constants. There is no observable behavior change. Spawn wall-clock time is dominated by the operating system process-creation cost and is unchanged; this reduces the per-spawn work done on the main thread and clarifies the contract between the JS and C++ layers. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
1 parent 3f42cfa commit 91a67db

4 files changed

Lines changed: 115 additions & 76 deletions

File tree

lib/internal/child_process.js

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ const dgram = require('dgram');
4141
const inspect = require('internal/util/inspect').inspect;
4242
const assert = require('internal/assert');
4343

44-
const { Process } = internalBinding('process_wrap');
44+
const { Process, constants: processConstants } = internalBinding('process_wrap');
4545
const {
4646
WriteWrap,
4747
kReadBytesOrError,
@@ -397,7 +397,27 @@ ChildProcess.prototype.spawn = function spawn(options) {
397397
childProcessSpawn.start.publish({ process: this, options });
398398
}
399399

400-
const err = this._handle.spawn(options);
400+
// Pack the boolean flags into a single integer and pass the remaining options
401+
// positionally so the native side can read each value directly instead of
402+
// performing a property lookup per option across the JS/C++ boundary.
403+
let spawnFlags = 0;
404+
if (options.detached)
405+
spawnFlags |= processConstants.kProcessFlagDetached;
406+
if (options.windowsHide)
407+
spawnFlags |= processConstants.kProcessFlagWindowsHide;
408+
if (options.windowsVerbatimArguments)
409+
spawnFlags |= processConstants.kProcessFlagWindowsVerbatimArguments;
410+
411+
const err = this._handle.spawn(
412+
options.file,
413+
options.args,
414+
options.cwd,
415+
options.envPairs,
416+
options.stdio,
417+
spawnFlags,
418+
options.uid,
419+
options.gid,
420+
);
401421

402422
// Run-time errors should emit an error, not throw an exception.
403423
if (err === UV_EACCES ||

src/process_wrap.cc

Lines changed: 46 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,21 @@ using v8::Nothing;
4949
using v8::Number;
5050
using v8::Object;
5151
using v8::String;
52+
using v8::Uint32;
5253
using v8::Value;
5354

5455
namespace {
5556

57+
// Flag bits packed by the JS caller and passed to Spawn() as a single integer
58+
// to avoid a cross-boundary property lookup per option. Keep in sync with the
59+
// `constants` exported below and consumed in lib/internal/child_process.js.
60+
enum ProcessFlags : uint32_t {
61+
kProcessFlagNone = 0,
62+
kProcessFlagDetached = 1 << 0,
63+
kProcessFlagWindowsHide = 1 << 1,
64+
kProcessFlagWindowsVerbatimArguments = 1 << 2,
65+
};
66+
5667
class ProcessWrap : public HandleWrap {
5768
public:
5869
static void Initialize(Local<Object> target,
@@ -71,6 +82,12 @@ class ProcessWrap : public HandleWrap {
7182
SetProtoMethod(isolate, constructor, "kill", Kill);
7283

7384
SetConstructorFunction(context, target, "Process", constructor);
85+
86+
Local<Object> constants = Object::New(isolate);
87+
NODE_DEFINE_CONSTANT(constants, kProcessFlagDetached);
88+
NODE_DEFINE_CONSTANT(constants, kProcessFlagWindowsHide);
89+
NODE_DEFINE_CONSTANT(constants, kProcessFlagWindowsVerbatimArguments);
90+
target->Set(context, env->constants_string(), constants).Check();
7491
}
7592

7693
static void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
@@ -118,14 +135,9 @@ class ProcessWrap : public HandleWrap {
118135

119136
static Maybe<void> ParseStdioOptions(
120137
Environment* env,
121-
Local<Object> js_options,
138+
Local<Value> stdios_val,
122139
std::vector<uv_stdio_container_t>* options_stdio) {
123140
Local<Context> context = env->context();
124-
Local<String> stdio_key = env->stdio_string();
125-
Local<Value> stdios_val;
126-
if (!js_options->Get(context, stdio_key).ToLocal(&stdios_val)) {
127-
return Nothing<void>();
128-
}
129141
if (!stdios_val->IsArray()) {
130142
THROW_ERR_INVALID_ARG_TYPE(env, "options.stdio must be an array");
131143
return Nothing<void>();
@@ -181,53 +193,42 @@ class ProcessWrap : public HandleWrap {
181193
return JustVoid();
182194
}
183195

196+
// Spawn(file, args, cwd, envPairs, stdio, flags, uid, gid)
197+
//
198+
// The arguments are passed positionally rather than as a single options
199+
// object so that each value can be read directly from the call arguments,
200+
// avoiding a property lookup across the JS/C++ boundary per option.
184201
static void Spawn(const FunctionCallbackInfo<Value>& args) {
185202
Environment* env = Environment::GetCurrent(args);
186203
Local<Context> context = env->context();
187204
ProcessWrap* wrap;
188205
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.This());
189206
int err = 0;
190207

191-
if (!args[0]->IsObject()) {
192-
return THROW_ERR_INVALID_ARG_TYPE(env, "options must be an object");
193-
}
194-
195-
Local<Object> js_options = args[0].As<Object>();
196-
197208
uv_process_options_t options;
198209
memset(&options, 0, sizeof(uv_process_options_t));
199210

200211
options.exit_cb = OnExit;
201212

202-
// options.file
203-
Local<Value> file_v;
204-
if (!js_options->Get(context, env->file_string()).ToLocal(&file_v)) {
205-
return;
206-
}
207-
CHECK(file_v->IsString());
208-
node::Utf8Value file(env->isolate(), file_v);
213+
// args[0] file
214+
CHECK(args[0]->IsString());
215+
node::Utf8Value file(env->isolate(), args[0]);
209216
options.file = *file;
210217

211218
THROW_IF_INSUFFICIENT_PERMISSIONS(
212219
env, permission::PermissionScope::kChildProcess, file.ToStringView());
213220

214-
// options.uid
215-
Local<Value> uid_v;
216-
if (!js_options->Get(context, env->uid_string()).ToLocal(&uid_v)) {
217-
return;
218-
}
221+
// args[6] uid
222+
Local<Value> uid_v = args[6];
219223
if (!uid_v->IsUndefined() && !uid_v->IsNull()) {
220224
CHECK(uid_v->IsInt32());
221225
const int32_t uid = uid_v.As<Int32>()->Value();
222226
options.flags |= UV_PROCESS_SETUID;
223227
options.uid = static_cast<uv_uid_t>(uid);
224228
}
225229

226-
// options.gid
227-
Local<Value> gid_v;
228-
if (!js_options->Get(context, env->gid_string()).ToLocal(&gid_v)) {
229-
return;
230-
}
230+
// args[7] gid
231+
Local<Value> gid_v = args[7];
231232
if (!gid_v->IsUndefined() && !gid_v->IsNull()) {
232233
CHECK(gid_v->IsInt32());
233234
const int32_t gid = gid_v.As<Int32>()->Value();
@@ -244,15 +245,11 @@ class ProcessWrap : public HandleWrap {
244245
err = UV_EINVAL;
245246
#endif
246247

247-
// options.args
248-
Local<Value> argv_v;
249-
if (!js_options->Get(context, env->args_string()).ToLocal(&argv_v)) {
250-
return;
251-
}
248+
// args[1] args
252249
std::vector<char*> options_args;
253250
std::vector<std::string> args_vals;
254-
if (argv_v->IsArray()) {
255-
Local<Array> js_argv = argv_v.As<Array>();
251+
if (args[1]->IsArray()) {
252+
Local<Array> js_argv = args[1].As<Array>();
256253
int argc = js_argv->Length();
257254
CHECK_LT(argc, INT_MAX); // Check for overflow.
258255
args_vals.reserve(argc);
@@ -273,26 +270,18 @@ class ProcessWrap : public HandleWrap {
273270
options.args = options_args.data();
274271
}
275272

276-
// options.cwd
277-
Local<Value> cwd_v;
278-
if (!js_options->Get(context, env->cwd_string()).ToLocal(&cwd_v)) {
279-
return;
280-
}
273+
// args[2] cwd
281274
node::Utf8Value cwd(env->isolate(),
282-
cwd_v->IsString() ? cwd_v : Local<Value>());
275+
args[2]->IsString() ? args[2] : Local<Value>());
283276
if (cwd.length() > 0) {
284277
options.cwd = *cwd;
285278
}
286279

287-
// options.env
288-
Local<Value> env_v;
289-
if (!js_options->Get(context, env->env_pairs_string()).ToLocal(&env_v)) {
290-
return;
291-
}
280+
// args[3] envPairs
292281
std::vector<char*> options_env;
293282
std::vector<std::string> env_vals;
294-
if (env_v->IsArray()) {
295-
Local<Array> env_opt = env_v.As<Array>();
283+
if (args[3]->IsArray()) {
284+
Local<Array> env_opt = args[3].As<Array>();
296285
int envc = env_opt->Length();
297286
CHECK_LT(envc, INT_MAX); // Check for overflow.
298287
env_vals.reserve(envc);
@@ -313,48 +302,31 @@ class ProcessWrap : public HandleWrap {
313302
options.env = options_env.data();
314303
}
315304

316-
// options.stdio
305+
// args[4] stdio
317306
std::vector<uv_stdio_container_t> options_stdio;
318-
if (ParseStdioOptions(env, js_options, &options_stdio).IsNothing()) {
307+
if (ParseStdioOptions(env, args[4], &options_stdio).IsNothing()) {
319308
return;
320309
}
321310
options.stdio = options_stdio.data();
322311
options.stdio_count = options_stdio.size();
323312

324-
// options.windowsHide
325-
Local<Value> hide_v;
326-
if (!js_options->Get(context, env->windows_hide_string())
327-
.ToLocal(&hide_v)) {
328-
return;
329-
}
313+
// args[5] flags (detached, windowsHide, windowsVerbatimArguments)
314+
CHECK(args[5]->IsUint32());
315+
const uint32_t flags = args[5].As<Uint32>()->Value();
330316

331-
if (hide_v->IsTrue()) {
317+
if (flags & kProcessFlagWindowsHide) {
332318
options.flags |= UV_PROCESS_WINDOWS_HIDE;
333319
}
334320

335321
if (env->hide_console_windows()) {
336322
options.flags |= UV_PROCESS_WINDOWS_HIDE_CONSOLE;
337323
}
338324

339-
// options.windows_verbatim_arguments
340-
Local<Value> wva_v;
341-
if (!js_options->Get(context, env->windows_verbatim_arguments_string())
342-
.ToLocal(&wva_v)) {
343-
return;
344-
}
345-
346-
if (wva_v->IsTrue()) {
325+
if (flags & kProcessFlagWindowsVerbatimArguments) {
347326
options.flags |= UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS;
348327
}
349328

350-
// options.detached
351-
Local<Value> detached_v;
352-
if (!js_options->Get(context, env->detached_string())
353-
.ToLocal(&detached_v)) {
354-
return;
355-
}
356-
357-
if (detached_v->IsTrue()) {
329+
if (flags & kProcessFlagDetached) {
358330
options.flags |= UV_PROCESS_DETACHED;
359331
}
360332

typings/globals.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { MessagingBinding } from './internalBinding/messaging';
1717
import { OptionsBinding } from './internalBinding/options';
1818
import { OSBinding } from './internalBinding/os';
1919
import { ProcessBinding } from './internalBinding/process';
20+
import { ProcessWrapBinding } from './internalBinding/process_wrap';
2021
import { SeaBinding } from './internalBinding/sea';
2122
import { SerdesBinding } from './internalBinding/serdes';
2223
import { StringDecoderBinding } from './internalBinding/string_decoder';
@@ -53,6 +54,7 @@ interface InternalBindingMap {
5354
options: OptionsBinding;
5455
os: OSBinding;
5556
process: ProcessBinding;
57+
process_wrap: ProcessWrapBinding;
5658
sea: SeaBinding;
5759
serdes: SerdesBinding;
5860
string_decoder: StringDecoderBinding;
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { owner_symbol } from './symbols';
2+
3+
declare namespace InternalProcessWrapBinding {
4+
type StdioType = 'ignore' | 'pipe' | 'overlapped' | 'wrap' | 'inherit' | 'fd';
5+
6+
interface StdioContainer {
7+
type: StdioType;
8+
handle?: object;
9+
fd?: number;
10+
}
11+
12+
class Process {
13+
constructor();
14+
[owner_symbol]?: object;
15+
pid: number;
16+
onexit: (exitCode: number, signalCode: string) => void;
17+
// Spawn(file, args, cwd, envPairs, stdio, flags, uid, gid)
18+
// `flags` is a bitfield built from the `constants` exported below.
19+
spawn(
20+
file: string,
21+
args: string[] | undefined,
22+
cwd: string | undefined,
23+
envPairs: string[] | undefined,
24+
stdio: StdioContainer[],
25+
flags: number,
26+
uid: number | null | undefined,
27+
gid: number | null | undefined,
28+
): number;
29+
kill(signal: number): number;
30+
ref(): void;
31+
unref(): void;
32+
close(callback?: () => void): void;
33+
}
34+
35+
interface ProcessConstants {
36+
kProcessFlagDetached: number;
37+
kProcessFlagWindowsHide: number;
38+
kProcessFlagWindowsVerbatimArguments: number;
39+
}
40+
}
41+
42+
export interface ProcessWrapBinding {
43+
Process: typeof InternalProcessWrapBinding.Process;
44+
constants: InternalProcessWrapBinding.ProcessConstants;
45+
}

0 commit comments

Comments
 (0)