[cpu-profiler] Do not store CodeEntries between profiling sessions.

ProfilerListener which holds CodeEntries has been moved from Logger to
CpuProfiler. This way we can clear entries when all the profiles
produced by a particular CpuProfiler are deleted.

BUG=v8:7719

Change-Id: I31d47dc7da44648c8fb8e87b47e2e6260d3dc5c3
Reviewed-on: https://chromium-review.googlesource.com/1043050Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53004}

src/log.cc

@@ -24,7 +24,6 @@
 #include "src/macro-assembler.h"
 #include "src/objects/api-callbacks.h"
 #include "src/perf-jit.h"
-#include "src/profiler/profiler-listener.h"
 #include "src/profiler/tick-sample.h"
 #include "src/runtime-profiler.h"
 #include "src/source-position-table.h"
@@ -1946,13 +1945,6 @@ void Logger::SetCodeEventHandler(uint32_t options,
   }
 }
 
-ProfilerListener* Logger::EnsureProfilerListener() {
-  CHECK(is_initialized_);
-  if (!profiler_listener_)
-    profiler_listener_.reset(new ProfilerListener(isolate_));
-  return profiler_listener_.get();
-}
-
 sampler::Sampler* Logger::sampler() {
   return ticker_;
 }
@@ -1996,10 +1988,6 @@ FILE* Logger::TearDown() {
     jit_logger_ = nullptr;
   }
 
-  if (profiler_listener_.get() != nullptr) {
-    RemoveCodeEventListener(profiler_listener_.get());
-  }
-
   return log_->Close();
 }
 

src/log.h

@@ -71,7 +71,6 @@ class LowLevelLogger;
 class PerfBasicLogger;
 class PerfJitLogger;
 class Profiler;
-class ProfilerListener;
 class RuntimeCallTimer;
 class Ticker;
 class WasmCompiledModule;
