FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: reorganize inspector and diagnostics initialization · nodejs/node@c20c6e5 · GitHub

/ node Public

Commit c20c6e5

Browse files
joyeecheung
authored and
BridgeAR
committed
src: reorganize inspector and diagnostics initialization
- Split the initialization of the inspector and other diagnostics into `Environment::InitializeInspector()` and `Environment::InitializeDiagnostics()` - these need to be reinitialized separately after snapshot deserialization. - Do not store worker url alongside the inspector parent handle, instead just get it from the handle. - Rename `Worker::profiler_idle_notifier_started_` to `Worker::start_profiler_idle_notifier_` because it stores the state inherited from the parent env to use for initializing itself. PR-URL: #27539 Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent c086736 commit c20c6e5

7 files changed

Lines changed: 74 additions & 55 deletions

File tree

‎src/env.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,8 +349,6 @@ Environment::Environment(IsolateData* isolate_data,
349349
credentials::SafeGetenv("NODE_DEBUG_NATIVE", &debug_cats, this);
350350
set_debug_categories(debug_cats, true);
351351

352-
isolate()->GetHeapProfiler()->AddBuildEmbedderGraphCallback(
353-
BuildEmbedderGraph, this);
354352
if (options_->no_force_async_hooks_checks) {
355353
async_hooks_.no_force_checks();
356354
}

‎src/env.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ class V8CoverageConnection;
7575
class V8CpuProfilerConnection;
7676
class V8HeapProfilerConnection;
7777
} // namespace profiler
78+
79+
namespace inspector {
80+
class ParentInspectorHandle;
81+
}
7882
#endif // HAVE_INSPECTOR
7983

