[heap] Support removing of observers during Step()

Allow AllocationObserver::Step to remove allocation observers as well.
They could already add new observers (to start incremental marking),
removing will be used when starting StressConcurrentAllocator from
an allocation observer.

Bug: v8:10315
Change-Id: I823ded2f9a408b3fa5269ee8416060d0cabb3162
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2357690Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69420}

src/heap/allocation-observer.cc

@@ -20,7 +20,7 @@ void AllocationCounter::AddAllocationObserver(AllocationObserver* observer) {
 #endif
 
   if (step_in_progress_) {
-    pending_.push_back(AllocationObserverCounter(observer, 0, 0));
+    pending_added_.push_back(AllocationObserverCounter(observer, 0, 0));
     return;
   }
 
@@ -41,12 +41,17 @@ void AllocationCounter::AddAllocationObserver(AllocationObserver* observer) {
 }
 
 void AllocationCounter::RemoveAllocationObserver(AllocationObserver* observer) {
-  DCHECK(!step_in_progress_);
   auto it = std::find_if(observers_.begin(), observers_.end(),
                          [observer](const AllocationObserverCounter& aoc) {
                            return aoc.observer_ == observer;
                          });
   DCHECK_NE(observers_.end(), it);
+
+  if (step_in_progress_) {
+    pending_removed_.insert(observer);
+    return;
+  }
+
   observers_.erase(it);
 
   if (observers_.size() == 0) {
@@ -88,7 +93,8 @@ void AllocationCounter::InvokeAllocationObservers(Address soon_object,
   step_in_progress_ = true;
   size_t step_size = 0;
 
-  DCHECK(pending_.empty());
+  DCHECK(pending_added_.empty());
+  DCHECK(pending_removed_.empty());
 
   for (AllocationObserverCounter& aoc : observers_) {
     if (aoc.next_counter_ - current_counter_ <= aligned_object_size) {
@@ -113,7 +119,7 @@ void AllocationCounter::InvokeAllocationObservers(Address soon_object,
   CHECK(step_run);
 
   // Now process newly added allocation observers.
-  for (AllocationObserverCounter& aoc : pending_) {
+  for (AllocationObserverCounter& aoc : pending_added_) {
     size_t observer_step_size = aoc.observer_->GetNextStepSize();
     aoc.prev_counter_ = current_counter_;
     aoc.next_counter_ =
@@ -125,7 +131,29 @@ void AllocationCounter::InvokeAllocationObservers(Address soon_object,
     observers_.push_back(aoc);
   }
 
-  pending_.clear();
+  pending_added_.clear();
+
+  if (!pending_removed_.empty()) {
+    observers_.erase(
+        std::remove_if(observers_.begin(), observers_.end(),
+                       [this](const AllocationObserverCounter& aoc) {
+                         return pending_removed_.count(aoc.observer_) != 0;
+                       }));
+    pending_removed_.clear();
+
+    // Some observers were removed, recalculate step size.
+    step_size = 0;
+    for (AllocationObserverCounter& aoc : observers_) {
+      size_t left_in_step = aoc.next_counter_ - current_counter_;
+      step_size = step_size ? Min(step_size, left_in_step) : left_in_step;
+    }
+
+    if (observers_.empty()) {
+      next_counter_ = current_counter_ = 0;
+      step_in_progress_ = false;
+      return;
+    }
+  }
 
   next_counter_ = current_counter_ + step_size;
   step_in_progress_ = false;

src/heap/allocation-observer.h

@@ -6,6 +6,7 @@
 #define V8_HEAP_ALLOCATION_OBSERVER_H_
 
 #include <cstdint>
+#include <unordered_set>
 #include <vector>
 
 #include "src/common/globals.h"
@@ -67,7 +68,8 @@ class AllocationCounter {
   };
 
   std::vector<AllocationObserverCounter> observers_;
-  std::vector<AllocationObserverCounter> pending_;
+  std::vector<AllocationObserverCounter> pending_added_;
+  std::unordered_set<AllocationObserver*> pending_removed_;
 
   bool paused_;
 

test/unittests/heap/allocation-observer-unittest.cc

@@ -107,5 +107,74 @@ TEST(AllocationObserverTest, Step) {
            20 /* aligned_object_size */ + 100 /* smallest step size*/);
 }
 
+namespace {
+class RecursiveAddObserver : public AllocationObserver {
+ public:
+  explicit RecursiveAddObserver(size_t step_size, AllocationCounter* counter,
+                                AllocationObserver* observer)
+      : AllocationObserver(step_size), counter_(counter), observer_(observer) {}
+
+  void Step(int bytes_allocated, Address soon_object, size_t size) override {
+    counter_->AddAllocationObserver(observer_);
+  }
+
+ private:
+  AllocationCounter* counter_;
+  AllocationObserver* observer_;
+};
+}  // namespace
+
+TEST(AllocationObserverTest, RecursiveAdd) {
+  AllocationCounter counter;
+  const Address kSomeObjectAddress = 8;
+
+  UnusedObserver observer50(50);
+  RecursiveAddObserver observer100(100, &counter, &observer50);
+
+  counter.AddAllocationObserver(&observer100);
+
+  CHECK_EQ(counter.NextBytes(), 100);
+  counter.AdvanceAllocationObservers(90);
+  counter.InvokeAllocationObservers(kSomeObjectAddress, 10, 10);
+
+  CHECK_EQ(counter.NextBytes(),
+           10 /* aligned_object_size */ + 50 /* smallest step size */);
+}
+
+namespace {
+class RecursiveRemoveObserver : public AllocationObserver {
+ public:
+  explicit RecursiveRemoveObserver(size_t step_size, AllocationCounter* counter,
+                                   AllocationObserver* observer)
+      : AllocationObserver(step_size), counter_(counter), observer_(observer) {}
+
+  void Step(int bytes_allocated, Address soon_object, size_t size) override {
+    counter_->RemoveAllocationObserver(observer_);
+  }
+
+ private:
+  AllocationCounter* counter_;
+  AllocationObserver* observer_;
+};
+}  // namespace
+
+TEST(AllocationObserverTest, RecursiveRemove) {
+  AllocationCounter counter;
+  const Address kSomeObjectAddress = 8;
+
+  UnusedObserver observer75(75);
+  RecursiveRemoveObserver observer50(50, &counter, &observer75);
+
+  counter.AddAllocationObserver(&observer50);
+  counter.AddAllocationObserver(&observer75);
+
+  CHECK_EQ(counter.NextBytes(), 50);
+  counter.AdvanceAllocationObservers(40);
+  counter.InvokeAllocationObservers(kSomeObjectAddress, 10, 10);
+
+  CHECK_EQ(counter.NextBytes(),
+           10 /* aligned_object_size */ + 50 /* smallest step size */);
+}
+
 }  // namespace internal
 }  // namespace v8