child_process: pass spawn options to the binding positionally · nodejs/node@2ab9848 · GitHub
Skip to content

Commit 2ab9848

Browse files
anonrigaduh95
authored andcommitted
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> PR-URL: #63930 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Daniel Lemire <daniel@lemire.me>
1 parent 373ec2d commit 2ab9848

4 files changed

Lines changed: 102 additions & 76 deletions

File tree

lib/internal/child_process.js

Lines changed: 19 additions & 2 deletions

src/process_wrap.cc

Lines changed: 38 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,18 @@ 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+
enum ProcessFlags : uint32_t {
58+
kProcessFlagNone = 0,
59+
kProcessFlagDetached = 1 << 0,
60+
kProcessFlagWindowsHide = 1 << 1,
61+
kProcessFlagWindowsVerbatimArguments = 1 << 2,
62+
};
63+
5664
class ProcessWrap : public HandleWrap {
5765
public:
5866
static void Initialize(Local<Object> target,
@@ -71,6 +79,12 @@ class ProcessWrap : public HandleWrap {
7179
SetProtoMethod(isolate, constructor, "kill", Kill);
7280

7381
SetConstructorFunction(context, target, "Process", constructor);
82+
83+
Local<Object> constants = Object::New(isolate);
84+
NODE_DEFINE_CONSTANT(constants, kProcessFlagDetached);
85+
NODE_DEFINE_CONSTANT(constants, kProcessFlagWindowsHide);
86+
NODE_DEFINE_CONSTANT(constants, kProcessFlagWindowsVerbatimArguments);
87+
target->Set(context, env->constants_string(), constants).Check();
7488
}
7589

7690
static void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
@@ -118,14 +132,9 @@ class ProcessWrap : public HandleWrap {
118132

119133
static Maybe<void> ParseStdioOptions(
120134
Environment* env,
121-
Local<Object> js_options,
135+
Local<Value> stdios_val,
122136
std::vector<uv_stdio_container_t>* options_stdio) {
123137
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-
}
129138
if (!stdios_val->IsArray()) {
130139
THROW_ERR_INVALID_ARG_TYPE(env, "options.stdio must be an array");
131140
return Nothing<void>();
@@ -188,46 +197,30 @@ class ProcessWrap : public HandleWrap {
188197
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.This());
189198
int err = 0;
190199

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-
197200
uv_process_options_t options;
198201
memset(&options, 0, sizeof(uv_process_options_t));
199202

200203
options.exit_cb = OnExit;
201204

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);
205+
// args[0] file
206+
CHECK(args[0]->IsString());
207+
node::Utf8Value file(env->isolate(), args[0]);
209208
options.file = *file;
210209

211210
THROW_IF_INSUFFICIENT_PERMISSIONS(
212211
env, permission::PermissionScope::kChildProcess, file.ToStringView());
213212

214-
// options.uid
215-
Local<Value> uid_v;
216-
if (!js_options->Get(context, env->uid_string()).ToLocal(&uid_v)) {
217-
return;
218-
}
213+
// args[6] uid
214+
Local<Value> uid_v = args[6];
219215
if (!uid_v->IsUndefined() && !uid_v->IsNull()) {
220216
CHECK(uid_v->IsInt32());
221217
const int32_t uid = uid_v.As<Int32>()->Value();
222218
options.flags |= UV_PROCESS_SETUID;
223219
options.uid = static_cast<uv_uid_t>(uid);
224220
}
225221

226-
// options.gid
227-
Local<Value> gid_v;
228-
if (!js_options->Get(context, env->gid_string()).ToLocal(&gid_v)) {
229-
return;
230-
}
222+
// args[7] gid
223+
Local<Value> gid_v = args[7];
231224
if (!gid_v->IsUndefined() && !gid_v->IsNull()) {
232225
CHECK(gid_v->IsInt32());
233226
const int32_t gid = gid_v.As<Int32>()->Value();
@@ -244,15 +237,11 @@ class ProcessWrap : public HandleWrap {
244237
err = UV_EINVAL;
245238
#endif
246239

247-
// options.args
248-
Local<Value> argv_v;
249-
if (!js_options->Get(context, env->args_string()).ToLocal(&argv_v)) {
250-
return;
251-
}
240+
// args[1] args
252241
std::vector<char*> options_args;
253242
std::vector<std::string> args_vals;
254-
if (argv_v->IsArray()) {
255-
Local<Array> js_argv = argv_v.As<Array>();
243+
if (args[1]->IsArray()) {
244+
Local<Array> js_argv = args[1].As<Array>();
256245
int argc = js_argv->Length();
257246
CHECK_LT(argc, INT_MAX); // Check for overflow.
258247
args_vals.reserve(argc);
@@ -273,26 +262,18 @@ class ProcessWrap : public HandleWrap {
273262
options.args = options_args.data();
274263
}
275264

276-
// options.cwd
277-
Local<Value> cwd_v;
278-
if (!js_options->Get(context, env->cwd_string()).ToLocal(&cwd_v)) {
279-
return;
280-
}
265+
// args[2] cwd
281266
node::Utf8Value cwd(env->isolate(),
282-
cwd_v->IsString() ? cwd_v : Local<Value>());
267+
args[2]->IsString() ? args[2] : Local<Value>());
283268
if (cwd.length() > 0) {
284269
options.cwd = *cwd;
285270
}
286271

287-
// options.env
288-
Local<Value> env_v;
289-
if (!js_options->Get(context, env->env_pairs_string()).ToLocal(&env_v)) {
290-
return;
291-
}
272+
// args[3] envPairs
292273
std::vector<char*> options_env;
293274
std::vector<std::string> env_vals;
294-
if (env_v->IsArray()) {
295-
Local<Array> env_opt = env_v.As<Array>();
275+
if (args[3]->IsArray()) {
276+
Local<Array> env_opt = args[3].As<Array>();
296277
int envc = env_opt->Length();
297278
CHECK_LT(envc, INT_MAX); // Check for overflow.
298279
env_vals.reserve(envc);
@@ -313,48 +294,31 @@ class ProcessWrap : public HandleWrap {
313294
options.env = options_env.data();
314295
}
315296

316-
// options.stdio
297+
// args[4] stdio
317298
std::vector<uv_stdio_container_t> options_stdio;
318-
if (ParseStdioOptions(env, js_options, &options_stdio).IsNothing()) {
299+
if (ParseStdioOptions(env, args[4], &options_stdio).IsNothing()) {
319300
return;
320301
}
321302
options.stdio = options_stdio.data();
322303
options.stdio_count = options_stdio.size();
323304

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-
}
305+
// args[5] flags (detached, windowsHide, windowsVerbatimArguments)
306+
CHECK(args[5]->IsUint32());
307+
const uint32_t flags = args[5].As<Uint32>()->Value();
330308

331-
if (hide_v->IsTrue()) {
309+
if (flags & kProcessFlagWindowsHide) {
332310
options.flags |= UV_PROCESS_WINDOWS_HIDE;
333311
}
334312

335313
if (env->hide_console_windows()) {
336314
options.flags |= UV_PROCESS_WINDOWS_HIDE_CONSOLE;
337315
}
338316

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()) {
317+
if (flags & kProcessFlagWindowsVerbatimArguments) {
347318
options.flags |= UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS;
348319
}
349320

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()) {
321+
if (flags & kProcessFlagDetached) {
358322
options.flags |= UV_PROCESS_DETACHED;
359323
}
360324

typings/globals.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { MessagingBinding } from './internalBinding/messaging';
1818
import { OptionsBinding } from './internalBinding/options';
1919
import { OSBinding } from './internalBinding/os';
2020
import { ProcessBinding } from './internalBinding/process';
21+
import { ProcessWrapBinding } from './internalBinding/process_wrap';
2122
import { SeaBinding } from './internalBinding/sea';
2223
import { SerdesBinding } from './internalBinding/serdes';
2324
import { StringDecoderBinding } from './internalBinding/string_decoder';
@@ -55,6 +56,7 @@ interface InternalBindingMap {
5556
options: OptionsBinding;
5657
os: OSBinding;
5758
process: ProcessBinding;
59+
process_wrap: ProcessWrapBinding;
5860
sea: SeaBinding;
5961
serdes: SerdesBinding;
6062
string_decoder: StringDecoderBinding;
Lines changed: 43 additions & 0 deletions

0 commit comments

Comments
 (0)