[profiler] Ensure there's a single ProfilerListener per isolate.

BUG=v8:7662

Change-Id: I8128ac96bcd2dc01b318c55843c4416bdd17c7ae
Reviewed-on: https://chromium-review.googlesource.com/1013318
Commit-Queue: Alexei Filippov <alph@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52653}

src/log.cc

@@ -814,13 +814,12 @@ Logger::~Logger() {
   delete log_;
 }
 
-void Logger::addCodeEventListener(CodeEventListener* listener) {
+void Logger::AddCodeEventListener(CodeEventListener* listener) {
   bool result = isolate_->code_event_dispatcher()->AddListener(listener);
-  USE(result);
-  DCHECK(result);
+  CHECK(result);
 }
 
-void Logger::removeCodeEventListener(CodeEventListener* listener) {
+void Logger::RemoveCodeEventListener(CodeEventListener* listener) {
   isolate_->code_event_dispatcher()->RemoveListener(listener);
 }
 
@@ -1540,7 +1539,7 @@ void Logger::StopProfiler() {
   if (profiler_ != nullptr) {
     profiler_->Pause();
     is_logging_ = false;
-    removeCodeEventListener(this);
+    RemoveCodeEventListener(this);
   }
 }
 
@@ -1892,17 +1891,17 @@ bool Logger::SetUp(Isolate* isolate) {
 
   if (FLAG_perf_basic_prof) {
     perf_basic_logger_ = new PerfBasicLogger();
-    addCodeEventListener(perf_basic_logger_);
+    AddCodeEventListener(perf_basic_logger_);
   }
 
   if (FLAG_perf_prof) {
     perf_jit_logger_ = new PerfJitLogger();
-    addCodeEventListener(perf_jit_logger_);
+    AddCodeEventListener(perf_jit_logger_);
   }
 
   if (FLAG_ll_prof) {
     ll_logger_ = new LowLevelLogger(log_file_name.str().c_str());
-    addCodeEventListener(ll_logger_);
+    AddCodeEventListener(ll_logger_);
   }
 
   ticker_ = new Ticker(isolate, FLAG_prof_sampling_interval);
@@ -1919,10 +1918,8 @@ bool Logger::SetUp(Isolate* isolate) {
     profiler_->Engage();
   }
 
-  profiler_listener_.reset();
-
   if (is_logging_) {
-    addCodeEventListener(this);
+    AddCodeEventListener(this);
   }
 
   return true;
@@ -1932,14 +1929,14 @@ bool Logger::SetUp(Isolate* isolate) {
 void Logger::SetCodeEventHandler(uint32_t options,
                                  JitCodeEventHandler event_handler) {
   if (jit_logger_) {
-      removeCodeEventListener(jit_logger_);
+    RemoveCodeEventListener(jit_logger_);
     delete jit_logger_;
     jit_logger_ = nullptr;
   }
 
   if (event_handler) {
     jit_logger_ = new JitLogger(event_handler);
-    addCodeEventListener(jit_logger_);
+    AddCodeEventListener(jit_logger_);
     if (options & kJitCodeEventEnumExisting) {
       HandleScope scope(isolate_);
       LogCodeObjects();
@@ -1948,17 +1945,11 @@ void Logger::SetCodeEventHandler(uint32_t options,
   }
 }
 
-void Logger::SetUpProfilerListener() {
-  if (!is_initialized_) return;
-  if (profiler_listener_.get() == nullptr) {
+ProfilerListener* Logger::EnsureProfilerListener() {
+  CHECK(is_initialized_);
+  if (!profiler_listener_)
     profiler_listener_.reset(new ProfilerListener(isolate_));
-  }
-  addCodeEventListener(profiler_listener_.get());
-}
-
-void Logger::TearDownProfilerListener() {
-  if (profiler_listener_->HasObservers()) return;
-  removeCodeEventListener(profiler_listener_.get());
+  return profiler_listener_.get();
 }
 
 sampler::Sampler* Logger::sampler() {
@@ -1981,31 +1972,31 @@ FILE* Logger::TearDown() {
   ticker_ = nullptr;
 
   if (perf_basic_logger_) {
-    removeCodeEventListener(perf_basic_logger_);
+    RemoveCodeEventListener(perf_basic_logger_);
     delete perf_basic_logger_;
     perf_basic_logger_ = nullptr;
   }
 
   if (perf_jit_logger_) {
-    removeCodeEventListener(perf_jit_logger_);
+    RemoveCodeEventListener(perf_jit_logger_);
     delete perf_jit_logger_;
     perf_jit_logger_ = nullptr;
   }
 
   if (ll_logger_) {
-    removeCodeEventListener(ll_logger_);
+    RemoveCodeEventListener(ll_logger_);
     delete ll_logger_;
     ll_logger_ = nullptr;
   }
 
   if (jit_logger_) {
-    removeCodeEventListener(jit_logger_);
+    RemoveCodeEventListener(jit_logger_);
     delete jit_logger_;
     jit_logger_ = nullptr;
   }
 
   if (profiler_listener_.get() != nullptr) {
-    removeCodeEventListener(profiler_listener_.get());
+    RemoveCodeEventListener(profiler_listener_.get());
   }
 
   return log_->Close();

src/log.h

@@ -107,15 +107,9 @@ class Logger : public CodeEventListener {
   void SetCodeEventHandler(uint32_t options,
                            JitCodeEventHandler event_handler);
 
-  // Sets up ProfilerListener.
-  void SetUpProfilerListener();
-
-  // Tear down ProfilerListener if it has no observers.
-  void TearDownProfilerListener();
-
   sampler::Sampler* sampler();
 
-  ProfilerListener* profiler_listener() { return profiler_listener_.get(); }
+  ProfilerListener* EnsureProfilerListener();
 
   // Frees resources acquired in SetUp.
   // When a temporary file is used for the log, returns its stream descriptor,
@@ -163,8 +157,8 @@ class Logger : public CodeEventListener {
   void ApiEntryCall(const char* name);
 
   // ==== Events logged by --log-code. ====
-  void addCodeEventListener(CodeEventListener* listener);
-  void removeCodeEventListener(CodeEventListener* listener);
+  void AddCodeEventListener(CodeEventListener* listener);
+  void RemoveCodeEventListener(CodeEventListener* listener);
 
   // Emits a code event for a callback function.
   void CallbackEvent(Name* name, Address entry_point);

src/profiler/cpu-profiler.cc

@@ -373,9 +373,7 @@ void CpuProfiler::StartProcessorIfNotStarted() {
   processor_.reset(new ProfilerEventsProcessor(isolate_, generator_.get(),
                                                sampling_interval_));
   CreateEntriesForRuntimeCallStats();
-  logger->SetUpProfilerListener();
-  ProfilerListener* profiler_listener = logger->profiler_listener();
-  profiler_listener->AddObserver(this);
+  logger->EnsureProfilerListener()->AddObserver(this);
   is_profiling_ = true;
   isolate_->set_is_profiling(true);
   // Enumerate stuff we already have in the heap.
@@ -410,10 +408,8 @@ void CpuProfiler::StopProcessor() {
   Logger* logger = isolate_->logger();
   is_profiling_ = false;
   isolate_->set_is_profiling(false);
-  ProfilerListener* profiler_listener = logger->profiler_listener();
-  profiler_listener->RemoveObserver(this);
+  logger->EnsureProfilerListener()->RemoveObserver(this);
   processor_->StopSynchronously();
-  logger->TearDownProfilerListener();
   processor_.reset();
   generator_.reset();
   logger->is_logging_ = saved_is_logging_;

src/profiler/profiler-listener.cc

@@ -16,7 +16,7 @@ namespace v8 {
 namespace internal {
 
 ProfilerListener::ProfilerListener(Isolate* isolate)
-    : function_and_resource_names_(isolate->heap()) {}
+    : isolate_(isolate), function_and_resource_names_(isolate->heap()) {}
 
 ProfilerListener::~ProfilerListener() = default;
 
@@ -308,20 +308,26 @@ CodeEntry* ProfilerListener::NewCodeEntry(
 
 void ProfilerListener::AddObserver(CodeEventObserver* observer) {
   base::LockGuard<base::Mutex> guard(&mutex_);
+  if (std::find(observers_.begin(), observers_.end(), observer) !=
+      observers_.end()) {
+    return;
+  }
   if (observers_.empty()) {
     code_entries_.clear();
+    isolate_->logger()->AddCodeEventListener(this);
   }
-  if (std::find(observers_.begin(), observers_.end(), observer) ==
-      observers_.end()) {
   observers_.push_back(observer);
-  }
 }
 
 void ProfilerListener::RemoveObserver(CodeEventObserver* observer) {
   base::LockGuard<base::Mutex> guard(&mutex_);
   auto it = std::find(observers_.begin(), observers_.end(), observer);
-  if (it == observers_.end()) return;
+  if (it != observers_.end()) {
     observers_.erase(it);
+  }
+  if (observers_.empty()) {
+    isolate_->logger()->RemoveCodeEventListener(this);
+  }
 }
 
 }  // namespace internal

src/profiler/profiler-listener.h

@@ -90,6 +90,7 @@ class ProfilerListener : public CodeEventListener {
     }
   }
 
+  Isolate* isolate_;
   StringsStorage function_and_resource_names_;
   std::vector<std::unique_ptr<CodeEntry>> code_entries_;
   std::vector<CodeEventObserver*> observers_;

src/snapshot/serializer.h

@@ -21,11 +21,11 @@ namespace internal {
 class CodeAddressMap : public CodeEventLogger {
  public:
   explicit CodeAddressMap(Isolate* isolate) : isolate_(isolate) {
-    isolate->logger()->addCodeEventListener(this);
+    isolate->logger()->AddCodeEventListener(this);
   }
 
   ~CodeAddressMap() override {
-    isolate_->logger()->removeCodeEventListener(this);
+    isolate_->logger()->RemoveCodeEventListener(this);
   }
 
   void CodeMoveEvent(AbstractCode* from, Address to) override {

test/cctest/test-cpu-profiler.cc

@@ -165,7 +165,6 @@ TEST(CodeEvents) {
   profiles->StartProfiling("", false);
   processor->Start();
   ProfilerListener profiler_listener(isolate);
-  isolate->code_event_dispatcher()->AddListener(&profiler_listener);
   profiler_listener.AddObserver(&profiler);
 
   // Enqueue code creation events.
@@ -183,7 +182,6 @@ TEST(CodeEvents) {
   EnqueueTickSampleEvent(processor, aaa_code->address());
 
   profiler_listener.RemoveObserver(&profiler);
-  isolate->code_event_dispatcher()->RemoveListener(&profiler_listener);
   processor->StopSynchronously();
 
   // Check the state of profile generator.
@@ -227,7 +225,6 @@ TEST(TickEvents) {
   profiles->StartProfiling("", false);
   processor->Start();
   ProfilerListener profiler_listener(isolate);
-  isolate->code_event_dispatcher()->AddListener(&profiler_listener);
   profiler_listener.AddObserver(&profiler);
 
   profiler_listener.CodeCreateEvent(i::Logger::BUILTIN_TAG, frame1_code, "bbb");
@@ -244,7 +241,6 @@ TEST(TickEvents) {
                          frame1_code->raw_instruction_end() - 1);
 
   profiler_listener.RemoveObserver(&profiler);
-  isolate->code_event_dispatcher()->RemoveListener(&profiler_listener);
   processor->StopSynchronously();
   CpuProfile* profile = profiles->StopProfiling("");
   CHECK(profile);
@@ -265,8 +261,6 @@ TEST(TickEvents) {
   const std::vector<ProfileNode*>* top_down_ddd_children =
       top_down_stub_children->back()->children();
   CHECK(top_down_ddd_children->empty());
-
-  isolate->code_event_dispatcher()->RemoveListener(&profiler_listener);
 }
 
 // http://crbug/51594
@@ -300,7 +294,6 @@ TEST(Issue1398) {
   profiles->StartProfiling("", false);
   processor->Start();
   ProfilerListener profiler_listener(isolate);
-  isolate->code_event_dispatcher()->AddListener(&profiler_listener);
   profiler_listener.AddObserver(&profiler);
 
   profiler_listener.CodeCreateEvent(i::Logger::BUILTIN_TAG, code, "bbb");
@@ -315,7 +308,6 @@ TEST(Issue1398) {
   processor->FinishTickSample();
 
   profiler_listener.RemoveObserver(&profiler);
-  isolate->code_event_dispatcher()->RemoveListener(&profiler_listener);
   processor->StopSynchronously();
   CpuProfile* profile = profiles->StopProfiling("");
   CHECK(profile);
@@ -1087,7 +1079,6 @@ static void TickLines(bool optimize) {
   profiles->StartProfiling("", false);
   processor->Start();
   ProfilerListener profiler_listener(isolate);
-  isolate->code_event_dispatcher()->AddListener(&profiler_listener);
   profiler_listener.AddObserver(&profiler);
 
   // Enqueue code creation events.
@@ -1101,7 +1092,6 @@ static void TickLines(bool optimize) {
   EnqueueTickSampleEvent(processor, code_address);
 
   profiler_listener.RemoveObserver(&profiler);
-  isolate->code_event_dispatcher()->RemoveListener(&profiler_listener);
   processor->StopSynchronously();
 
   CpuProfile* profile = profiles->StopProfiling("");
@@ -2385,7 +2375,7 @@ TEST(CodeEntriesMemoryLeak) {
     profile->Delete();
   }
   ProfilerListener* profiler_listener =
-      CcTest::i_isolate()->logger()->profiler_listener();
+      CcTest::i_isolate()->logger()->EnsureProfilerListener();
 
   CHECK_GE(10000ul, profiler_listener->entries_count_for_test());
 }
@@ -2483,6 +2473,15 @@ TEST(SourcePositionTable) {
   CHECK_EQ(3, info->GetSourceLineNumber(std::numeric_limits<int>::max()));
 }
 
+TEST(MultipleProfilers) {
+  std::unique_ptr<CpuProfiler> profiler1(new CpuProfiler(CcTest::i_isolate()));
+  std::unique_ptr<CpuProfiler> profiler2(new CpuProfiler(CcTest::i_isolate()));
+  profiler1->StartProfiling("1");
+  profiler2->StartProfiling("2");
+  profiler1->StopProfiling("1");
+  profiler2->StopProfiling("2");
+}
+
 }  // namespace test_cpu_profiler
 }  // namespace internal
 }  // namespace v8

test/cctest/test-log.cc

@@ -708,7 +708,7 @@ TEST(Issue539892) {
 
   {
     ScopedLoggerInitializer logger(saved_log, saved_prof, isolate);
-    logger.logger()->addCodeEventListener(&code_event_logger);
+    logger.logger()->AddCodeEventListener(&code_event_logger);
 
     // Function with a really large name.
     const char* source_text =