worker: pre-allocate thread id

Allocate a thread id before actually creating the Environment instance. PR-URL: nodejs#26011 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
SimenB · Feb 11, 2019 · 58ba8bf · 58ba8bf
1 parent e11388b
commit 58ba8bf
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 6 deletions.
diff --git a/src/env.cc b/src/env.cc
@@ -169,9 +169,14 @@ void Environment::TrackingTraceStateObserver::UpdateTraceCategoryState() {
 
 static std::atomic<uint64_t> next_thread_id{0};
 
+uint64_t Environment::AllocateThreadId() {
+  return next_thread_id++;
+}
+
 Environment::Environment(IsolateData* isolate_data,
                          Local<Context> context,
-                         Flags flags)
+                         Flags flags,
+                         uint64_t thread_id)
     : isolate_(context->GetIsolate()),
       isolate_data_(isolate_data),
       immediate_info_(context->GetIsolate()),
@@ -181,7 +186,7 @@ Environment::Environment(IsolateData* isolate_data,
       trace_category_state_(isolate_, kTraceCategoryCount),
       stream_base_state_(isolate_, StreamBase::kNumStreamBaseStateFields),
       flags_(flags),
-      thread_id_(next_thread_id++),
+      thread_id_(thread_id == kNoThreadId ? AllocateThreadId() : thread_id),
       fs_stats_field_array_(isolate_, kFsStatsBufferLength),
       fs_stats_field_bigint_array_(isolate_, kFsStatsBufferLength),
       context_(context->GetIsolate(), context) {

diff --git a/src/env.h b/src/env.h
@@ -618,7 +618,8 @@ class Environment {
 
   Environment(IsolateData* isolate_data,
               v8::Local<v8::Context> context,
-              Flags flags = Flags());
+              Flags flags = Flags(),
+              uint64_t thread_id = kNoThreadId);
   ~Environment();
 
   void Start(bool start_profiler_idle_notifier);
@@ -763,6 +764,9 @@ class Environment {
   inline bool has_run_bootstrapping_code() const;
   inline void set_has_run_bootstrapping_code(bool has_run_bootstrapping_code);
 
+  static uint64_t AllocateThreadId();
+  static constexpr uint64_t kNoThreadId = -1;
+
   inline bool is_main_thread() const;
   inline bool owns_process_state() const;
   inline bool owns_inspector() const;

diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -71,7 +71,8 @@ Worker::Worker(Environment* env,
                Local<Object> wrap,
                const std::string& url,
                std::shared_ptr<PerIsolateOptions> per_isolate_opts)
-    : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_WORKER), url_(url) {
+    : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_WORKER), url_(url),
+      thread_id_(Environment::AllocateThreadId()) {
   Debug(this, "Creating new worker instance at %p", static_cast<void*>(this));
 
   // Set up everything that needs to be set up in the parent environment.
@@ -114,11 +115,11 @@ Worker::Worker(Environment* env,
     Context::Scope context_scope(context);
 
     // TODO(addaleax): Use CreateEnvironment(), or generally another public API.
-    env_.reset(new Environment(isolate_data_.get(), context));
+    env_.reset(new Environment(
+        isolate_data_.get(), context, Flags::kNoFlags, thread_id_));
     CHECK_NOT_NULL(env_);
     env_->set_abort_on_uncaught_exception(false);
     env_->set_worker_context(this);
-    thread_id_ = env_->thread_id();
 
     env_->Start(env->profiler_idle_notifier_started());
     env_->ProcessCliArgs(std::vector<std::string>{},