[cpu-profiler] Make ProfilerEventsProcessor the CodeEventObserver.

Currently ProfilerListener channels the code events to Processor
via CpuProfiler - we don't need this indirection and can just hook
it up directly. This also makes it easier to test because we don't need
a CpuProfiler object just to test the Processor.

Drive-by cleanup:
- Remove NUMBER_OF_TYPES from CodeEventRecord as it is not used.
- Remove Isolate* parameter from AddDeoptStack and AddCurrentStack as
  a Processor object is only ever for one Isolate. Store the Isolate*
  on the ProfilerEventsProcessor object itself.
- Remove the default case from switch in ProcessCodeEvent().

Bug: v8:5193
Change-Id: I26c1a46b0eec34b5248b707d1997c3a9409a9604
Reviewed-on: https://chromium-review.googlesource.com/c/1286341Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56740}

src/profiler/cpu-profiler.cc

@@ -45,17 +45,19 @@ class CpuSampler : public sampler::Sampler {
   SamplingEventsProcessor* processor_;
 };
 
-ProfilerEventsProcessor::ProfilerEventsProcessor(ProfileGenerator* generator)
+ProfilerEventsProcessor::ProfilerEventsProcessor(Isolate* isolate,
+                                                 ProfileGenerator* generator)
     : Thread(Thread::Options("v8:ProfEvntProc", kProfilerStackSize)),
       generator_(generator),
       running_(1),
       last_code_event_id_(0),
-      last_processed_code_event_id_(0) {}
+      last_processed_code_event_id_(0),
+      isolate_(isolate) {}
 
 SamplingEventsProcessor::SamplingEventsProcessor(Isolate* isolate,
                                                  ProfileGenerator* generator,
                                                  base::TimeDelta period)
