Reland: [heap] Add missing steps for Add/Remove observers

This is a reland of https://chromium-review.googlesource.com/c/v8/v8/+/715118.
The UI doesn't allow me to reland this automatically due to merge conflicts
so I ended up doing this manually.

Change-Id: Ic013b61f459cb1e91e24203fb31ae7c14f3754f8
Reviewed-on: https://chromium-review.googlesource.com/720136Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
Cr-Commit-Position: refs/heads/master@{#48606}

src/heap/heap.cc

@@ -6378,5 +6378,16 @@ void Heap::CreateObjectStats() {
   }
 }
 
+void AllocationObserver::AllocationStep(int bytes_allocated,
+                                        Address soon_object, size_t size) {
+  DCHECK_GE(bytes_allocated, 0);
+  bytes_to_next_step_ -= bytes_allocated;
+  if (bytes_to_next_step_ <= 0) {
+    Step(static_cast<int>(step_size_ - bytes_to_next_step_), soon_object, size);
+    step_size_ = GetNextStepSize();
+    bytes_to_next_step_ = step_size_;
+  }
+}
+
 }  // namespace internal
 }  // namespace v8

src/heap/heap.h

@@ -2654,15 +2654,7 @@ class AllocationObserver {
   // Called each time the observed space does an allocation step. This may be
   // more frequently than the step_size we are monitoring (e.g. when there are
   // multiple observers, or when page or space boundary is encountered.)
-  void AllocationStep(int bytes_allocated, Address soon_object, size_t size) {
-    bytes_to_next_step_ -= bytes_allocated;
-    if (bytes_to_next_step_ <= 0) {
-      Step(static_cast<int>(step_size_ - bytes_to_next_step_), soon_object,
-           size);
-      step_size_ = GetNextStepSize();
-      bytes_to_next_step_ = step_size_;
-    }
-  }
+  void AllocationStep(int bytes_allocated, Address soon_object, size_t size);
 
  protected:
   intptr_t step_size() const { return step_size_; }

src/heap/scavenge-job.cc

@@ -103,7 +103,7 @@ void ScavengeJob::ScheduleIdleTaskIfNeeded(Heap* heap, int bytes_allocated) {
 
 
 void ScavengeJob::ScheduleIdleTask(Heap* heap) {
-  if (!idle_task_pending_) {
+  if (!idle_task_pending_ && heap->use_tasks()) {
     v8::Isolate* isolate = reinterpret_cast<v8::Isolate*>(heap->isolate());
     if (V8::GetCurrentPlatform()->IdleTasksEnabled(isolate)) {
       idle_task_pending_ = true;

src/heap/spaces-inl.h

@@ -459,6 +459,12 @@ AllocationResult NewSpace::AllocateRawUnaligned(int size_in_bytes) {
 
 AllocationResult NewSpace::AllocateRaw(int size_in_bytes,
                                        AllocationAlignment alignment) {
+  if (top() < top_on_previous_step_) {
+    // Generated code decreased the top() pointer to do folded allocations
+    DCHECK_EQ(Page::FromAddress(top()),
+              Page::FromAddress(top_on_previous_step_));
+    top_on_previous_step_ = top();
+  }
 #ifdef V8_HOST_ARCH_32_BIT
   return alignment == kDoubleAligned
              ? AllocateRawAligned(size_in_bytes, kDoubleAligned)

src/heap/spaces.cc

@@ -1696,12 +1696,7 @@ void PagedSpace::EmptyAllocationInfo() {
     }
   }
 
-  if (top_on_previous_step_) {
-    DCHECK(current_top >= top_on_previous_step_);
-    AllocationStep(static_cast<int>(current_top - top_on_previous_step_),
-                   nullptr, 0);
-    top_on_previous_step_ = 0;
-  }
+  InlineAllocationStep(current_top, nullptr, nullptr, 0);
   SetTopAndLimit(nullptr, nullptr);
   DCHECK_GE(current_limit, current_top);
   Free(current_top, current_limit - current_top);
@@ -2153,6 +2148,30 @@ void NewSpace::StartNextInlineAllocationStep() {
   }
 }
 
+// TODO(ofrobots): refactor into SpaceWithLinearArea
+void NewSpace::AddAllocationObserver(AllocationObserver* observer) {
+  InlineAllocationStep(top(), top(), nullptr, 0);
+  Space::AddAllocationObserver(observer);
+}
+
+// TODO(ofrobots): refactor into SpaceWithLinearArea
+void PagedSpace::AddAllocationObserver(AllocationObserver* observer) {
+  InlineAllocationStep(top(), top(), nullptr, 0);
+  Space::AddAllocationObserver(observer);
+}
+
+// TODO(ofrobots): refactor into SpaceWithLinearArea
+void NewSpace::RemoveAllocationObserver(AllocationObserver* observer) {
+  InlineAllocationStep(top(), top(), nullptr, 0);
+  Space::RemoveAllocationObserver(observer);
+}
+
+// TODO(ofrobots): refactor into SpaceWithLinearArea
+void PagedSpace::RemoveAllocationObserver(AllocationObserver* observer) {
+  InlineAllocationStep(top(), top(), nullptr, 0);
+  Space::RemoveAllocationObserver(observer);
+}
+
 void NewSpace::PauseAllocationObservers() {
   // Do a step to account for memory allocated so far.
   InlineAllocationStep(top(), top(), nullptr, 0);
@@ -2163,10 +2182,7 @@ void NewSpace::PauseAllocationObservers() {
 
 void PagedSpace::PauseAllocationObservers() {
   // Do a step to account for memory allocated so far.
-  if (top_on_previous_step_) {
-    int bytes_allocated = static_cast<int>(top() - top_on_previous_step_);
-    AllocationStep(bytes_allocated, nullptr, 0);
-  }
+  InlineAllocationStep(top(), nullptr, nullptr, 0);
   Space::PauseAllocationObservers();
   top_on_previous_step_ = 0;
 }
@@ -2184,13 +2200,21 @@ void PagedSpace::ResumeAllocationObservers() {
   StartNextInlineAllocationStep();
 }
 
+// TODO(ofrobots): refactor into SpaceWithLinearArea
+void PagedSpace::InlineAllocationStep(Address top, Address new_top,
+                                      Address soon_object, size_t size) {
+  if (top_on_previous_step_) {
+    int bytes_allocated = static_cast<int>(top - top_on_previous_step_);
+    AllocationStep(bytes_allocated, soon_object, static_cast<int>(size));
+    top_on_previous_step_ = new_top;
+  }
+}
+
 void NewSpace::InlineAllocationStep(Address top, Address new_top,
                                     Address soon_object, size_t size) {
   if (top_on_previous_step_) {
     int bytes_allocated = static_cast<int>(top - top_on_previous_step_);
-    for (AllocationObserver* observer : allocation_observers_) {
-      observer->AllocationStep(bytes_allocated, soon_object, size);
-    }
+    AllocationStep(bytes_allocated, soon_object, static_cast<int>(size));
     top_on_previous_step_ = new_top;
   }
 }

src/heap/spaces.h

@@ -903,9 +903,11 @@ class Space : public Malloced {
   // Identity used in error reporting.
   AllocationSpace identity() { return id_; }
 
-  void AddAllocationObserver(AllocationObserver* observer);
+  V8_EXPORT_PRIVATE virtual void AddAllocationObserver(
+      AllocationObserver* observer);
 
-  void RemoveAllocationObserver(AllocationObserver* observer);
+  V8_EXPORT_PRIVATE virtual void RemoveAllocationObserver(
+      AllocationObserver* observer);
 
   V8_EXPORT_PRIVATE virtual void PauseAllocationObservers();
 
@@ -2078,9 +2080,14 @@ class V8_EXPORT_PRIVATE PagedSpace : NON_EXPORTED_BASE(public Space) {
 
   void ResetFreeList() { free_list_.Reset(); }
 
+  void AddAllocationObserver(AllocationObserver* observer) override;
+  void RemoveAllocationObserver(AllocationObserver* observer) override;
   void PauseAllocationObservers() override;
   void ResumeAllocationObservers() override;
 
+  void InlineAllocationStep(Address top, Address new_top, Address soon_object,
+                            size_t size);
+
   // Empty space allocation info, returning unused area to free list.
   void EmptyAllocationInfo();
 
@@ -2730,6 +2737,8 @@ class NewSpace : public Space {
 
   SemiSpace* active_space() { return &to_space_; }
 
+  void AddAllocationObserver(AllocationObserver* observer) override;
+  void RemoveAllocationObserver(AllocationObserver* observer) override;
   void PauseAllocationObservers() override;
   void ResumeAllocationObservers() override;
 

test/cctest/test-heap-profiler.cc

@@ -3137,3 +3137,28 @@ TEST(SamplingHeapProfilerPretenuredInlineAllocations) {
 
   CHECK_GE(count, 9000);
 }
+
+TEST(SamplingHeapProfilerLargeInterval) {
+  v8::HandleScope scope(v8::Isolate::GetCurrent());
+  LocalContext env;
+  v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
+
+  // Suppress randomness to avoid flakiness in tests.
+  v8::internal::FLAG_sampling_heap_profiler_suppress_randomness = true;
+
+  heap_profiler->StartSamplingHeapProfiler(512 * 1024);
+
+  for (int i = 0; i < 8 * 1024; ++i) {
+    CcTest::i_isolate()->factory()->NewFixedArray(1024);
+  }
+
+  std::unique_ptr<v8::AllocationProfile> profile(
+      heap_profiler->GetAllocationProfile());
+  CHECK(profile);
+  const char* names[] = {"(EXTERNAL)"};
+  auto node = FindAllocationProfileNode(env->GetIsolate(), *profile,
+                                        ArrayVector(names));
+  CHECK(node);
+
+  heap_profiler->StopSamplingHeapProfiler();
+}