Some fixes to GC heuristics.

runtime/vm/pages.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/pages.h"
 
 #include "platform/address_sanitizer.h"
 #include "platform/assert.h"
 #include "vm/compiler_stats.h"
 #include "vm/gc_marker.h"
@@ skipping 1193 matching lines @@
 void PageSpaceController::EvaluateGarbageCollection(SpaceUsage before,
                                                     SpaceUsage after,
                                                     int64_t start,
                                                     int64_t end) {
   ASSERT(end >= start);
   history_.AddGarbageCollectionTime(start, end);
   const int gc_time_fraction = history_.GarbageCollectionTimeFraction();
   heap_->RecordData(PageSpace::kGCTimeFraction, gc_time_fraction);
 
   // Assume garbage increases linearly with allocation:
   // G = kA, and estimate k from the previous cycle.

[kasperl, 2017/04/27 06:58:16]

Nit: Can we rename 'k' to garbage_ratio, ratio, or 'r'? To me, k is a bit
overloaded (kilo?) and is usually used for integer constants (like n).

   const intptr_t allocated_since_previous_gc =
       before.used_in_words - last_usage_.used_in_words;
   if (allocated_since_previous_gc > 0) {
     const intptr_t garbage = before.used_in_words - after.used_in_words;
     ASSERT(garbage >= 0);
-    const double k = garbage / static_cast<double>(allocated_since_previous_gc);
+    // It makes no sense to expect that each kb allocated will cause more than
+    // one kb of garbage, so we clamp k at 1.0.
+    const double k = Utils::Minimum(
+        1.0, garbage / static_cast<double>(allocated_since_previous_gc));
+
     const int garbage_ratio = static_cast<int>(k * 100);

[kasperl, 2017/04/27 06:58:16]

Maybe we wouldn't need this named constant if 'k' had a better name?

     heap_->RecordData(PageSpace::kGarbageRatio, garbage_ratio);
 
     // Define GC to be 'worthwhile' iff at least fraction t of heap is garbage.
     double t = 1.0 - desired_utilization_;
     // If we spend too much time in GC, strive for even more free space.
     if (gc_time_fraction > garbage_collection_time_ratio_) {
       t += (gc_time_fraction - garbage_collection_time_ratio_) / 100.0;
     }
 
     const intptr_t grow_ratio =
         (static_cast<intptr_t>(after.capacity_in_words / desired_utilization_) -
          after.capacity_in_words) /
         PageSpace::kPageSizeInWords;
     if (garbage_ratio == 0) {

[kasperl, 2017/04/27 06:58:16]

Express through 'k' instead?

       // No garbage in the previous cycle so it would be hard to compute a
       // grow_heap_ size based on estimated garbage so we use growth ratio
       // heuristics instead.
       grow_heap_ =
           Utils::Maximum(static_cast<intptr_t>(heap_growth_max_), grow_ratio);
     } else {
       // Find minimum 'grow_heap_' such that after increasing capacity by
       // 'grow_heap_' pages and filling them, we expect a GC to be worthwhile.
       intptr_t max = heap_growth_max_;
       intptr_t min = 0;
       intptr_t local_grow_heap = 0;
       while (min < max) {
         local_grow_heap = (max + min) / 2;
         const intptr_t limit = after.capacity_in_words +
-                               (grow_heap_ * PageSpace::kPageSizeInWords);
+                               (local_grow_heap * PageSpace::kPageSizeInWords);
         const intptr_t allocated_before_next_gc = limit - after.used_in_words;
         const double estimated_garbage = k * allocated_before_next_gc;
         if (t <= estimated_garbage / limit) {
           max = local_grow_heap - 1;
         } else {
           min = local_grow_heap + 1;
         }
       }
       local_grow_heap = (max + min) / 2;
       grow_heap_ = local_grow_heap;
@@ skipping 42 matching lines @@
     return 0;
   } else {
     ASSERT(total_time >= gc_time);
     int result = static_cast<int>(
         (static_cast<double>(gc_time) / static_cast<double>(total_time)) * 100);
     return result;
   }
 }
 
 }  // namespace dart