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

This reverts commit 9a19ce25.

Reason for revert:
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20TSAN/builds/20359

Original change's description:
> [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}

TBR=alph@chromium.org,yangguo@chromium.org

Change-Id: I3c3b6eb8d6f9911fa318f24a2e6e74180b83398e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:7662
Reviewed-on: https://chromium-review.googlesource.com/1015561Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52654}

src/log.cc

@@ -814,12 +814,13 @@ Logger::~Logger() {
   delete log_;
 }
 
-void Logger::AddCodeEventListener(CodeEventListener* listener) {
+void Logger::addCodeEventListener(CodeEventListener* listener) {
   bool result = isolate_->code_event_dispatcher()->AddListener(listener);
-  CHECK(result);
+  USE(result);
+  DCHECK(result);
 }
 
-void Logger::RemoveCodeEventListener(CodeEventListener* listener) {
+void Logger::removeCodeEventListener(CodeEventListener* listener) {
   isolate_->code_event_dispatcher()->RemoveListener(listener);
 }
 
@@ -1539,7 +1540,7 @@ void Logger::StopProfiler() {
   if (profiler_ != nullptr) {
     profiler_->Pause();
     is_logging_ = false;
-    RemoveCodeEventListener(this);
+    removeCodeEventListener(this);
   }
 }
 
@@ -1891,17 +1892,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);
@@ -1918,8 +1919,10 @@ bool Logger::SetUp(Isolate* isolate) {
     profiler_->Engage();
   }
 
+  profiler_listener_.reset();
+
   if (is_logging_) {
-    AddCodeEventListener(this);
+    addCodeEventListener(this);
   }
 
   return true;
@@ -1929,14 +1932,14 @@ bool Logger::SetUp(Isolate* isolate) {
 void Logger::SetCodeEventHandler(uint32_t options,
                                  JitCodeEventHandler event_handler) {
   if (jit_logger_) {
-    RemoveCodeEventListener(jit_logger_);
-    delete jit_logger_;
-    jit_logger_ = nullptr;
+      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();
@@ -1945,11 +1948,17 @@ void Logger::SetCodeEventHandler(uint32_t options,
   }
 }
 
-ProfilerListener* Logger::EnsureProfilerListener() {
-  CHECK(is_initialized_);
-  if (!profiler_listener_)
+void Logger::SetUpProfilerListener() {
+  if (!is_initialized_) return;
+  if (profiler_listener_.get() == nullptr) {
     profiler_listener_.reset(new ProfilerListener(isolate_));
-  return profiler_listener_.get();
+  }
+  addCodeEventListener(profiler_listener_.get());
+}
+
+void Logger::TearDownProfilerListener() {
+  if (profiler_listener_->HasObservers()) return;
+  removeCodeEventListener(profiler_listener_.get());
 }
 
 sampler::Sampler* Logger::sampler() {
@@ -1972,31 +1981,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,9 +107,15 @@ 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* EnsureProfilerListener();
+  ProfilerListener* profiler_listener() { return profiler_listener_.get(); }
 
   // Frees resources acquired in SetUp.
   // When a temporary file is used for the log, returns its stream descriptor,
@@ -157,8 +163,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,7 +373,9 @@ void CpuProfiler::StartProcessorIfNotStarted() {
   processor_.reset(new ProfilerEventsProcessor(isolate_, generator_.get(),
                                                sampling_interval_));
   CreateEntriesForRuntimeCallStats();
-  logger->EnsureProfilerListener()->AddObserver(this);
+  logger->SetUpProfilerListener();
+  ProfilerListener* profiler_listener = logger->profiler_listener();
+  profiler_listener->AddObserver(this);
   is_profiling_ = true;
   isolate_->set_is_profiling(true);
   // Enumerate stuff we already have in the heap.
@@ -408,8 +410,10 @@ void CpuProfiler::StopProcessor() {
   Logger* logger = isolate_->logger();
   is_profiling_ = false;
   isolate_->set_is_profiling(false);
-  logger->EnsureProfilerListener()->RemoveObserver(this);
+  ProfilerListener* profiler_listener = logger->profiler_listener();
+  profiler_listener->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)
-    : isolate_(isolate), function_and_resource_names_(isolate->heap()) {}
+    : function_and_resource_names_(isolate->heap()) {}
 
 ProfilerListener::~ProfilerListener() = default;
 
@@ -308,26 +308,20 @@ 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);
   }
-  observers_.push_back(observer);
+  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()) {
-    observers_.erase(it);
-  }
-  if (observers_.empty()) {
-    isolate_->logger()->RemoveCodeEventListener(this);
-  }
+  if (it == observers_.end()) return;
+  observers_.erase(it);
 }
 
 }  // namespace internal

src/profiler/profiler-listener.h

@@ -90,7 +90,6 @@ 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,6 +165,7 @@ 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.
@@ -182,6 +183,7 @@ 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.
@@ -225,6 +227,7 @@ 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");
@@ -241,6 +244,7 @@ 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);
@@ -261,6 +265,8 @@ 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
@@ -294,6 +300,7 @@ 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");
@@ -308,6 +315,7 @@ TEST(Issue1398) {
   processor->FinishTickSample();
 
   profiler_listener.RemoveObserver(&profiler);
+  isolate->code_event_dispatcher()->RemoveListener(&profiler_listener);
   processor->StopSynchronously();
   CpuProfile* profile = profiles->StopProfiling("");
   CHECK(profile);
@@ -1079,6 +1087,7 @@ 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.
@@ -1092,6 +1101,7 @@ 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("");
@@ -2375,7 +2385,7 @@ TEST(CodeEntriesMemoryLeak) {
     profile->Delete();
   }
   ProfilerListener* profiler_listener =
-      CcTest::i_isolate()->logger()->EnsureProfilerListener();
+      CcTest::i_isolate()->logger()->profiler_listener();
 
   CHECK_GE(10000ul, profiler_listener->entries_count_for_test());
 }
@@ -2473,15 +2483,6 @@ 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 =