src: always enable idle notifier

This removes the functionality behind `process._startProfilerIdleNotifier()` and `process._stopProfilerIdleNotifier()` and instead always informs V8 over the current event loop state. The methods themselves are left as noop stubs as per our deprecation policy. Fixes: nodejs#19009
addaleax · Apr 29, 2020 · 6272ac5 · 6272ac5
1 parent 766b3c1
commit 6272ac5
Show file tree

Hide file tree

Showing 9 changed files with 9 additions and 54 deletions.
diff --git a/lib/internal/bootstrap/switches/is_main_thread.js b/lib/internal/bootstrap/switches/is_main_thread.js
@@ -6,8 +6,10 @@ 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;
+
+// Stubs for legacy methods.
+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
@@ -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
@@ -441,7 +441,7 @@ MaybeLocal<Value> LoadEnvironment(
     Environment* env,
     StartExecutionCallback cb,
     std::unique_ptr<InspectorParentHandle> 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
@@ -373,10 +373,6 @@ inline Environment* Environment::GetThreadLocalEnv() {
   return static_cast<Environment*>(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
@@ -504,7 +504,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());
 
@@ -557,9 +557,7 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) {
   // FreeEnvironment.
   RegisterHandleCleanups();
 
-  if (start_profiler_idle_notifier) {
-    StartProfilerIdleNotifier();
-  }
+  StartProfilerIdleNotifier();
 }
 
 void Environment::ExitEnv() {
@@ -636,11 +634,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);
@@ -652,12 +645,6 @@ void Environment::StartProfilerIdleNotifier() {
   });
 }
 
-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
@@ -895,7 +895,7 @@ class Environment : public MemoryRetainer {
               ThreadId thread_id);
   ~Environment() override;
 
-  void InitializeLibuv(bool start_profiler_idle_notifier);
+  void InitializeLibuv();
   inline const std::vector<std::string>& exec_argv();
   inline const std::vector<std::string>& argv();
   const std::string& exec_path() const;
@@ -926,8 +926,6 @@ class Environment : public MemoryRetainer {
                               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;
@@ -1269,7 +1267,6 @@ class Environment : public MemoryRetainer {
   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
@@ -149,8 +149,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;
@@ -743,19 +741,11 @@ int ProcessGlobalArgs(std::vector<std::string>* 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
@@ -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
@@ -226,16 +226,6 @@ void RawDebug(const FunctionCallbackInfo<Value>& args) {
   fflush(stderr);
 }
 
-static void StartProfilerIdleNotifier(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  env->StartProfilerIdleNotifier();
-}
-
-static void StopProfilerIdleNotifier(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  env->StopProfilerIdleNotifier();
-}
-
 static void Umask(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   CHECK(env->has_run_bootstrapping_code());
@@ -469,10 +459,6 @@ static void InitializeProcessMethods(Local<Object> 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);