Trigger old-space GC when scavange made the external heap of old-space larger than max old-space si…

runtime/vm/heap.cc

Index: runtime/vm/heap.cc
diff --git a/runtime/vm/heap.cc b/runtime/vm/heap.cc
index 835139a9cbcedeb64b23601bb8b651bb804dc549..d2aacdecb81540323d9d63e623736452b3005862 100644
--- a/runtime/vm/heap.cc
+++ b/runtime/vm/heap.cc
@@ -214,7 +214,9 @@ void Heap::CollectGarbage(Space space, ApiCallbacks api_callbacks) {
       new_space_->Scavenge(invoke_api_callbacks);
       RecordAfterGC();
       PrintStats();
-      if (new_space_->HadPromotionFailure()) {
+      if (new_space_->HadPromotionFailure() ||
+          (old_space_->ExternalInWords() >
+              (FLAG_new_gen_heap_size * MBInWords))) {

[koda, 2014/04/01 16:58:07]

Thanks for this fix. It does address the immediate problem with repeated
promotion overflowing old space. However, I now realize there is still an issue
with direct allocation in old space not being tested against the growth policy
and max capacity until something else triggers GC.

I will work on a CL that tries to unify growth control for external with rest of
old space. (It's slightly tricky since the current policy is based on
page-granularity.) But until that's done this patch gives some protection.

As for the code: You meant *old* gen heap size here, right? Also, please
consider encapsulating this check into a new method like
PageSpace::NeedExternalGC() {
return ExternalInWords() > max_capacity_in_words_;
}

[Anders Johnsen, 2014/04/02 11:16:28]

Yes, refactor error - should be old. Changed to your suggestion, though I use 

    used_in_words_ + ExternalInWords() > max_capacity_in_words_;

as internal + external should be < max.

         // Old collections should call the API callbacks.
         CollectGarbage(kOld, kInvokeApiCallbacks);
       }