[profiler] Refactor SamplingHeapProfiler

Remove extra allocation observer.
Minor tweaks.

Change-Id: Ic7e6e2f8cb39ff960960cca6cc3ece46438a4bd5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1588405Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61130}

src/profiler/sampling-heap-profiler.cc

@@ -25,10 +25,9 @@ namespace internal {
 //
 // Let u be a uniformly distributed random number between 0 and 1, then
 // next_sample = (- ln u) / λ
-intptr_t SamplingAllocationObserver::GetNextSampleInterval(uint64_t rate) {
-  if (FLAG_sampling_heap_profiler_suppress_randomness) {
+intptr_t SamplingHeapProfiler::Observer::GetNextSampleInterval(uint64_t rate) {
+  if (FLAG_sampling_heap_profiler_suppress_randomness)
     return static_cast<intptr_t>(rate);
-  }
   double u = random_->NextDouble();
   double next = (-base::ieee754::log(u)) * rate;
   return next < kTaggedSize
@@ -55,12 +54,8 @@ SamplingHeapProfiler::SamplingHeapProfiler(
     v8::HeapProfiler::SamplingFlags flags)
     : isolate_(Isolate::FromHeap(heap)),
       heap_(heap),
-      new_space_observer_(new SamplingAllocationObserver(
-          heap_, static_cast<intptr_t>(rate), rate, this,
-          isolate_->random_number_generator())),
-      other_spaces_observer_(new SamplingAllocationObserver(
-          heap_, static_cast<intptr_t>(rate), rate, this,
-          isolate_->random_number_generator())),
+      allocation_observer_(heap_, static_cast<intptr_t>(rate), rate, this,
+                           isolate_->random_number_generator()),
       names_(names),
       profile_root_(nullptr, "(root)", v8::UnboundScript::kNoScriptId, 0,
                     next_node_id()),
@@ -68,13 +63,13 @@ SamplingHeapProfiler::SamplingHeapProfiler(
       rate_(rate),
       flags_(flags) {
   CHECK_GT(rate_, 0u);
-  heap_->AddAllocationObserversToAllSpaces(other_spaces_observer_.get(),
-                                           new_space_observer_.get());
+  heap_->AddAllocationObserversToAllSpaces(&allocation_observer_,
+                                           &allocation_observer_);
 }
 
 SamplingHeapProfiler::~SamplingHeapProfiler() {
-  heap_->RemoveAllocationObserversFromAllSpaces(other_spaces_observer_.get(),
-                                                new_space_observer_.get());
+  heap_->RemoveAllocationObserversFromAllSpaces(&allocation_observer_,
+                                                &allocation_observer_);
 }
 
 void SamplingHeapProfiler::SampleObject(Address soon_object, size_t size) {
@@ -279,7 +274,7 @@ v8::AllocationProfile* SamplingHeapProfiler::GetAllocationProfile() {
   }
   auto profile = new v8::internal::AllocationProfile();
   TranslateAllocationNode(profile, &profile_root_, scripts);
-  profile->samples_ = SamplingHeapProfiler::BuildSamples();
+  profile->samples_ = BuildSamples();
 
   return profile;
 }

src/profiler/sampling-heap-profiler.h

@@ -9,6 +9,7 @@
 #include <map>
 #include <memory>
 #include <unordered_map>
+
 #include "include/v8-profiler.h"
 #include "src/heap/heap.h"
 #include "src/profiler/strings-storage.h"
@@ -105,11 +106,9 @@ class SamplingHeapProfiler {
            SamplingHeapProfiler* profiler_, uint64_t sample_id)
         : size(size_),
           owner(owner_),
-          global(Global<Value>(
-              reinterpret_cast<v8::Isolate*>(profiler_->isolate_), local_)),
+          global(reinterpret_cast<v8::Isolate*>(profiler_->isolate_), local_),
           profiler(profiler_),
           sample_id(sample_id) {}
-    ~Sample() { global.Reset(); }
     const size_t size;
     AllocationNode* const owner;
     Global<Value> global;
@@ -128,6 +127,38 @@ class SamplingHeapProfiler {
   StringsStorage* names() const { return names_; }
 
  private:
+  class Observer : public AllocationObserver {
+   public:
+    Observer(Heap* heap, intptr_t step_size, uint64_t rate,
+             SamplingHeapProfiler* profiler,
+             base::RandomNumberGenerator* random)
+        : AllocationObserver(step_size),
+          profiler_(profiler),
+          heap_(heap),
+          random_(random),
+          rate_(rate) {}
+
+   protected:
+    void Step(int bytes_allocated, Address soon_object, size_t size) override {
+      USE(heap_);
+      DCHECK(heap_->gc_state() == Heap::NOT_IN_GC);
+      if (soon_object) {
+        // TODO(ofrobots): it would be better to sample the next object rather
+        // than skipping this sample epoch if soon_object happens to be null.
+        profiler_->SampleObject(soon_object, size);
+      }
+    }
+
+    intptr_t GetNextStepSize() override { return GetNextSampleInterval(rate_); }
+
+   private:
+    intptr_t GetNextSampleInterval(uint64_t rate);
+    SamplingHeapProfiler* const profiler_;
+    Heap* const heap_;
+    base::RandomNumberGenerator* const random_;
+    uint64_t const rate_;
+  };
+
   void SampleObject(Address soon_object, size_t size);
 
   const std::vector<v8::AllocationProfile::Sample> BuildSamples() const;
@@ -157,8 +188,7 @@ class SamplingHeapProfiler {
   Heap* const heap_;
   uint64_t last_sample_id_ = 0;
   uint32_t last_node_id_ = 0;
-  std::unique_ptr<SamplingAllocationObserver> new_space_observer_;
-  std::unique_ptr<SamplingAllocationObserver> other_spaces_observer_;
+  Observer allocation_observer_;
   StringsStorage* const names_;
   AllocationNode profile_root_;
   std::unordered_map<Sample*, std::unique_ptr<Sample>> samples_;
@@ -166,44 +196,9 @@ class SamplingHeapProfiler {
   const uint64_t rate_;
   v8::HeapProfiler::SamplingFlags flags_;
 
-  friend class SamplingAllocationObserver;
-
   DISALLOW_COPY_AND_ASSIGN(SamplingHeapProfiler);
 };
 
-class SamplingAllocationObserver : public AllocationObserver {
- public:
-  SamplingAllocationObserver(Heap* heap, intptr_t step_size, uint64_t rate,
-                             SamplingHeapProfiler* profiler,
-                             base::RandomNumberGenerator* random)
-      : AllocationObserver(step_size),
-        profiler_(profiler),
-        heap_(heap),
-        random_(random),
-        rate_(rate) {}
-  ~SamplingAllocationObserver() override = default;
-
- protected:
-  void Step(int bytes_allocated, Address soon_object, size_t size) override {
-    USE(heap_);
-    DCHECK(heap_->gc_state() == Heap::NOT_IN_GC);
-    if (soon_object) {
-      // TODO(ofrobots): it would be better to sample the next object rather
-      // than skipping this sample epoch if soon_object happens to be null.
-      profiler_->SampleObject(soon_object, size);
-    }
-  }
-
-  intptr_t GetNextStepSize() override { return GetNextSampleInterval(rate_); }
-
- private:
-  intptr_t GetNextSampleInterval(uint64_t rate);
-  SamplingHeapProfiler* const profiler_;
-  Heap* const heap_;
-  base::RandomNumberGenerator* const random_;
-  uint64_t const rate_;
-};
-
 }  // namespace internal
 }  // namespace v8