@@ -109,8 +108,6 @@ class Logger : public CodeEventListener {
 
   sampler::Sampler* sampler();
 
-  ProfilerListener* EnsureProfilerListener();
-
   // Frees resources acquired in SetUp.
   // When a temporary file is used for the log, returns its stream descriptor,
   // leaving the file open.
@@ -318,7 +315,6 @@ class Logger : public CodeEventListener {
   PerfJitLogger* perf_jit_logger_;
   LowLevelLogger* ll_logger_;
   JitLogger* jit_logger_;
-  std::unique_ptr<ProfilerListener> profiler_listener_;
   std::set<int> logged_source_code_;
   uint32_t next_source_info_id_ = 0;
 

src/profiler/cpu-profiler.cc

@@ -318,6 +318,7 @@ void CpuProfiler::set_sampling_interval(base::TimeDelta value) {
 void CpuProfiler::ResetProfiles() {
   profiles_.reset(new CpuProfilesCollection(isolate_));
   profiles_->set_cpu_profiler(this);
+  profiler_listener_.reset();
 }
 
 void CpuProfiler::CreateEntriesForRuntimeCallStats() {
@@ -372,8 +373,11 @@ void CpuProfiler::StartProcessorIfNotStarted() {
   generator_.reset(new ProfileGenerator(profiles_.get()));
   processor_.reset(new ProfilerEventsProcessor(isolate_, generator_.get(),
                                                sampling_interval_));
+  if (!profiler_listener_) {
+    profiler_listener_.reset(new ProfilerListener(isolate_, this));
+  }
   CreateEntriesForRuntimeCallStats();
-  logger->EnsureProfilerListener()->AddObserver(this);
+  logger->AddCodeEventListener(profiler_listener_.get());
   is_profiling_ = true;
   isolate_->set_is_profiling(true);
   // Enumerate stuff we already have in the heap.
@@ -408,7 +412,7 @@ void CpuProfiler::StopProcessor() {
   Logger* logger = isolate_->logger();
   is_profiling_ = false;
   isolate_->set_is_profiling(false);
-  logger->EnsureProfilerListener()->RemoveObserver(this);
+  logger->RemoveCodeEventListener(profiler_listener_.get());
   processor_->StopSynchronously();
   processor_.reset();
   generator_.reset();

src/profiler/cpu-profiler.h

@@ -216,6 +216,10 @@ class CpuProfiler : public CodeEventObserver {
   ProfilerEventsProcessor* processor() const { return processor_.get(); }
   Isolate* isolate() const { return isolate_; }
 
+  ProfilerListener* profiler_listener_for_test() {
+    return profiler_listener_.get();
+  }
+
  private:
   void StartProcessorIfNotStarted();
   void StopProcessorIfLastProfile(const char* title);
@@ -229,6 +233,7 @@ class CpuProfiler : public CodeEventObserver {
   std::unique_ptr<CpuProfilesCollection> profiles_;
   std::unique_ptr<ProfileGenerator> generator_;
   std::unique_ptr<ProfilerEventsProcessor> processor_;
+  std::unique_ptr<ProfilerListener> profiler_listener_;
   std::vector<std::unique_ptr<CodeEntry>> static_entries_;
   bool saved_is_logging_;
   bool is_profiling_;

src/profiler/profile-generator.cc

@@ -472,6 +472,8 @@ void CpuProfile::Print() {
   top_down_.Print();
 }
 
+CodeMap::CodeMap() = default;
+
 void CodeMap::AddCode(Address addr, CodeEntry* entry, unsigned size) {
   DeleteAllCoveredCode(addr, addr + size);
   code_map_.insert({addr, CodeEntryInfo(entry, size)});

src/profiler/profile-generator.h

@@ -315,7 +315,7 @@ class CpuProfile {
 
 class CodeMap {
  public:
-  CodeMap() {}
+  CodeMap();
 
   void AddCode(Address addr, CodeEntry* entry, unsigned size);
   void MoveCode(Address from, Address to);
@@ -325,7 +325,7 @@ class CodeMap {
  private:
   struct CodeEntryInfo {
     CodeEntryInfo(CodeEntry* an_entry, unsigned a_size)
-        : entry(an_entry), size(a_size) { }
+        : entry(an_entry), size(a_size) {}
     CodeEntry* entry;
     unsigned size;
   };

src/profiler/profiler-listener.cc

@@ -15,8 +15,9 @@
 namespace v8 {
 namespace internal {
 
-ProfilerListener::ProfilerListener(Isolate* isolate)
-    : isolate_(isolate), function_and_resource_names_(isolate->heap()) {}
+ProfilerListener::ProfilerListener(Isolate* isolate,
+                                   CodeEventObserver* observer)
+    : observer_(observer), function_and_resource_names_(isolate->heap()) {}
 
 ProfilerListener::~ProfilerListener() = default;
 
@@ -303,27 +304,5 @@ CodeEntry* ProfilerListener::NewCodeEntry(
   return raw_code_entry;
 }
 
-void ProfilerListener::AddObserver(CodeEventObserver* observer) {
-  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);
-}
-
-void ProfilerListener::RemoveObserver(CodeEventObserver* observer) {
-  auto it = std::find(observers_.begin(), observers_.end(), observer);
-  if (it != observers_.end()) {
-    observers_.erase(it);
-  }
-  if (observers_.empty()) {
-    isolate_->logger()->RemoveCodeEventListener(this);
-  }
-}
-
 }  // namespace internal
 }  // namespace v8

src/profiler/profiler-listener.h

@@ -23,7 +23,7 @@ class CodeEventObserver {
 
 class ProfilerListener : public CodeEventListener {
  public:
-  explicit ProfilerListener(Isolate* isolate);
+  ProfilerListener(Isolate*, CodeEventObserver*);
   ~ProfilerListener() override;
 
   void CallbackEvent(Name* name, Address entry_point) override;
@@ -61,10 +61,6 @@ class ProfilerListener : public CodeEventListener {
       std::unique_ptr<SourcePositionTable> line_info = nullptr,
       Address instruction_start = kNullAddress);
 
-  void AddObserver(CodeEventObserver* observer);
-  void RemoveObserver(CodeEventObserver* observer);
-  V8_INLINE bool HasObservers() { return !observers_.empty(); }
-
   const char* GetName(Name* name) {
     return function_and_resource_names_.GetName(name);
   }
@@ -77,22 +73,18 @@ class ProfilerListener : public CodeEventListener {
   const char* GetFunctionName(const char* name) {
     return function_and_resource_names_.GetFunctionName(name);
   }
-  size_t entries_count_for_test() const { return code_entries_.size(); }
 
  private:
   void RecordInliningInfo(CodeEntry* entry, AbstractCode* abstract_code);
   void RecordDeoptInlinedFrames(CodeEntry* entry, AbstractCode* abstract_code);
   Name* InferScriptName(Name* name, SharedFunctionInfo* info);
   V8_INLINE void DispatchCodeEvent(const CodeEventsContainer& evt_rec) {
-    for (auto observer : observers_) {
-      observer->CodeEventHandler(evt_rec);
-    }
+    observer_->CodeEventHandler(evt_rec);
   }
 
-  Isolate* isolate_;
+  CodeEventObserver* observer_;
   StringsStorage function_and_resource_names_;
-  std::vector<std::unique_ptr<CodeEntry>> code_entries_;
-  std::vector<CodeEventObserver*> observers_;
+  std::deque<std::unique_ptr<CodeEntry>> code_entries_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfilerListener);
 };

test/cctest/test-cpu-profiler.cc

@@ -164,8 +164,8 @@ TEST(CodeEvents) {
   CpuProfiler profiler(isolate, profiles, generator, processor);
   profiles->StartProfiling("", false);
   processor->Start();
-  ProfilerListener profiler_listener(isolate);
-  profiler_listener.AddObserver(&profiler);
+  ProfilerListener profiler_listener(isolate, &profiler);
+  isolate->logger()->AddCodeEventListener(&profiler_listener);
 
   // Enqueue code creation events.
   const char* aaa_str = "aaa";
@@ -181,7 +181,7 @@ TEST(CodeEvents) {
   // Enqueue a tick event to enable code events processing.
   EnqueueTickSampleEvent(processor, aaa_code->address());
 
-  profiler_listener.RemoveObserver(&profiler);
+  isolate->logger()->RemoveCodeEventListener(&profiler_listener);
   processor->StopSynchronously();
 
   // Check the state of profile generator.
@@ -224,8 +224,8 @@ TEST(TickEvents) {
   CpuProfiler profiler(isolate, profiles, generator, processor);
   profiles->StartProfiling("", false);
   processor->Start();
-  ProfilerListener profiler_listener(isolate);
-  profiler_listener.AddObserver(&profiler);
+  ProfilerListener profiler_listener(isolate, &profiler);
+  isolate->logger()->AddCodeEventListener(&profiler_listener);
 
   profiler_listener.CodeCreateEvent(i::Logger::BUILTIN_TAG, frame1_code, "bbb");
   profiler_listener.CodeCreateEvent(i::Logger::STUB_TAG, frame2_code, "ccc");
@@ -240,7 +240,7 @@ TEST(TickEvents) {
                          frame2_code->raw_instruction_end() - 1,
                          frame1_code->raw_instruction_end() - 1);
 
-  profiler_listener.RemoveObserver(&profiler);
+  isolate->logger()->RemoveCodeEventListener(&profiler_listener);
   processor->StopSynchronously();
   CpuProfile* profile = profiles->StopProfiling("");
   CHECK(profile);
@@ -293,8 +293,7 @@ TEST(Issue1398) {
   CpuProfiler profiler(isolate, profiles, generator, processor);
   profiles->StartProfiling("", false);
   processor->Start();
-  ProfilerListener profiler_listener(isolate);
-  profiler_listener.AddObserver(&profiler);
+  ProfilerListener profiler_listener(isolate, &profiler);
 
   profiler_listener.CodeCreateEvent(i::Logger::BUILTIN_TAG, code, "bbb");
 
@@ -307,7 +306,6 @@ TEST(Issue1398) {
   }
   processor->FinishTickSample();
 
-  profiler_listener.RemoveObserver(&profiler);
   processor->StopSynchronously();
   CpuProfile* profile = profiles->StopProfiling("");
   CHECK(profile);
@@ -1078,8 +1076,7 @@ static void TickLines(bool optimize) {
   CpuProfiler profiler(isolate, profiles, generator, processor);
   profiles->StartProfiling("", false);
   processor->Start();
-  ProfilerListener profiler_listener(isolate);
-  profiler_listener.AddObserver(&profiler);
+  ProfilerListener profiler_listener(isolate, &profiler);
 
   // Enqueue code creation events.
   i::Handle<i::String> str = factory->NewStringFromAsciiChecked(func_name);
@@ -1091,7 +1088,6 @@ static void TickLines(bool optimize) {
   // Enqueue a tick event to enable code events processing.
   EnqueueTickSampleEvent(processor, code_address);
 
-  profiler_listener.RemoveObserver(&profiler);
   processor->StopSynchronously();
 
   CpuProfile* profile = profiles->StopProfiling("");
@@ -2374,10 +2370,10 @@ TEST(CodeEntriesMemoryLeak) {
     v8::CpuProfile* profile = helper.Run(function, nullptr, 0);
     profile->Delete();
   }
-  ProfilerListener* profiler_listener =
-      CcTest::i_isolate()->logger()->EnsureProfilerListener();
 
-  CHECK_GE(10000ul, profiler_listener->entries_count_for_test());
+  i::CpuProfiler* profiler =
+      reinterpret_cast<i::CpuProfiler*>(helper.profiler());
+  CHECK(!profiler->profiler_listener_for_test());
 }
 
 TEST(NativeFrameStackTrace) {