[cpu-profiler] Move SetIdle() to v8::Isolate

hashseed · Commit Bot · commit 308d4e28f354 · 2018-03-07T06:16:08.000Z
The VM state is a property of the isolate, not the CPU profiler. Having to create a v8::CpuProfiler instance in order to change the property is somewhat inefficient. See nodejs/node#18039 and nodejs/node#18534 for context. Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: I70e31deca6529bccc05a0f4ed500ee268fb63cb8 Reviewed-on: https://chromium-review.googlesource.com/900622 Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#51779}
diff --git a/include/v8-profiler.h b/include/v8-profiler.h
@@ -335,7 +335,8 @@ class V8_EXPORT CpuProfiler {
   /**
    * Tells the profiler whether the embedder is idle.
    */
-  void SetIdle(bool is_idle);
+  V8_DEPRECATED("Use Isolate::SetIdle(bool) instead.",
+                void SetIdle(bool is_idle));
 
  private:
   CpuProfiler();
diff --git a/include/v8.h b/include/v8.h
@@ -7283,6 +7283,11 @@ class V8_EXPORT Isolate {
   V8_DEPRECATED("CpuProfiler should be created with CpuProfiler::New call.",
                 CpuProfiler* GetCpuProfiler());
 
+  /**
+   * Tells the CPU profiler whether the embedder is idle.
+   */
+  void SetIdle(bool is_idle);
+
   /** Returns true if this isolate has a current context. */
   bool InContext();
 
diff --git a/src/api.cc b/src/api.cc
@@ -8110,6 +8110,10 @@ CpuProfiler* Isolate::GetCpuProfiler() {
   return reinterpret_cast<CpuProfiler*>(cpu_profiler);
 }
 
+void Isolate::SetIdle(bool is_idle) {
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
+  isolate->SetIdle(is_idle);
+}
 
 bool Isolate::InContext() {
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
@@ -10126,15 +10130,7 @@ CpuProfile* CpuProfiler::StopProfiling(Local<String> title) {
 void CpuProfiler::SetIdle(bool is_idle) {
   i::CpuProfiler* profiler = reinterpret_cast<i::CpuProfiler*>(this);
   i::Isolate* isolate = profiler->isolate();
-  if (!isolate->is_profiling()) return;
-  v8::StateTag state = isolate->current_vm_state();
-  DCHECK(state == v8::EXTERNAL || state == v8::IDLE);
-  if (isolate->js_entry_sp() != nullptr) return;
-  if (is_idle) {
-    isolate->set_current_vm_state(v8::IDLE);
-  } else if (state == v8::IDLE) {
-    isolate->set_current_vm_state(v8::EXTERNAL);
-  }
+  isolate->SetIdle(is_idle);
 }
 
 
diff --git a/src/inspector/v8-inspector-impl.cc b/src/inspector/v8-inspector-impl.cc
@@ -234,21 +234,9 @@ void V8InspectorImpl::resetContextGroup(int contextGroupId) {
   m_debugger->wasmTranslation()->Clear();
 }
 
-void V8InspectorImpl::idleStarted() {
-  for (auto& it : m_sessions) {
-    for (auto& it2 : it.second) {
-      if (it2.second->profilerAgent()->idleStarted()) return;
-    }
-  }
-}
+void V8InspectorImpl::idleStarted() { m_isolate->SetIdle(true); }
 
-void V8InspectorImpl::idleFinished() {
-  for (auto& it : m_sessions) {
-    for (auto& it2 : it.second) {
-      if (it2.second->profilerAgent()->idleFinished()) return;
-    }
-  }
-}
+void V8InspectorImpl::idleFinished() { m_isolate->SetIdle(false); }
 
 unsigned V8InspectorImpl::exceptionThrown(
     v8::Local<v8::Context> context, const StringView& message,
diff --git a/src/inspector/v8-profiler-agent-impl.cc b/src/inspector/v8-profiler-agent-impl.cc
@@ -476,7 +476,6 @@ void V8ProfilerAgentImpl::startProfiling(const String16& title) {
   if (!m_startedProfilesCount) {
     DCHECK(!m_profiler);
     m_profiler = v8::CpuProfiler::New(m_isolate);
-    m_profiler->SetIdle(m_idle);
     int interval =
         m_state->integerProperty(ProfilerAgentState::samplingInterval, 0);
     if (interval) m_profiler->SetSamplingInterval(interval);
@@ -503,16 +502,4 @@ std::unique_ptr<protocol::Profiler::Profile> V8ProfilerAgentImpl::stopProfiling(
   return result;
 }
 
-bool V8ProfilerAgentImpl::idleStarted() {
-  m_idle = true;
-  if (m_profiler) m_profiler->SetIdle(m_idle);
-  return m_profiler;
-}
-
-bool V8ProfilerAgentImpl::idleFinished() {
-  m_idle = false;
-  if (m_profiler) m_profiler->SetIdle(m_idle);
-  return m_profiler;
-}
-
 }  // namespace v8_inspector
diff --git a/src/inspector/v8-profiler-agent-impl.h b/src/inspector/v8-profiler-agent-impl.h
@@ -57,9 +57,6 @@ class V8ProfilerAgentImpl : public protocol::Profiler::Backend {
   void consoleProfile(const String16& title);
   void consoleProfileEnd(const String16& title);
 
-  bool idleStarted();
-  bool idleFinished();
-
  private:
   String16 nextProfileId();
 
@@ -77,7 +74,6 @@ class V8ProfilerAgentImpl : public protocol::Profiler::Backend {
   class ProfileDescriptor;
   std::vector<ProfileDescriptor> m_startedProfiles;
   String16 m_frontendInitiatedProfileId;
-  bool m_idle = false;
   int m_startedProfilesCount = 0;
 
   DISALLOW_COPY_AND_ASSIGN(V8ProfilerAgentImpl);
diff --git a/src/isolate.cc b/src/isolate.cc
@@ -4038,6 +4038,18 @@ void Isolate::PrintWithTimestamp(const char* format, ...) {
   va_end(arguments);
 }
 
+void Isolate::SetIdle(bool is_idle) {
+  if (!is_profiling()) return;
+  StateTag state = current_vm_state();
+  DCHECK(state == EXTERNAL || state == IDLE);
+  if (js_entry_sp() != nullptr) return;
+  if (is_idle) {
+    set_current_vm_state(IDLE);
+  } else if (state == IDLE) {
+    set_current_vm_state(EXTERNAL);
+  }
+}
+
 bool StackLimitCheck::JsHasOverflowed(uintptr_t gap) const {
   StackGuard* stack_guard = isolate_->stack_guard();
 #ifdef USE_SIMULATOR
diff --git a/src/isolate.h b/src/isolate.h
@@ -1354,6 +1354,8 @@ class Isolate {
     top_backup_incumbent_scope_ = top_backup_incumbent_scope;
   }
 
+  void SetIdle(bool is_idle);
+
  protected:
   explicit Isolate(bool enable_serializer);
   bool IsArrayOrObjectOrStringPrototype(Object* object);
diff --git a/test/cctest/test-cpu-profiler.cc b/test/cctest/test-cpu-profiler.cc
@@ -1651,11 +1651,11 @@ TEST(IdleTime) {
       reinterpret_cast<i::CpuProfiler*>(cpu_profiler)->processor();
 
   processor->AddCurrentStack(isolate, true);
-  cpu_profiler->SetIdle(true);
+  isolate->SetIdle(true);
   for (int i = 0; i < 3; i++) {
     processor->AddCurrentStack(isolate, true);
   }
-  cpu_profiler->SetIdle(false);
+  isolate->SetIdle(false);
   processor->AddCurrentStack(isolate, true);
 
   v8::CpuProfile* profile = cpu_profiler->StopProfiling(profile_name);

Original file line number	Diff line number	Diff line change
`@@ -1354,6 +1354,8 @@ class Isolate {`
`1354`	`1354`	`top_backup_incumbent_scope_ = top_backup_incumbent_scope;`
`1355`	`1355`	`}`
`1356`	`1356`
	`1357`	`+ void SetIdle(bool is_idle);`
	`1358`	`+`
`1357`	`1359`	`protected:`
`1358`	`1360`	`explicit Isolate(bool enable_serializer);`
`1359`	`1361`	`bool IsArrayOrObjectOrStringPrototype(Object* object);`