Fix: Avoid extra old-space GCs (#1637)

src/heap.cc

@@ -317,6 +317,9 @@ GcType ObjectHeap::gc(bool try_hard) {
     // triple GC, which throws an exception.
     limit_ = max_heap_size_;
   }
+  // Will be cleared again immediately if the GC succeeded in freeing up enough
+  // to complete the primitive.
+  retrying_primitive_ = true;
   return type;
 }
 

src/heap.h

@@ -159,7 +159,10 @@ class ObjectHeap {
   bool has_max_heap_size() const { return max_heap_size_ != 0; }
   bool has_pending_limit() const { return limit_ != pending_limit_; }
 
-  void check_install_heap_limit() {
+  bool retrying_primitive() const { return retrying_primitive_; }
+
+  void leave_primitive() {
+    retrying_primitive_ = false;
     if (limit_ != pending_limit_) install_heap_limit();
   }
 
@@ -178,8 +181,7 @@ class ObjectHeap {
 
   void install_heap_limit();
 
-  bool in_gc_ = false;
-  bool gc_allowed_ = true;
+  bool retrying_primitive_ = false;
   AllocationResult last_allocation_result_ = ALLOCATION_SUCCESS;
 
   Process* owner_;
@@ -188,7 +190,7 @@ class ObjectHeap {
   static const word _UNLIMITED_EXPANSION = 0x7fffffff;
 
   // Number of bytes used before forcing a GC, including external memory.
-  // Set to zero to have no limit.
+  // Set to max_heap_size_ to have no limit.
   word limit_ = 0;
   // This limit will be installed at the end of the current primitive.
   word pending_limit_ = 0;

src/interpreter.cc

@@ -156,7 +156,7 @@ Object** Interpreter::push_error(Object** sp, Object* type, const char* message)
     sp = gc(sp, false, attempts, false);
     instance = process->object_heap()->allocate_instance(process->program()->exception_class_id());
   }
-  process->object_heap()->check_install_heap_limit();
+  process->object_heap()->leave_primitive();
 
   if (instance == null) {
     DROP(1);
@@ -174,7 +174,7 @@ Object** Interpreter::push_error(Object** sp, Object* type, const char* message)
     sp = gc(sp, true, attempts, false);
     buffer.allocate(STACK_ENCODING_BUFFER_SIZE);
   }
-  process->object_heap()->check_install_heap_limit();
+  process->object_heap()->leave_primitive();
 
   if (!buffer.has_content()) {
     DROP(2);
@@ -192,7 +192,7 @@ Object** Interpreter::push_error(Object** sp, Object* type, const char* message)
       sp = gc(sp, false, attempts, false);
       trace = process->allocate_byte_array(buffer.size());
     }
-    process->object_heap()->check_install_heap_limit();
+    process->object_heap()->leave_primitive();
 
     if (trace == null) {
       DROP(2);
@@ -261,7 +261,7 @@ Object** Interpreter::handle_stack_overflow(Object** sp, OverflowState* state, M
     sp = gc(sp, false, attempts, false);
     new_stack = process->object_heap()->allocate_stack(new_length);
   }
-  process->object_heap()->check_install_heap_limit();
+  process->object_heap()->leave_primitive();
 
   // Then check for out of memory.
   if (new_stack == null) {

src/interpreter_run.cc

@@ -281,7 +281,7 @@ Interpreter::Result Interpreter::run() {
   // after a failed allocation attempt, so we do not get the
   // pending heap limit installed. Do it here before starting
   // the interpretation.
-  process_->object_heap()->check_install_heap_limit();
+  process_->object_heap()->leave_primitive();
 
   // Interpretation state.
   Program* program = process_->program();
@@ -539,7 +539,7 @@ Interpreter::Result Interpreter::run() {
       sp = gc(sp, false, attempts, false);
       result = process_->object_heap()->allocate_instance(Smi::from(class_index));
     }
-    process_->object_heap()->check_install_heap_limit();
+    process_->object_heap()->leave_primitive();
 
     if (result == null) {
       sp = push_error(sp, program->allocation_failed(), "");
@@ -553,7 +553,7 @@ Interpreter::Result Interpreter::run() {
     PUSH(result);
     if (Flags::gc_a_lot) {
       sp = gc(sp, false, 1, false);
-      process_->object_heap()->check_install_heap_limit();
+      process_->object_heap()->leave_primitive();
     }
   OPCODE_END();
 
@@ -1083,7 +1083,7 @@ Interpreter::Result Interpreter::run() {
 
       // GC might have taken place in object heap but local "method" is from program heap.
       PUSH(result);
-      process_->object_heap()->check_install_heap_limit();
+      process_->object_heap()->leave_primitive();
       DISPATCH(PRIMITIVE_LENGTH);
 
     done:
@@ -1095,7 +1095,7 @@ Interpreter::Result Interpreter::run() {
       DROP(arity);
       ASSERT(!is_stack_empty());
       PUSH(result);
-      process_->object_heap()->check_install_heap_limit();
+      process_->object_heap()->leave_primitive();
       CHECK_PROPAGATED_TYPES_RETURN();
       DISPATCH(0);
     }

src/program_heap.cc

@@ -38,8 +38,7 @@ namespace toit {
 ProgramHeap::ProgramHeap(Program* program)
     : ProgramRawHeap()
     , program_(program)
-    , in_gc_(false)
-    , gc_allowed_(true)
+    , retrying_primitive_(false)
     , total_bytes_allocated_(0)
     , last_allocation_result_(ALLOCATION_SUCCESS) {
   blocks_.append(ProgramBlock::allocate_program_block());

src/program_heap.h

@@ -98,8 +98,7 @@ class ProgramHeap : public ProgramRawHeap {
   Program* const program_;
   HeapObject* _allocate_raw(int byte_size);
   virtual AllocationResult _expand();
-  bool in_gc_;
-  bool gc_allowed_;
+  bool retrying_primitive_;
   int64 total_bytes_allocated_;
   AllocationResult last_allocation_result_;
 

src/third_party/dartino/two_space_heap.cc

@@ -38,12 +38,10 @@ HeapObject* TwoSpaceHeap::allocate(uword size) {
 }
 
 HeapObject* TwoSpaceHeap::new_space_allocation_failure(uword size) {
-  if (!process_heap_->has_limit()) {
+  if (process_heap_->retrying_primitive()) {
     // When we are rerunning a primitive after a GC we don't want to
     // trigger a new GC unless we abolutely have to, so we allow allocation
-    // directly into old-space.  We recognize this situation by there not
-    // being an allocation limit (it is installed when the primitive
-    // completes).
+    // directly into old-space.
     uword result = old_space_.allocate(size);
     if (result != 0) {
       // The code that populates newly allocated objects assumes that they