src: inline ProcessCliArgs in the Environment constructor

Inline `ProcessCliArgs()` in the `Environment` constructor, and emit the `Environment` creation trace events with the arguments earlier. Remove the unused arguments passed to `CreateProcessObject()` since these are now attached to process in `PatchProcessObject()` during pre-execution instead.
nodejs · joyeecheung · May 20, 2019 · May 20, 2019 · May 20, 2019 · May 20, 2019
commit 7de0af70050435410c41b3c15c5d08e90f83f65f
diff --git a/src/api/environment.cc b/src/api/environment.cc
@@ -244,11 +244,12 @@ Environment* CreateEnvironment(IsolateData* isolate_data,
   Environment* env = new Environment(
       isolate_data,
       context,
+      args,
+      exec_args,
       static_cast<Environment::Flags>(Environment::kIsMainThread |
                                       Environment::kOwnsProcessState |
                                       Environment::kOwnsInspector));
   env->InitializeLibuv(per_process::v8_is_profiling);
-  env->ProcessCliArgs(args, exec_args);
   if (RunBootstrapping(env).IsEmpty()) {
     return nullptr;
   }

diff --git a/src/env.cc b/src/env.cc
@@ -236,13 +236,17 @@ uint64_t Environment::AllocateThreadId() {
 
 Environment::Environment(IsolateData* isolate_data,
                          Local<Context> context,
+                         const std::vector<std::string>& args,
+                         const std::vector<std::string>& exec_args,
                          Flags flags,
                          uint64_t thread_id)
     : isolate_(context->GetIsolate()),
       isolate_data_(isolate_data),
       immediate_info_(context->GetIsolate()),
       tick_info_(context->GetIsolate()),
       timer_base_(uv_now(isolate_data->event_loop())),
+      exec_argv_(exec_args),
+      argv_(args),
       should_abort_on_uncaught_toggle_(isolate_, 1),
       stream_base_state_(isolate_, StreamBase::kNumStreamBaseStateFields),
       flags_(flags),
@@ -305,6 +309,22 @@ Environment::Environment(IsolateData* isolate_data,
       performance::NODE_PERFORMANCE_MILESTONE_V8_START,
       performance::performance_v8_start);
 
+  if (*TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(
+          TRACING_CATEGORY_NODE1(environment)) != 0) {
+    auto traced_value = tracing::TracedValue::Create();
+    traced_value->BeginArray("args");
+    for (const std::string& arg : args) traced_value->AppendString(arg);
+    traced_value->EndArray();
+    traced_value->BeginArray("exec_args");
+    for (const std::string& arg : exec_args) traced_value->AppendString(arg);
+    traced_value->EndArray();
+    TRACE_EVENT_NESTABLE_ASYNC_BEGIN1(TRACING_CATEGORY_NODE1(environment),
+                                      "Environment",
+                                      this,
+                                      "args",
+                                      std::move(traced_value));
+  }
+
   // By default, always abort when --abort-on-uncaught-exception was passed.
   should_abort_on_uncaught_toggle_[0] = 1;
 
@@ -317,6 +337,8 @@ Environment::Environment(IsolateData* isolate_data,
   if (options_->no_force_async_hooks_checks) {
     async_hooks_.no_force_checks();
   }
+
+  set_process_object(node::CreateProcessObject(this).ToLocalChecked());
 }
 
 CompileFnEntry::CompileFnEntry(Environment* env, uint32_t id)
@@ -433,35 +455,6 @@ void Environment::ExitEnv() {
   isolate_->TerminateExecution();
 }
 
-MaybeLocal<Object> Environment::ProcessCliArgs(
-    const std::vector<std::string>& args,
-    const std::vector<std::string>& exec_args) {
-  argv_ = args;
-  exec_argv_ = exec_args;
-
-  if (*TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(
-          TRACING_CATEGORY_NODE1(environment)) != 0) {
-    auto traced_value = tracing::TracedValue::Create();
-    traced_value->BeginArray("args");
-    for (const std::string& arg : args) traced_value->AppendString(arg);
-    traced_value->EndArray();
-    traced_value->BeginArray("exec_args");
-    for (const std::string& arg : exec_args) traced_value->AppendString(arg);
-    traced_value->EndArray();
-    TRACE_EVENT_NESTABLE_ASYNC_BEGIN1(TRACING_CATEGORY_NODE1(environment),
-                                      "Environment",
-                                      this,
-                                      "args",
-                                      std::move(traced_value));
-  }
-
-  Local<Object> process_object =
-      node::CreateProcessObject(this, args, exec_args)
-          .FromMaybe(Local<Object>());
-  set_process_object(process_object);
-  return process_object;
-}
-
 void Environment::RegisterHandleCleanups() {
   HandleCleanupCb close_and_finish = [](Environment* env, uv_handle_t* handle,
                                         void* arg) {

diff --git a/src/env.h b/src/env.h
@@ -821,14 +821,13 @@ class Environment : public MemoryRetainer {
 
   Environment(IsolateData* isolate_data,
               v8::Local<v8::Context> context,
+              const std::vector<std::string>& args,
+              const std::vector<std::string>& exec_args,
               Flags flags = Flags(),
               uint64_t thread_id = kNoThreadId);
   ~Environment();
 
   void InitializeLibuv(bool start_profiler_idle_notifier);
-  v8::MaybeLocal<v8::Object> ProcessCliArgs(
-      const std::vector<std::string>& args,
-      const std::vector<std::string>& exec_args);
   inline const std::vector<std::string>& exec_argv();
   inline const std::vector<std::string>& argv();
 

diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc
@@ -188,11 +188,12 @@ std::unique_ptr<Environment> NodeMainInstance::CreateMainEnvironment(
   std::unique_ptr<Environment> env = std::make_unique<Environment>(
       isolate_data_.get(),
       context,
+      args_,
+      exec_args_,
       static_cast<Environment::Flags>(Environment::kIsMainThread |
                                       Environment::kOwnsProcessState |
                                       Environment::kOwnsInspector));
   env->InitializeLibuv(per_process::v8_is_profiling);
-  env->ProcessCliArgs(args_, exec_args_);
 
 #if HAVE_INSPECTOR && NODE_USE_V8_PLATFORM
   CHECK(!env->inspector_agent()->IsListening());

diff --git a/src/node_process.h b/src/node_process.h
@@ -31,10 +31,7 @@ v8::Maybe<bool> ProcessEmitDeprecationWarning(Environment* env,
                                               const char* warning,
                                               const char* deprecation_code);
 
-v8::MaybeLocal<v8::Object> CreateProcessObject(
-    Environment* env,
-    const std::vector<std::string>& args,
-    const std::vector<std::string>& exec_args);
+v8::MaybeLocal<v8::Object> CreateProcessObject(Environment* env);
 void PatchProcessObject(const v8::FunctionCallbackInfo<v8::Value>& args);
 
 namespace task_queue {

diff --git a/src/node_process_object.cc b/src/node_process_object.cc
@@ -68,10 +68,7 @@ static void GetParentProcessId(Local<Name> property,
   info.GetReturnValue().Set(uv_os_getppid());
 }
 
-MaybeLocal<Object> CreateProcessObject(
-    Environment* env,
-    const std::vector<std::string>& args,
-    const std::vector<std::string>& exec_args) {
+MaybeLocal<Object> CreateProcessObject(Environment* env) {
   Isolate* isolate = env->isolate();
   EscapableHandleScope scope(isolate);
   Local<Context> context = env->context();

diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -255,6 +255,8 @@ void Worker::Run() {
         // public API.
         env_.reset(new Environment(data.isolate_data_.get(),
                                    context,
+                                   std::move(argv_),
+                                   std::move(exec_argv_),
                                    Environment::kNoFlags,
                                    thread_id_));
         CHECK_NOT_NULL(env_);
@@ -263,7 +265,6 @@ void Worker::Run() {
         env_->set_worker_context(this);
 
         env_->InitializeLibuv(profiler_idle_notifier_started_);
-        env_->ProcessCliArgs(std::move(argv_), std::move(exec_argv_));
       }
       {
         Mutex::ScopedLock lock(mutex_);