-    : ProfilerEventsProcessor(generator),
+    : ProfilerEventsProcessor(isolate, generator),
       sampler_(new CpuSampler(isolate, this)),
       period_(period) {
   sampler_->IncreaseProfilingDepth();
@@ -72,31 +74,29 @@ void ProfilerEventsProcessor::Enqueue(const CodeEventsContainer& event) {
   events_buffer_.Enqueue(event);
 }
 
-
-void ProfilerEventsProcessor::AddDeoptStack(Isolate* isolate, Address from,
-                                            int fp_to_sp_delta) {
+void ProfilerEventsProcessor::AddDeoptStack(Address from, int fp_to_sp_delta) {
   TickSampleEventRecord record(last_code_event_id_);
   RegisterState regs;
-  Address fp = isolate->c_entry_fp(isolate->thread_local_top());
+  Address fp = isolate_->c_entry_fp(isolate_->thread_local_top());
   regs.sp = reinterpret_cast<void*>(fp - fp_to_sp_delta);
   regs.fp = reinterpret_cast<void*>(fp);
   regs.pc = reinterpret_cast<void*>(from);
-  record.sample.Init(isolate, regs, TickSample::kSkipCEntryFrame, false, false);
+  record.sample.Init(isolate_, regs, TickSample::kSkipCEntryFrame, false,
+                     false);
   ticks_from_vm_buffer_.Enqueue(record);
 }
 
-void ProfilerEventsProcessor::AddCurrentStack(Isolate* isolate,
-                                              bool update_stats) {
+void ProfilerEventsProcessor::AddCurrentStack(bool update_stats) {
   TickSampleEventRecord record(last_code_event_id_);
   RegisterState regs;
-  StackFrameIterator it(isolate);
+  StackFrameIterator it(isolate_);
   if (!it.done()) {
     StackFrame* frame = it.frame();
     regs.sp = reinterpret_cast<void*>(frame->sp());
     regs.fp = reinterpret_cast<void*>(frame->fp());
     regs.pc = reinterpret_cast<void*>(frame->pc());
   }
-  record.sample.Init(isolate, regs, TickSample::kSkipCEntryFrame, update_stats,
+  record.sample.Init(isolate_, regs, TickSample::kSkipCEntryFrame, update_stats,
                      false);
   ticks_from_vm_buffer_.Enqueue(record);
 }
@@ -133,6 +133,28 @@ bool ProfilerEventsProcessor::ProcessCodeEvent() {
   return false;
 }
 
+void ProfilerEventsProcessor::CodeEventHandler(
+    const CodeEventsContainer& evt_rec) {
+  switch (evt_rec.generic.type) {
+    case CodeEventRecord::CODE_CREATION:
+    case CodeEventRecord::CODE_MOVE:
+    case CodeEventRecord::CODE_DISABLE_OPT:
+      Enqueue(evt_rec);
+      break;
+    case CodeEventRecord::CODE_DEOPT: {
+      const CodeDeoptEventRecord* rec = &evt_rec.CodeDeoptEventRecord_;
+      Address pc = rec->pc;
+      int fp_to_sp_delta = rec->fp_to_sp_delta;
+      Enqueue(evt_rec);
+      AddDeoptStack(pc, fp_to_sp_delta);
+      break;
+    }
+    case CodeEventRecord::NONE:
+    case CodeEventRecord::REPORT_BUILTIN:
+      UNREACHABLE();
+  }
+}
+
 ProfilerEventsProcessor::SampleProcessingResult
 SamplingEventsProcessor::ProcessOneSample() {
   TickSampleEventRecord record1;
@@ -233,26 +255,6 @@ void CpuProfiler::DeleteProfile(CpuProfile* profile) {
   }
 }
 
-void CpuProfiler::CodeEventHandler(const CodeEventsContainer& evt_rec) {
-  switch (evt_rec.generic.type) {
-    case CodeEventRecord::CODE_CREATION:
-    case CodeEventRecord::CODE_MOVE:
-    case CodeEventRecord::CODE_DISABLE_OPT:
-      processor_->Enqueue(evt_rec);
-      break;
-    case CodeEventRecord::CODE_DEOPT: {
-      const CodeDeoptEventRecord* rec = &evt_rec.CodeDeoptEventRecord_;
-      Address pc = rec->pc;
-      int fp_to_sp_delta = rec->fp_to_sp_delta;
-      processor_->Enqueue(evt_rec);
-      processor_->AddDeoptStack(isolate_, pc, fp_to_sp_delta);
-      break;
-    }
-    default:
-      UNREACHABLE();
-  }
-}
-
 namespace {
 
 class CpuProfilersManager {
@@ -345,7 +347,7 @@ void CpuProfiler::CollectSample(Isolate* isolate) {
 
 void CpuProfiler::CollectSample() {
   if (processor_) {
-    processor_->AddCurrentStack(isolate_);
+    processor_->AddCurrentStack();
   }
 }
 
@@ -365,7 +367,7 @@ void CpuProfiler::StartProfiling(String* title, bool record_samples,
 
 void CpuProfiler::StartProcessorIfNotStarted() {
   if (processor_) {
-    processor_->AddCurrentStack(isolate_);
+    processor_->AddCurrentStack();
     return;
   }
   Logger* logger = isolate_->logger();
@@ -382,7 +384,7 @@ void CpuProfiler::StartProcessorIfNotStarted() {
   processor_.reset(new SamplingEventsProcessor(isolate_, generator_.get(),
                                                sampling_interval_));
   if (!profiler_listener_) {
-    profiler_listener_.reset(new ProfilerListener(isolate_, this));
+    profiler_listener_.reset(new ProfilerListener(isolate_, processor_.get()));
   }
   logger->AddCodeEventListener(profiler_listener_.get());
   is_profiling_ = true;
@@ -398,7 +400,7 @@ void CpuProfiler::StartProcessorIfNotStarted() {
     LogBuiltins();
   }
   // Enable stack sampling.
-  processor_->AddCurrentStack(isolate_);
+  processor_->AddCurrentStack();
   processor_->StartSynchronously();
 }
 

src/profiler/cpu-profiler.h

@@ -42,7 +42,6 @@ class CodeEventRecord {
   enum Type {
     NONE = 0,
     CODE_EVENTS_TYPE_LIST(DECLARE_TYPE)
-    NUMBER_OF_TYPES
   };
 #undef DECLARE_TYPE
 
@@ -131,24 +130,27 @@ class CodeEventsContainer {
 
 // This class implements both the profile events processor thread and
 // methods called by event producers: VM and stack sampler threads.
-class ProfilerEventsProcessor : public base::Thread {
+class ProfilerEventsProcessor : public base::Thread, public CodeEventObserver {
  public:
   virtual ~ProfilerEventsProcessor();
 
+  void CodeEventHandler(const CodeEventsContainer& evt_rec) override;
+
   // Thread control.
-  virtual void Run() = 0;
+  void Run() override = 0;
   void StopSynchronously();
   V8_INLINE bool running() { return !!base::Relaxed_Load(&running_); }
   void Enqueue(const CodeEventsContainer& event);
 
   // Puts current stack into the tick sample events buffer.
-  void AddCurrentStack(Isolate* isolate, bool update_stats = false);
-  void AddDeoptStack(Isolate* isolate, Address from, int fp_to_sp_delta);
+  void AddCurrentStack(bool update_stats = false);
+  void AddDeoptStack(Address from, int fp_to_sp_delta);
   // Puts the given sample into the tick sample events buffer.
   void AddSample(TickSample sample);
 
  protected:
-  explicit ProfilerEventsProcessor(ProfileGenerator* generator);
+  explicit ProfilerEventsProcessor(Isolate* isolate,
+                                   ProfileGenerator* generator);
 
   // Called from events processing thread (Run() method.)
   bool ProcessCodeEvent();
@@ -166,6 +168,7 @@ class ProfilerEventsProcessor : public base::Thread {
   LockedQueue<TickSampleEventRecord> ticks_from_vm_buffer_;
   std::atomic<unsigned> last_code_event_id_;
   unsigned last_processed_code_event_id_;
+  Isolate* isolate_;
 };
 
 class SamplingEventsProcessor : public ProfilerEventsProcessor {
@@ -202,7 +205,7 @@ class SamplingEventsProcessor : public ProfilerEventsProcessor {
   const base::TimeDelta period_;  // Samples & code events processing period.
 };
 
-class CpuProfiler : public CodeEventObserver {
+class CpuProfiler {
  public:
   explicit CpuProfiler(Isolate* isolate);
 
@@ -210,7 +213,7 @@ class CpuProfiler : public CodeEventObserver {
               ProfileGenerator* test_generator,
               ProfilerEventsProcessor* test_processor);
 
-  ~CpuProfiler() override;
+  ~CpuProfiler();
 
   static void CollectSample(Isolate* isolate);
 
@@ -228,8 +231,6 @@ class CpuProfiler : public CodeEventObserver {
   void DeleteAllProfiles();
   void DeleteProfile(CpuProfile* profile);
 
-  void CodeEventHandler(const CodeEventsContainer& evt_rec) override;
-
   bool is_profiling() const { return is_profiling_; }
 
   ProfileGenerator* generator() const { return generator_.get(); }

test/cctest/test-cpu-profiler.cc

@@ -162,10 +162,8 @@ TEST(CodeEvents) {
   ProfileGenerator* generator = new ProfileGenerator(profiles);
   ProfilerEventsProcessor* processor = new SamplingEventsProcessor(
       isolate, generator, v8::base::TimeDelta::FromMicroseconds(100));
-  CpuProfiler profiler(isolate, profiles, generator, processor);
-  profiles->StartProfiling("", false);
   processor->Start();
-  ProfilerListener profiler_listener(isolate, &profiler);
+  ProfilerListener profiler_listener(isolate, processor);
   isolate->logger()->AddCodeEventListener(&profiler_listener);
 
   // Enqueue code creation events.
@@ -227,7 +225,7 @@ TEST(TickEvents) {
   CpuProfiler profiler(isolate, profiles, generator, processor);
   profiles->StartProfiling("", false);
   processor->Start();
-  ProfilerListener profiler_listener(isolate, &profiler);
+  ProfilerListener profiler_listener(isolate, processor);
   isolate->logger()->AddCodeEventListener(&profiler_listener);
 
   profiler_listener.CodeCreateEvent(i::Logger::BUILTIN_TAG, frame1_code, "bbb");
@@ -296,7 +294,7 @@ TEST(Issue1398) {
   CpuProfiler profiler(isolate, profiles, generator, processor);
   profiles->StartProfiling("", false);
   processor->Start();
-  ProfilerListener profiler_listener(isolate, &profiler);
+  ProfilerListener profiler_listener(isolate, processor);
 
   profiler_listener.CodeCreateEvent(i::Logger::BUILTIN_TAG, code, "bbb");
 
@@ -1152,7 +1150,7 @@ static void TickLines(bool optimize) {
   CpuProfiler profiler(isolate, profiles, generator, processor);
   profiles->StartProfiling("", false);
   processor->Start();
-  ProfilerListener profiler_listener(isolate, &profiler);
+  ProfilerListener profiler_listener(isolate, processor);
 
   // Enqueue code creation events.
   i::Handle<i::String> str = factory->NewStringFromAsciiChecked(func_name);
@@ -1716,13 +1714,13 @@ TEST(IdleTime) {
   i::ProfilerEventsProcessor* processor =
       reinterpret_cast<i::CpuProfiler*>(cpu_profiler)->processor();
 
-  processor->AddCurrentStack(isolate, true);
+  processor->AddCurrentStack(true);
   isolate->SetIdle(true);
   for (int i = 0; i < 3; i++) {
-    processor->AddCurrentStack(isolate, true);
+    processor->AddCurrentStack(true);
   }
   isolate->SetIdle(false);
-  processor->AddCurrentStack(isolate, true);
+  processor->AddCurrentStack(true);
 
   v8::CpuProfile* profile = cpu_profiler->StopProfiling(profile_name);
   CHECK(profile);