[heap] Refactor MemoryController and HeapController

- Move generic pieces into MemoryController.
- Keep V8's specific factor computations in HeapController.

Bug: chromium:948807
Change-Id: I1c1fc0516a429b19ce6458f75888b3f9d51824ac
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1617678
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61626}

src/heap/heap-controller.cc

@@ -49,17 +49,17 @@ namespace internal {
 //   F * (1 - MU / (R * (1 - MU))) = 1
 //   F * (R * (1 - MU) - MU) / (R * (1 - MU)) = 1
 //   F = R * (1 - MU) / (R * (1 - MU) - MU)
-double MemoryController::GrowingFactor(double gc_speed, double mutator_speed,
-                                       double max_factor) {
+double HeapController::GrowingFactor(double gc_speed, double mutator_speed,
+                                     double max_factor) {
   DCHECK_LE(min_growing_factor_, max_factor);
   DCHECK_GE(max_growing_factor_, max_factor);
   if (gc_speed == 0 || mutator_speed == 0) return max_factor;
 
   const double speed_ratio = gc_speed / mutator_speed;
 
-  const double a = speed_ratio * (1 - target_mutator_utilization_);
-  const double b = speed_ratio * (1 - target_mutator_utilization_) -
-                   target_mutator_utilization_;
+  const double a = speed_ratio * (1 - kTargetMutatorUtilization);
+  const double b =
+      speed_ratio * (1 - kTargetMutatorUtilization) - kTargetMutatorUtilization;
 
   // The factor is a / b, but we need to check for small b first.
   double factor = (a < b * max_factor) ? a / b : max_factor;
@@ -68,27 +68,19 @@ double MemoryController::GrowingFactor(double gc_speed, double mutator_speed,
   return factor;
 }
 
-size_t MemoryController::CalculateAllocationLimit(
-    size_t curr_size, size_t max_size, double max_factor, double gc_speed,
-    double mutator_speed, size_t new_space_capacity,
+size_t MemoryController::CalculateAllocationLimitBase(
+    size_t curr_size, size_t max_size, double factor, size_t additional_bytes,
     Heap::HeapGrowingMode growing_mode) {
-  double factor = GrowingFactor(gc_speed, mutator_speed, max_factor);
-
-  if (FLAG_trace_gc_verbose) {
-    Isolate::FromHeap(heap_)->PrintWithTimestamp(
-        "%s factor %.1f based on mu=%.3f, speed_ratio=%.f "
-        "(gc=%.f, mutator=%.f)\n",
-        ControllerName(), factor, target_mutator_utilization_,
-        gc_speed / mutator_speed, gc_speed, mutator_speed);
-  }
-
-  if (growing_mode == Heap::HeapGrowingMode::kConservative ||
-      growing_mode == Heap::HeapGrowingMode::kSlow) {
-    factor = Min(factor, conservative_growing_factor_);
-  }
-
-  if (growing_mode == Heap::HeapGrowingMode::kMinimal) {
-    factor = min_growing_factor_;
+  switch (growing_mode) {
+    case Heap::HeapGrowingMode::kConservative:
+    case Heap::HeapGrowingMode::kSlow:
+      factor = Min(factor, conservative_growing_factor_);
+      break;
+    case Heap::HeapGrowingMode::kMinimal:
+      factor = min_growing_factor_;
+      break;
+    case Heap::HeapGrowingMode::kDefault:
+      break;
   }
 
   if (FLAG_heap_growing_percent > 0) {
@@ -97,20 +89,19 @@ size_t MemoryController::CalculateAllocationLimit(
 
   CHECK_LT(1.0, factor);
   CHECK_LT(0, curr_size);
-  uint64_t limit = static_cast<uint64_t>(curr_size * factor);
-  limit = Max(limit, static_cast<uint64_t>(curr_size) +
-                         MinimumAllocationLimitGrowingStep(growing_mode));
-  limit += new_space_capacity;
-  uint64_t halfway_to_the_max =
+  const uint64_t limit =
+      Max(static_cast<uint64_t>(curr_size * factor),
+          static_cast<uint64_t>(curr_size) +
+              MinimumAllocationLimitGrowingStep(growing_mode)) +
+      additional_bytes;
+  const uint64_t halfway_to_the_max =
       (static_cast<uint64_t>(curr_size) + max_size) / 2;
-  size_t result = static_cast<size_t>(Min(limit, halfway_to_the_max));
-
+  const size_t result = static_cast<size_t>(Min(limit, halfway_to_the_max));
   if (FLAG_trace_gc_verbose) {
     Isolate::FromHeap(heap_)->PrintWithTimestamp(
-        "%s Limit: old size: %zu KB, new limit: %zu KB (%.1f)\n",
+        "[%s] Limit: old size: %zu KB, new limit: %zu KB (%.1f)\n",
         ControllerName(), curr_size / KB, result / KB, factor);
   }
-
   return result;
 }
 
@@ -125,9 +116,9 @@ size_t MemoryController::MinimumAllocationLimitGrowingStep(
 }
 
 double HeapController::MaxGrowingFactor(size_t curr_max_size) {
-  const double min_small_factor = 1.3;
-  const double max_small_factor = 2.0;
-  const double high_factor = 4.0;
+  constexpr double kMinSmallFactor = 1.3;
+  constexpr double kMaxSmallFactor = 2.0;
+  constexpr double kHighFactor = 4.0;
 
   size_t max_size_in_mb = curr_max_size / MB;
   max_size_in_mb = Max(max_size_in_mb, kMinSize);
@@ -135,7 +126,7 @@ double HeapController::MaxGrowingFactor(size_t curr_max_size) {
   // If we are on a device with lots of memory, we allow a high heap
   // growing factor.
   if (max_size_in_mb >= kMaxSize) {
-    return high_factor;
+    return kHighFactor;
   }
 
   DCHECK_GE(max_size_in_mb, kMinSize);
@@ -143,11 +134,29 @@ double HeapController::MaxGrowingFactor(size_t curr_max_size) {
 
   // On smaller devices we linearly scale the factor: (X-A)/(B-A)*(D-C)+C
   double factor = (max_size_in_mb - kMinSize) *
-                      (max_small_factor - min_small_factor) /
+                      (kMaxSmallFactor - kMinSmallFactor) /
                       (kMaxSize - kMinSize) +
-                  min_small_factor;
+                  kMinSmallFactor;
   return factor;
 }
 
+size_t HeapController::CalculateAllocationLimit(
+    size_t curr_size, size_t max_size, double gc_speed, double mutator_speed,
+    size_t new_space_capacity, Heap::HeapGrowingMode growing_mode) {
+  const double max_factor = MaxGrowingFactor(max_size);
+  const double factor = GrowingFactor(gc_speed, mutator_speed, max_factor);
+
+  if (FLAG_trace_gc_verbose) {
+    Isolate::FromHeap(heap_)->PrintWithTimestamp(
+        "[%s] factor %.1f based on mu=%.3f, speed_ratio=%.f "
+        "(gc=%.f, mutator=%.f)\n",
+        ControllerName(), factor, kTargetMutatorUtilization,
+        gc_speed / mutator_speed, gc_speed, mutator_speed);
+  }
+
+  return CalculateAllocationLimitBase(curr_size, max_size, factor,
+                                      new_space_capacity, growing_mode);
+}
+
 }  // namespace internal
 }  // namespace v8

src/heap/heap-controller.h

@@ -15,42 +15,31 @@ namespace internal {
 
 class V8_EXPORT_PRIVATE MemoryController {
  public:
+  virtual ~MemoryController() = default;
+
+  // Computes the growing step when the limit increases.
+  size_t MinimumAllocationLimitGrowingStep(Heap::HeapGrowingMode growing_mode);
+
+ protected:
   MemoryController(Heap* heap, double min_growing_factor,
                    double max_growing_factor,
-                   double conservative_growing_factor,
-                   double target_mutator_utilization)
+                   double conservative_growing_factor)
       : heap_(heap),
         min_growing_factor_(min_growing_factor),
         max_growing_factor_(max_growing_factor),
-        conservative_growing_factor_(conservative_growing_factor),
-        target_mutator_utilization_(target_mutator_utilization) {}
-  virtual ~MemoryController() = default;
+        conservative_growing_factor_(conservative_growing_factor) {}
 
   // Computes the allocation limit to trigger the next garbage collection.
-  size_t CalculateAllocationLimit(size_t curr_size, size_t max_size,
-                                  double max_factor, double gc_speed,
-                                  double mutator_speed,
-                                  size_t new_space_capacity,
-                                  Heap::HeapGrowingMode growing_mode);
+  size_t CalculateAllocationLimitBase(size_t curr_size, size_t max_size,
+                                      double factor, size_t additional_bytes,
+                                      Heap::HeapGrowingMode growing_mode);
 
-  // Computes the growing step when the limit increases.
-  size_t MinimumAllocationLimitGrowingStep(Heap::HeapGrowingMode growing_mode);
-
- protected:
-  double GrowingFactor(double gc_speed, double mutator_speed,
-                       double max_factor);
   virtual const char* ControllerName() = 0;
 
   Heap* const heap_;
   const double min_growing_factor_;
   const double max_growing_factor_;
   const double conservative_growing_factor_;
-  const double target_mutator_utilization_;
-
-  FRIEND_TEST(HeapControllerTest, HeapGrowingFactor);
-  FRIEND_TEST(HeapControllerTest, MaxHeapGrowingFactor);
-  FRIEND_TEST(HeapControllerTest, MaxOldGenerationSize);
-  FRIEND_TEST(HeapControllerTest, OldGenerationAllocationLimit);
 };
 
 class V8_EXPORT_PRIVATE HeapController : public MemoryController {
@@ -58,13 +47,27 @@ class V8_EXPORT_PRIVATE HeapController : public MemoryController {
   // Sizes are in MB.
   static constexpr size_t kMinSize = 128 * Heap::kPointerMultiplier;
   static constexpr size_t kMaxSize = 1024 * Heap::kPointerMultiplier;
+  static constexpr double kTargetMutatorUtilization = 0.97;
 
-  explicit HeapController(Heap* heap)
-      : MemoryController(heap, 1.1, 4.0, 1.3, 0.97) {}
-  double MaxGrowingFactor(size_t curr_max_size);
+  explicit HeapController(Heap* heap) : MemoryController(heap, 1.1, 4.0, 1.3) {}
+
+  size_t CalculateAllocationLimit(size_t curr_size, size_t max_size,
+                                  double gc_speed, double mutator_speed,
+                                  size_t new_space_capacity,
+                                  Heap::HeapGrowingMode growing_mode);
 
  protected:
+  double GrowingFactor(double gc_speed, double mutator_speed,
+                       double max_factor);
+
+  double MaxGrowingFactor(size_t curr_max_size);
+
   const char* ControllerName() override { return "HeapController"; }
+
+  FRIEND_TEST(HeapControllerTest, HeapGrowingFactor);
+  FRIEND_TEST(HeapControllerTest, MaxHeapGrowingFactor);
+  FRIEND_TEST(HeapControllerTest, MaxOldGenerationSize);
+  FRIEND_TEST(HeapControllerTest, OldGenerationAllocationLimit);
 };
 
 }  // namespace internal

src/heap/heap.cc

@@ -1880,22 +1880,18 @@ bool Heap::PerformGarbageCollection(
         isolate()->isolate_data()->external_memory_ +
         kExternalAllocationSoftLimit;
 
-    double max_factor =
-        heap_controller()->MaxGrowingFactor(max_old_generation_size_);
-    size_t new_limit = heap_controller()->CalculateAllocationLimit(
-        old_gen_size, max_old_generation_size_, max_factor, gc_speed,
-        mutator_speed, new_space()->Capacity(), CurrentHeapGrowingMode());
-    old_generation_allocation_limit_ = new_limit;
+    old_generation_allocation_limit_ =
+        heap_controller()->CalculateAllocationLimit(
+            old_gen_size, max_old_generation_size_, gc_speed, mutator_speed,
+            new_space()->Capacity(), CurrentHeapGrowingMode());
 
     CheckIneffectiveMarkCompact(
         old_gen_size, tracer()->AverageMarkCompactMutatorUtilization());
   } else if (HasLowYoungGenerationAllocationRate() &&
              old_generation_size_configured_) {
-    double max_factor =
-        heap_controller()->MaxGrowingFactor(max_old_generation_size_);
     size_t new_limit = heap_controller()->CalculateAllocationLimit(
-        old_gen_size, max_old_generation_size_, max_factor, gc_speed,
-        mutator_speed, new_space()->Capacity(), CurrentHeapGrowingMode());
+        old_gen_size, max_old_generation_size_, gc_speed, mutator_speed,
+        new_space()->Capacity(), CurrentHeapGrowingMode());
     if (new_limit < old_generation_allocation_limit_) {
       old_generation_allocation_limit_ = new_limit;
     }

src/heap/heap.h

@@ -2061,6 +2061,7 @@ class Heap {
   friend class ConcurrentMarking;
   friend class GCCallbacksScope;
   friend class GCTracer;
+  friend class HeapController;
   friend class MemoryController;
   friend class HeapIterator;
   friend class IdleScavengeObserver;

test/unittests/heap/heap-controller-unittest.cc

@@ -75,32 +75,28 @@ TEST_F(HeapControllerTest, OldGenerationAllocationLimit) {
   double factor =
       heap_controller.GrowingFactor(gc_speed, mutator_speed, max_factor);
 
-  EXPECT_EQ(
-      static_cast<size_t>(old_gen_size * factor + new_space_capacity),
-      heap->heap_controller()->CalculateAllocationLimit(
-          old_gen_size, max_old_generation_size, max_factor, gc_speed,
-          mutator_speed, new_space_capacity, Heap::HeapGrowingMode::kDefault));
+  EXPECT_EQ(static_cast<size_t>(old_gen_size * factor + new_space_capacity),
+            heap->heap_controller()->CalculateAllocationLimit(
+                old_gen_size, max_old_generation_size, gc_speed, mutator_speed,
+                new_space_capacity, Heap::HeapGrowingMode::kDefault));
 
   factor = Min(factor, heap_controller.conservative_growing_factor_);
-  EXPECT_EQ(
-      static_cast<size_t>(old_gen_size * factor + new_space_capacity),
-      heap->heap_controller()->CalculateAllocationLimit(
-          old_gen_size, max_old_generation_size, max_factor, gc_speed,
-          mutator_speed, new_space_capacity, Heap::HeapGrowingMode::kSlow));
+  EXPECT_EQ(static_cast<size_t>(old_gen_size * factor + new_space_capacity),
+            heap->heap_controller()->CalculateAllocationLimit(
+                old_gen_size, max_old_generation_size, gc_speed, mutator_speed,
+                new_space_capacity, Heap::HeapGrowingMode::kSlow));
 
   factor = Min(factor, heap_controller.conservative_growing_factor_);
   EXPECT_EQ(static_cast<size_t>(old_gen_size * factor + new_space_capacity),
             heap->heap_controller()->CalculateAllocationLimit(
-                old_gen_size, max_old_generation_size, max_factor, gc_speed,
-                mutator_speed, new_space_capacity,
-                Heap::HeapGrowingMode::kConservative));
+                old_gen_size, max_old_generation_size, gc_speed, mutator_speed,
+                new_space_capacity, Heap::HeapGrowingMode::kConservative));
 
   factor = heap_controller.min_growing_factor_;
-  EXPECT_EQ(
-      static_cast<size_t>(old_gen_size * factor + new_space_capacity),
-      heap->heap_controller()->CalculateAllocationLimit(
-          old_gen_size, max_old_generation_size, max_factor, gc_speed,
-          mutator_speed, new_space_capacity, Heap::HeapGrowingMode::kMinimal));
+  EXPECT_EQ(static_cast<size_t>(old_gen_size * factor + new_space_capacity),
+            heap->heap_controller()->CalculateAllocationLimit(
+                old_gen_size, max_old_generation_size, gc_speed, mutator_speed,
+                new_space_capacity, Heap::HeapGrowingMode::kMinimal));
 }
 
 TEST_F(HeapControllerTest, MaxOldGenerationSize) {