[cpu-profiler] Cleanup is_logging/is_profiling state tracking

Refactors logging suppression and profiling state tracking on isolates
to be tied to a RAII ProfilerScope. Fixes the case where multiple
concurrent profilers on the same isolate restore the wrong value of
is_logging.

Change-Id: I34b59422a4e6e077ae0abb46eb09d78a77870d46
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1575918
Commit-Queue: Andrew Comminos <acomminos@fb.com>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61027}

src/isolate.h

@@ -393,6 +393,8 @@ using DebugObjectCache = std::vector<Handle<HeapObject>>;
   V(int, external_script_source_size, 0)                                       \
   /* true if being profiled. Causes collection of extra compile info. */       \
   V(bool, is_profiling, false)                                                 \
+  /* Number of CPU profilers running on the isolate. */                        \
+  V(size_t, num_cpu_profilers, 0)                                              \
   /* true if a trace is being formatted through Error.prepareStackTrace. */    \
   V(bool, formatting_stack_trace, false)                                       \
   /* Perform side effect checks on function call and API callbacks. */         \

src/log.cc

@@ -1079,6 +1079,12 @@ void Logger::LeaveExternal(Isolate* isolate) {
   isolate->set_current_vm_state(JS);
 }
 
+bool Logger::is_logging() {
+  // Disable logging while the CPU profiler is running.
+  if (isolate_->is_profiling()) return false;
+  return is_logging_;
+}
+
 // Instantiate template methods.
 #define V(TimerName, expose)                                           \
   template void TimerEventScope<TimerEvent##TimerName>::LogTimerEvent( \

src/log.h

@@ -247,12 +247,7 @@ class Logger : public CodeEventListener {
   V8_INLINE static void CallEventLogger(Isolate* isolate, const char* name,
                                         StartEnd se, bool expose_to_api);
 
-  bool is_logging() {
-    return is_logging_;
-  }
-
-  // Used by CpuProfiler. TODO(petermarshall): Untangle
-  void set_is_logging(bool new_value) { is_logging_ = new_value; }
+  V8_EXPORT_PRIVATE bool is_logging();
 
   bool is_listening_to_code_events() override {
     return is_logging() || jit_logger_ != nullptr;

src/profiler/cpu-profiler.cc

@@ -51,7 +51,8 @@ ProfilerEventsProcessor::ProfilerEventsProcessor(Isolate* isolate,
       running_(1),
       last_code_event_id_(0),
       last_processed_code_event_id_(0),
-      isolate_(isolate) {}
+      isolate_(isolate),
+      profiling_scope_(isolate) {}
 
 SamplingEventsProcessor::SamplingEventsProcessor(Isolate* isolate,
                                                  ProfileGenerator* generator,
@@ -396,9 +397,6 @@ void CpuProfiler::StartProcessorIfNotStarted() {
   }
   isolate_->wasm_engine()->EnableCodeLogging(isolate_);
   Logger* logger = isolate_->logger();
-  // Disable logging when using the new implementation.
-  saved_is_logging_ = logger->is_logging();
-  logger->set_is_logging(false);
 
   bool codemap_needs_initialization = false;
   if (!generator_) {
@@ -416,7 +414,6 @@ void CpuProfiler::StartProcessorIfNotStarted() {
   }
   logger->AddCodeEventListener(profiler_listener_.get());
   is_profiling_ = true;
-  isolate_->set_is_profiling(true);
   // Enumerate stuff we already have in the heap.
   DCHECK(isolate_->heap()->HasBeenSetUp());
   if (codemap_needs_initialization) {
@@ -450,11 +447,9 @@ void CpuProfiler::StopProcessorIfLastProfile(const char* title) {
 void CpuProfiler::StopProcessor() {
   Logger* logger = isolate_->logger();
   is_profiling_ = false;
-  isolate_->set_is_profiling(false);
   logger->RemoveCodeEventListener(profiler_listener_.get());
   processor_->StopSynchronously();
   processor_.reset();
-  logger->set_is_logging(saved_is_logging_);
 }
 
 

src/profiler/cpu-profiler.h

@@ -129,6 +129,27 @@ class CodeEventsContainer {
   };
 };
 
+// Maintains the number of active CPU profilers in an isolate.
+class ProfilingScope {
+ public:
+  explicit ProfilingScope(Isolate* isolate) : isolate_(isolate) {
+    size_t profiler_count = isolate_->num_cpu_profilers();
+    profiler_count++;
+    isolate_->set_num_cpu_profilers(profiler_count);
+    isolate_->set_is_profiling(true);
+  }
+
+  ~ProfilingScope() {
+    size_t profiler_count = isolate_->num_cpu_profilers();
+    DCHECK_GT(profiler_count, 0);
+    profiler_count--;
+    isolate_->set_num_cpu_profilers(profiler_count);
+    if (profiler_count == 0) isolate_->set_is_profiling(false);
+  }
+
+ private:
+  Isolate* const isolate_;
+};
 
 // This class implements both the profile events processor thread and
 // methods called by event producers: VM and stack sampler threads.
@@ -173,6 +194,7 @@ class V8_EXPORT_PRIVATE ProfilerEventsProcessor : public base::Thread,
   std::atomic<unsigned> last_code_event_id_;
   unsigned last_processed_code_event_id_;
   Isolate* isolate_;
+  ProfilingScope profiling_scope_;
 };
 
 class V8_EXPORT_PRIVATE SamplingEventsProcessor
@@ -268,7 +290,6 @@ class V8_EXPORT_PRIVATE CpuProfiler {
   std::unique_ptr<ProfileGenerator> generator_;
   std::unique_ptr<ProfilerEventsProcessor> processor_;
   std::unique_ptr<ProfilerListener> profiler_listener_;
-  bool saved_is_logging_;
   bool is_profiling_;
 
   DISALLOW_COPY_AND_ASSIGN(CpuProfiler);