diff --git a/lib/internal/bootstrap/switches/is_main_thread.js b/lib/internal/bootstrap/switches/is_main_thread.js index be145e84dc27df..08623898edafac 100644 --- a/lib/internal/bootstrap/switches/is_main_thread.js +++ b/lib/internal/bootstrap/switches/is_main_thread.js @@ -6,8 +6,12 @@ const rawMethods = internalBinding('process_methods'); // TODO(joyeecheung): deprecate and remove these underscore methods process._debugProcess = rawMethods._debugProcess; process._debugEnd = rawMethods._debugEnd; -process._startProfilerIdleNotifier = rawMethods._startProfilerIdleNotifier; -process._stopProfilerIdleNotifier = rawMethods._stopProfilerIdleNotifier; + +// See the discussion in https://github.com/nodejs/node/issues/19009 and +// https://github.com/nodejs/node/pull/34010 for why these are no-ops. +// Five word summary: they were broken beyond repair. +process._startProfilerIdleNotifier = () => {}; +process._stopProfilerIdleNotifier = () => {}; function defineStream(name, getter) { ObjectDefineProperty(process, name, { diff --git a/lib/internal/bootstrap/switches/is_not_main_thread.js b/lib/internal/bootstrap/switches/is_not_main_thread.js index 852352eead0d73..379ad0a587a22a 100644 --- a/lib/internal/bootstrap/switches/is_not_main_thread.js +++ b/lib/internal/bootstrap/switches/is_not_main_thread.js @@ -4,8 +4,6 @@ const { ObjectDefineProperty } = primordials; delete process._debugProcess; delete process._debugEnd; -delete process._startProfilerIdleNotifier; -delete process._stopProfilerIdleNotifier; function defineStream(name, getter) { ObjectDefineProperty(process, name, { diff --git a/src/api/environment.cc b/src/api/environment.cc index 20408c79f858be..90cb2be2e6edc8 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -420,7 +420,7 @@ MaybeLocal LoadEnvironment( Environment* env, StartExecutionCallback cb, std::unique_ptr removeme) { - env->InitializeLibuv(per_process::v8_is_profiling); + env->InitializeLibuv(); env->InitializeDiagnostics(); return StartExecution(env, cb); diff --git a/src/env-inl.h b/src/env-inl.h index 2f0573de210fab..5126b18e83daa5 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -378,10 +378,6 @@ inline Environment* Environment::GetThreadLocalEnv() { return static_cast(uv_key_get(&thread_local_env)); } -inline bool Environment::profiler_idle_notifier_started() const { - return profiler_idle_notifier_started_; -} - inline v8::Isolate* Environment::isolate() const { return isolate_; } diff --git a/src/env.cc b/src/env.cc index 4030df9ec90f57..991ba3cf98d4a2 100644 --- a/src/env.cc +++ b/src/env.cc @@ -485,7 +485,7 @@ Environment::~Environment() { CHECK_EQ(base_object_count_, 0); } -void Environment::InitializeLibuv(bool start_profiler_idle_notifier) { +void Environment::InitializeLibuv() { HandleScope handle_scope(isolate()); Context::Scope context_scope(context()); @@ -499,17 +499,6 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) { uv_check_start(immediate_check_handle(), CheckImmediate); - // Inform V8's CPU profiler when we're idle. The profiler is sampling-based - // but not all samples are created equal; mark the wall clock time spent in - // epoll_wait() and friends so profiling tools can filter it out. The samples - // still end up in v8.log but with state=IDLE rather than state=EXTERNAL. - // TODO(bnoordhuis) Depends on a libuv implementation detail that we should - // probably fortify in the API contract, namely that the last started prepare - // or check watcher runs first. It's not 100% foolproof; if an add-on starts - // a prepare or check watcher after us, any samples attributed to its callback - // will be recorded with state=IDLE. - uv_prepare_init(event_loop(), &idle_prepare_handle_); - uv_check_init(event_loop(), &idle_check_handle_); uv_async_init( event_loop(), &task_queues_async_, @@ -518,8 +507,6 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) { &Environment::task_queues_async_, async); env->RunAndClearNativeImmediates(); }); - uv_unref(reinterpret_cast(&idle_prepare_handle_)); - uv_unref(reinterpret_cast(&idle_check_handle_)); uv_unref(reinterpret_cast(&task_queues_async_)); { @@ -536,10 +523,6 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) { // the one environment per process setup, but will be called in // FreeEnvironment. RegisterHandleCleanups(); - - if (start_profiler_idle_notifier) { - StartProfilerIdleNotifier(); - } } void Environment::ExitEnv() { @@ -567,8 +550,6 @@ void Environment::RegisterHandleCleanups() { register_handle(reinterpret_cast(timer_handle())); register_handle(reinterpret_cast(immediate_check_handle())); register_handle(reinterpret_cast(immediate_idle_handle())); - register_handle(reinterpret_cast(&idle_prepare_handle_)); - register_handle(reinterpret_cast(&idle_check_handle_)); register_handle(reinterpret_cast(&task_queues_async_)); } @@ -600,29 +581,6 @@ void Environment::CleanupHandles() { } } -void Environment::StartProfilerIdleNotifier() { - if (profiler_idle_notifier_started_) - return; - - profiler_idle_notifier_started_ = true; - - uv_prepare_start(&idle_prepare_handle_, [](uv_prepare_t* handle) { - Environment* env = ContainerOf(&Environment::idle_prepare_handle_, handle); - env->isolate()->SetIdle(true); - }); - - uv_check_start(&idle_check_handle_, [](uv_check_t* handle) { - Environment* env = ContainerOf(&Environment::idle_check_handle_, handle); - env->isolate()->SetIdle(false); - }); -} - -void Environment::StopProfilerIdleNotifier() { - profiler_idle_notifier_started_ = false; - uv_prepare_stop(&idle_prepare_handle_); - uv_check_stop(&idle_check_handle_); -} - void Environment::PrintSyncTrace() const { if (!trace_sync_io_) return; diff --git a/src/env.h b/src/env.h index 0a647ecea8757b..4533f8b8978307 100644 --- a/src/env.h +++ b/src/env.h @@ -879,7 +879,7 @@ class Environment : public MemoryRetainer { ThreadId thread_id); ~Environment() override; - void InitializeLibuv(bool start_profiler_idle_notifier); + void InitializeLibuv(); inline const std::vector& exec_argv(); inline const std::vector& argv(); const std::string& exec_path() const; @@ -909,10 +909,6 @@ class Environment : public MemoryRetainer { inline void AssignToContext(v8::Local context, const ContextInfo& info); - void StartProfilerIdleNotifier(); - void StopProfilerIdleNotifier(); - inline bool profiler_idle_notifier_started() const; - inline v8::Isolate* isolate() const; inline uv_loop_t* event_loop() const; inline void TryLoadAddon( @@ -1234,11 +1230,8 @@ class Environment : public MemoryRetainer { uv_timer_t timer_handle_; uv_check_t immediate_check_handle_; uv_idle_t immediate_idle_handle_; - uv_prepare_t idle_prepare_handle_; - uv_check_t idle_check_handle_; uv_async_t task_queues_async_; int64_t task_queues_async_refs_ = 0; - bool profiler_idle_notifier_started_ = false; AsyncHooks async_hooks_; ImmediateInfo immediate_info_; diff --git a/src/node.cc b/src/node.cc index b3b69922cbe8ab..1c776c79fc6887 100644 --- a/src/node.cc +++ b/src/node.cc @@ -148,8 +148,6 @@ bool v8_initialized = false; // node_internals.h // process-relative uptime base in nanoseconds, initialized in node::Start() uint64_t node_start_time; -// Tells whether --prof is passed. -bool v8_is_profiling = false; // node_v8_platform-inl.h struct V8Platform v8_platform; @@ -789,19 +787,11 @@ int ProcessGlobalArgs(std::vector* args, env_opts->abort_on_uncaught_exception = true; } - // TODO(bnoordhuis) Intercept --prof arguments and start the CPU profiler - // manually? That would give us a little more control over its runtime - // behavior but it could also interfere with the user's intentions in ways - // we fail to anticipate. Dillema. - if (std::find(v8_args.begin(), v8_args.end(), "--prof") != v8_args.end()) { - per_process::v8_is_profiling = true; - } - #ifdef __POSIX__ // Block SIGPROF signals when sleeping in epoll_wait/kevent/etc. Avoids the // performance penalty of frequent EINTR wakeups when the profiler is running. // Only do this for v8.log profiling, as it breaks v8::CpuProfiler users. - if (per_process::v8_is_profiling) { + if (std::find(v8_args.begin(), v8_args.end(), "--prof") != v8_args.end()) { uv_loop_configure(uv_default_loop(), UV_LOOP_BLOCK_SIGNAL, SIGPROF); } #endif diff --git a/src/node_internals.h b/src/node_internals.h index 65a4edcbea6b81..dffaa084db409b 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -55,7 +55,6 @@ class NativeModuleLoader; namespace per_process { extern Mutex env_var_mutex; extern uint64_t node_start_time; -extern bool v8_is_profiling; } // namespace per_process // Forward declaration diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 3a025d68ef3436..0a40ba006b5d2b 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -224,16 +224,6 @@ void RawDebug(const FunctionCallbackInfo& args) { fflush(stderr); } -static void StartProfilerIdleNotifier(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - env->StartProfilerIdleNotifier(); -} - -static void StopProfilerIdleNotifier(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - env->StopProfilerIdleNotifier(); -} - static void Umask(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK(env->has_run_bootstrapping_code()); @@ -456,10 +446,6 @@ static void InitializeProcessMethods(Local target, env->SetMethod(target, "chdir", Chdir); } - env->SetMethod( - target, "_startProfilerIdleNotifier", StartProfilerIdleNotifier); - env->SetMethod(target, "_stopProfilerIdleNotifier", StopProfilerIdleNotifier); - env->SetMethod(target, "umask", Umask); env->SetMethod(target, "_rawDebug", RawDebug); env->SetMethod(target, "memoryUsage", MemoryUsage);