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

runtime/vm/heap.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/heap.h"
 
 #include "platform/assert.h"
 #include "platform/utils.h"
 #include "vm/flags.h"
 #include "vm/isolate.h"
@@ skipping 196 matching lines @@
   Isolate* isolate = Isolate::Current();
   bool invoke_api_callbacks = (api_callbacks == kInvokeApiCallbacks);
   switch (space) {
     case kNew: {
       VMTagScope tagScope(isolate, VMTag::kGCNewSpaceTagId);
       RecordBeforeGC(kNew, kNewSpace);
       UpdateClassHeapStatsBeforeGC(kNew);
       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);
       }
       break;
     }
     case kOld:
     case kCode: {
       VMTagScope tagScope(isolate, VMTag::kGCOldSpaceTagId);
       bool promotion_failure = new_space_->HadPromotionFailure();
       RecordBeforeGC(kOld, promotion_failure ? kPromotionFailure : kOldSpace);
@@ skipping 351 matching lines @@
     heap->DisableGrowthControl();
 }
 
 
 NoHeapGrowthControlScope::~NoHeapGrowthControlScope() {
     Heap* heap = reinterpret_cast<Isolate*>(isolate())->heap();
     heap->SetGrowthControlState(current_growth_controller_state_);
 }
 
 }  // namespace dart