8084
namespace worker {
@@ -797,6 +801,13 @@ class Environment : public MemoryRetainer {
797801
void MemoryInfo(MemoryTracker* tracker) const override;
798802

799803
void CreateProperties();
804+
// Should be called before InitializeInspector()
805+
void InitializeDiagnostics();
806+
#if HAVE_INSPECTOR && NODE_USE_V8_PLATFORM
807+
// If the environment is created for a worker, pass parent_handle and
808+
// the ownership if transferred into the Environment.
809+
int InitializeInspector(inspector::ParentInspectorHandle* parent_handle);
810+
#endif
800811

801812
inline size_t async_callback_scope_depth() const;
802813
inline void PushAsyncCallbackScope();

‎src/inspector/worker_inspector.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ class ParentInspectorHandle {
6060
bool WaitForConnect() {
6161
return wait_;
6262
}
63+
const std::string& url() const { return url_; }
6364

6465
private:
6566
int id_;

‎src/node.cc‎

Lines changed: 50 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
#endif
4848

4949
#if HAVE_INSPECTOR
50+
#include "inspector_agent.h"
5051
#include "inspector_io.h"
5152
#endif
5253

@@ -59,6 +60,10 @@
5960
#endif // NODE_USE_V8_PLATFORM
6061
#include "v8-profiler.h"
6162

63+
#if HAVE_INSPECTOR
64+
#include "inspector/worker_inspector.h" // ParentInspectorHandle
65+
#endif
66+
6267
#ifdef NODE_ENABLE_VTUNE_PROFILING
6368
#include "../deps/v8/src/third_party/vtune/v8-vtune.h"
6469
#endif
@@ -125,7 +130,6 @@ using v8::Local;
125130
using v8::Maybe;
126131
using v8::MaybeLocal;
127132
using v8::Object;
128-
using v8::Script;
129133
using v8::String;
130134
using v8::Undefined;
131135
using v8::V8;
@@ -222,24 +226,61 @@ MaybeLocal<Value> ExecuteBootstrapper(Environment* env,
222226
return scope.EscapeMaybe(result);
223227
}
224228

229+
#if HAVE_INSPECTOR && NODE_USE_V8_PLATFORM
230+
int Environment::InitializeInspector(
231+
inspector::ParentInspectorHandle* parent_handle) {
232+
std::string inspector_path;
233+
if (parent_handle != nullptr) {
234+
DCHECK(!is_main_thread());
235+
inspector_path = parent_handle->url();
236+
inspector_agent_->SetParentHandle(
237+
std::unique_ptr<inspector::ParentInspectorHandle>(parent_handle));
238+
} else {
239+
inspector_path = argv_.size() > 1 ? argv_[1].c_str() : "";
240+
}
241+
242+
CHECK(!inspector_agent_->IsListening());
243+
// Inspector agent can't fail to start, but if it was configured to listen
244+
// right away on the websocket port and fails to bind/etc, this will return
245+
// false.
246+
inspector_agent_->Start(inspector_path,
247+
options_->debug_options(),
248+
inspector_host_port(),
249+
is_main_thread());
250+
if (options_->debug_options().inspector_enabled &&
251+
!inspector_agent_->IsListening()) {
252+
return 12; // Signal internal error
253+
}
254+
255+
profiler::StartProfilers(this);
256+
257+
if (options_->debug_options().break_node_first_line) {
258+
inspector_agent_->PauseOnNextJavascriptStatement("Break at bootstrap");
259+
}
260+
261+
return 0;
262+
}
263+
#endif // HAVE_INSPECTOR && NODE_USE_V8_PLATFORM
264+
265+
void Environment::InitializeDiagnostics() {
266+
isolate_->GetHeapProfiler()->AddBuildEmbedderGraphCallback(
267+
Environment::BuildEmbedderGraph, this);
268+
269+
#if defined HAVE_DTRACE || defined HAVE_ETW
270+
InitDTrace(this);
271+
#endif
272+
}
273+
225274
MaybeLocal<Value> RunBootstrapping(Environment* env) {
226275
CHECK(!env->has_run_bootstrapping_code());
227276

228277
EscapableHandleScope scope(env->isolate());
229278
Isolate* isolate = env->isolate();
230279
Local<Context> context = env->context();
231280

232-
#if HAVE_INSPECTOR
233-
profiler::StartProfilers(env);
234-
#endif // HAVE_INSPECTOR
235281

236282
// Add a reference to the global object
237283
Local<Object> global = context->Global();
238-
239-
#if defined HAVE_DTRACE || defined HAVE_ETW
240-
InitDTrace(env);
241-
#endif
242-
243284
Local<Object> process = env->process_object();
244285

245286
// Setting global properties for the bootstrappers to use:
@@ -249,12 +290,6 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
249290
global->Set(context, FIXED_ONE_BYTE_STRING(env->isolate(), "global"), global)
250291
.Check();
251292

252-
#if HAVE_INSPECTOR
253-
if (env->options()->debug_options().break_node_first_line) {
254-
env->inspector_agent()->PauseOnNextJavascriptStatement(
255-
"Break at bootstrap");
256-
}
257-
#endif // HAVE_INSPECTOR
258293

259294
// Create binding loaders
260295
std::vector<Local<String>> loaders_params = {

‎src/node_main_instance.cc‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -194,27 +194,16 @@ std::unique_ptr<Environment> NodeMainInstance::CreateMainEnvironment(
194194
Environment::kOwnsProcessState |
195195
Environment::kOwnsInspector));
196196
env->InitializeLibuv(per_process::v8_is_profiling);
197+
env->InitializeDiagnostics();
197198

199+
// TODO(joyeecheung): when we snapshot the bootstrapped context,
200+
// the inspector and diagnostics setup should after after deserialization.
198201
#if HAVE_INSPECTOR && NODE_USE_V8_PLATFORM
199-
CHECK(!env->inspector_agent()->IsListening());
200-
// Inspector agent can't fail to start, but if it was configured to listen
201-
// right away on the websocket port and fails to bind/etc, this will return
202-
// false.
203-
env->inspector_agent()->Start(args_.size() > 1 ? args_[1].c_str() : "",
204-
env->options()->debug_options(),
205-
env->inspector_host_port(),
206-
true);
207-
if (env->options()->debug_options().inspector_enabled &&
208-
!env->inspector_agent()->IsListening()) {
209-
*exit_code = 12; // Signal internal error.
202+
*exit_code = env->InitializeInspector(nullptr);
203+
#endif
204+
if (*exit_code != 0) {
210205
return env;
211206
}
212-
#else
213-
// inspector_enabled can't be true if !HAVE_INSPECTOR or
214-
// !NODE_USE_V8_PLATFORM
215-
// - the option parser should not allow that.
216-
CHECK(!env->options()->debug_options().inspector_enabled);
217-
#endif // HAVE_INSPECTOR && NODE_USE_V8_PLATFORM
218207

219208
if (RunBootstrapping(env.get()).IsEmpty()) {
220209
*exit_code = 1;

‎src/node_worker.cc‎

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,6 @@ namespace worker {
4242
namespace {
4343

4444
#if NODE_USE_V8_PLATFORM && HAVE_INSPECTOR
45-
void StartWorkerInspector(
46-
Environment* child,
47-
std::unique_ptr<inspector::ParentInspectorHandle> parent_handle,
48-
const std::string& url) {
49-
child->inspector_agent()->SetParentHandle(std::move(parent_handle));
50-
child->inspector_agent()->Start(url,
51-
child->options()->debug_options(),
52-
child->inspector_host_port(),
53-
false);
54-
}
55-
5645
void WaitForWorkerInspectorToStop(Environment* child) {
5746
child->inspector_agent()->WaitForDisconnect();
5847
child->inspector_agent()->Stop();
@@ -67,11 +56,10 @@ Worker::Worker(Environment* env,
6756
std::shared_ptr<PerIsolateOptions> per_isolate_opts,
6857
std::vector<std::string>&& exec_argv)
6958
: AsyncWrap(env, wrap, AsyncWrap::PROVIDER_WORKER),
70-
url_(url),
7159
per_isolate_opts_(per_isolate_opts),
7260
exec_argv_(exec_argv),
7361
platform_(env->isolate_data()->platform()),
74-
profiler_idle_notifier_started_(env->profiler_idle_notifier_started()),
62+
start_profiler_idle_notifier_(env->profiler_idle_notifier_started()),
7563
thread_id_(Environment::AllocateThreadId()),
7664
env_vars_(env->env_vars()) {
7765
Debug(this, "Creating new worker instance with thread id %llu", thread_id_);
@@ -265,7 +253,7 @@ void Worker::Run() {
265253
env_->set_abort_on_uncaught_exception(false);
266254
env_->set_worker_context(this);
267255

268-
env_->InitializeLibuv(profiler_idle_notifier_started_);
256+
env_->InitializeLibuv(start_profiler_idle_notifier_);
269257
}
270258
{
271259
Mutex::ScopedLock lock(mutex_);
@@ -275,13 +263,11 @@ void Worker::Run() {
275263
Debug(this, "Created Environment for worker with id %llu", thread_id_);
276264
if (is_stopped()) return;
277265
{
266+
env_->InitializeDiagnostics();
278267
#if NODE_USE_V8_PLATFORM && HAVE_INSPECTOR
279-
StartWorkerInspector(env_.get(),
280-
std::move(inspector_parent_handle_),
281-
url_);
282-
#endif
268+
env_->InitializeInspector(inspector_parent_handle_.release());
283269
inspector_started = true;
284-
270+
#endif
285271
HandleScope handle_scope(isolate_);
286272
AsyncCallbackScope callback_scope(env_.get());
287273
env_->async_hooks()->push_async_ids(1, 0);

‎src/node_worker.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,14 @@ class Worker : public AsyncWrap {
5454
private:
5555
void OnThreadStopped();
5656
void CreateEnvMessagePort(Environment* env);
57-
const std::string url_;
5857

5958
std::shared_ptr<PerIsolateOptions> per_isolate_opts_;
6059
std::vector<std::string> exec_argv_;
6160
std::vector<std::string> argv_;
6261

6362
MultiIsolatePlatform* platform_;
6463
v8::Isolate* isolate_ = nullptr;
65-
bool profiler_idle_notifier_started_;
64+
bool start_profiler_idle_notifier_;
6665
uv_thread_t tid_;
6766

6867
#if NODE_USE_V8_PLATFORM && HAVE_INSPECTOR

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL