Clean up the marking speed heuristics. This reduces the

src/incremental-marking.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 729 matching lines @@
     return;
   }
 
   allocated_ += allocated_bytes;
 
   if (allocated_ < kAllocatedThreshold) return;
 
   if (state_ == MARKING && no_marking_scope_depth_ > 0) return;
 
   intptr_t bytes_to_process = allocated_ * allocation_marking_factor_;
+  bytes_scanned_ += bytes_to_process;

[ulan, 2011/11/07 17:24:50]

Should the heuristics below be active in SWEEPING state? If yes this might be
imprecise.

[Erik Corry, 2011/11/08 10:29:34]

Added a reset of this variable when we start marking, and a change to not
increase the speed of marking until we start marking.

 
   double start = 0;
 
   if (FLAG_trace_incremental_marking || FLAG_trace_gc) {
     start = OS::TimeCurrentMillis();
   }
 
   if (state_ == SWEEPING) {
     if (heap_->old_pointer_space()->AdvanceSweeper(bytes_to_process) &&
         heap_->old_data_space()->AdvanceSweeper(bytes_to_process)) {
@@ skipping 41 matching lines @@
     if (marking_deque_.IsEmpty()) MarkingComplete();
   }
 
   allocated_ = 0;
 
   steps_count_++;
   steps_count_since_last_gc_++;
 
   bool speed_up = false;
 
+  if ((steps_count_ % kAllocationMarkingFactorSpeedupInterval) == 0) {
+    if (FLAG_trace_gc) {
+      PrintF("Speed up marking after %d steps\n",
+             static_cast<int>(kAllocationMarkingFactorSpeedupInterval));
+    }
+    speed_up = true;
+  }
+
   if (old_generation_space_available_at_start_of_incremental_ < 10 * MB ||
-      SpaceLeftInOldSpace() <
-          old_generation_space_available_at_start_of_incremental_ >> 1) {
-    // Half of the space that was available is gone while we were
+      SpaceLeftInOldSpace() * (allocation_marking_factor_ + 1) <
+          old_generation_space_available_at_start_of_incremental_) {
+    // 1/n of the space that was available is gone while we were

[ulan, 2011/11/07 17:24:50]

I think the condition says that 1/n is left and (n-1)/n is gone.

[Erik Corry, 2011/11/08 10:29:34]

Done.

     // incrementally marking.
+    if (FLAG_trace_gc) PrintF("Speed up marking because of low space left\n");
     speed_up = true;
-    old_generation_space_available_at_start_of_incremental_ =
-        SpaceLeftInOldSpace();
   }
 
   if (heap_->PromotedTotalSize() >
-      old_generation_space_used_at_start_of_incremental_ << 1) {
-    // Size of old space doubled while we were incrementally marking.
+      (allocation_marking_factor_ + 1) *
+          old_generation_space_used_at_start_of_incremental_) {
+    // Size of old space multiplied by n while we were incrementally marking.
     speed_up = true;
-    old_generation_space_used_at_start_of_incremental_ =
-        heap_->PromotedTotalSize();
+    if (FLAG_trace_gc) {
+      PrintF("Speed up marking because of heap size increase\n");
+    }
   }
 
-  if ((steps_count_ % kAllocationMarkingFactorSpeedupInterval) == 0 &&
-      allocation_marking_factor_ < kMaxAllocationMarkingFactor) {
+  // We try to scan at at least twice the speed that we are allocating.
+  if ((bytes_scanned_ >> 1)
+          + heap_->MaxSemiSpaceSize()  // A single scavenge cannot trigger this.
+          + allocation_marking_factor_ * MB <  // Delay before upping again.

[ulan, 2011/11/07 17:24:50]

Possible overflow in allocation_marking_factor_ * MB?

bytes_scanned / 2 saves parentheses, and should be as fast as (bytes_scanned >>
1).

Variables with descriptive names would be better than comments:
if (bytes_scanned_ / 2 < promoted_since_start_of_incremental - delay) ...

[Erik Corry, 2011/11/08 10:29:34]

Reduced max allocation marking factor to 1000 to avoid this.
Done.
Done.

+      heap_->PromotedTotalSize()
+          - old_generation_space_used_at_start_of_incremental_) {
+    if (FLAG_trace_gc) {
+      PrintF("Speed up marking because marker was not keeping up\n");
+    }
     speed_up = true;
   }
 
   if (speed_up) {
     allocation_marking_factor_ += kAllocationMarkingFactorSpeedup;
     allocation_marking_factor_ =
-        static_cast<int>(allocation_marking_factor_ * 1.3);
+        Min(kMaxAllocationMarkingFactor,
+            static_cast<intptr_t>(allocation_marking_factor_ * 1.3));
     if (FLAG_trace_gc) {
       PrintF("Marking speed increased to %d\n", allocation_marking_factor_);
     }
   }
 
   if (FLAG_trace_incremental_marking || FLAG_trace_gc) {
     double end = OS::TimeCurrentMillis();
     double delta = (end - start);
     longest_step_ = Max(longest_step_, delta);
     steps_took_ += delta;
     steps_took_since_last_gc_ += delta;
   }
 }
 
 
 void IncrementalMarking::ResetStepCounters() {
   steps_count_ = 0;
   steps_took_ = 0;
   longest_step_ = 0.0;
   old_generation_space_available_at_start_of_incremental_ =
       SpaceLeftInOldSpace();
   old_generation_space_used_at_start_of_incremental_ =
       heap_->PromotedTotalSize();
   steps_count_since_last_gc_ = 0;
   steps_took_since_last_gc_ = 0;
   bytes_rescanned_ = 0;
   allocation_marking_factor_ = kInitialAllocationMarkingFactor;
+  bytes_scanned_ = 0;
 }
 
 
 int64_t IncrementalMarking::SpaceLeftInOldSpace() {
   return heap_->MaxOldGenerationSize() - heap_->PromotedSpaceSize();
 }
 
 } }  // namespace v8::internal