Trigger a cross-process compaction when malloc fails (#717)

* Trigger a cross-process compaction when malloc fails

* Heap range check in release mode

* feedback

* Disable flaky RPC test for now

* Try to fix Windows

* Compare result against rounded

Co-authored-by: Kasper Lund <[email protected]>

src/heap.cc

@@ -309,7 +309,6 @@ ObjectHeap::~ObjectHeap() {
   ASSERT(_object_notifiers.is_empty());
 
 #ifdef LEGACY_GC
-  // TODO: ObjectHeap deletion in new GC.
   // Deleting a heap is like a scavenge where nothing survives.
   ScavengeScope scope(VM::current()->heap_memory(), this);
   _blocks.free_blocks(this);
@@ -482,8 +481,8 @@ void ObjectHeap::iterate_roots(RootCallback* callback) {
 
 #ifndef LEGACY_GC
 
-int ObjectHeap::gc() {
-  _two_space_heap.collect_new_space();
+int ObjectHeap::gc(bool try_hard) {
+  _two_space_heap.collect_new_space(try_hard);
   _gc_count++;
   _pending_limit = _calculate_limit();  // GC limit to install after next GC.
   _limit = _max_heap_size;  // Only the hard limit for the rest of this primitive.
@@ -499,7 +498,7 @@ class HasForwardingAddress : public LivenessOracle {
   }
 };
 
-int ObjectHeap::gc() {
+int ObjectHeap::gc(bool try_hard) {
   if (program() == null) FATAL("cannot gc external process");
 
   word blocks_before = _blocks.length();

src/heap.h

@@ -98,13 +98,15 @@ class ObjectHeap {
   ObjectHeap(Program* program, Process* owner, InitialMemory* initial_memory);
   ~ObjectHeap();
 
-  // TODO: In the new heap there is no max allocation size.
+  // TODO: In the new heap there need not be a max allocation size.
   static int max_allocation_size() { return TOIT_PAGE_SIZE - 96; }
 
   inline void do_objects(const std::function<void (HeapObject*)>& func) {
     _two_space_heap.do_objects(func);
   }
 
+  inline bool cross_process_gc_needed() const { return _two_space_heap.cross_process_gc_needed(); }
+
 #endif
 
   // Shared allocation operations.
@@ -157,7 +159,13 @@ class ObjectHeap {
 #endif
 
   bool system_refused_memory() const {
-    return _last_allocation_result == ALLOCATION_OUT_OF_MEMORY;
+    return
+#ifdef LEGACY_GC
+        _last_allocation_result == ALLOCATION_OUT_OF_MEMORY;
+#else
+        _last_allocation_result == ALLOCATION_OUT_OF_MEMORY ||
+        _two_space_heap.cross_process_gc_needed();
+#endif
   }
 
   enum AllocationResult {
@@ -198,7 +206,7 @@ class ObjectHeap {
   void set_task(Task* task);
 
   // Garbage collection operation for runtime objects.
-  int gc();
+  int gc(bool try_hard);
 
   bool add_finalizer(HeapObject* key, Object* lambda);
   bool has_finalizer(HeapObject* key, Object* lambda);

src/os_esp32.cc

@@ -385,8 +385,8 @@ OS::HeapMemoryRange OS::get_heap_memory_range() {
   // Internal SRAM 0 192k 3ffe_0000 - 4000_0000    4007_0000 - 400a_0000
   // Internal SRAM 1 128k                          400a_0000 - 400c_0000
   HeapMemoryRange range;
-  range.address = reinterpret_cast<void*>(0x3ffae000);
-  range.size = 392 * KB;
+  range.address = reinterpret_cast<void*>(0x3ffc0000);
+  range.size = 256 * KB;
   return range;
 }
 

src/os_win.cc

@@ -313,8 +313,8 @@ bool OS::use_virtual_memory(void* addr, uword sz) {
   uword end = address + sz;
   uword rounded = Utils::round_down(address, 4096);
   uword size = Utils::round_up(end - rounded, 4096);
-  void* result = VirtualAlloc(reinterpret_cast<void*>(address), size, MEM_COMMIT, PAGE_READWRITE);
-  if (result != reinterpret_cast<void*>(address)) FATAL("use_virtual_memory");
+  void* result = VirtualAlloc(reinterpret_cast<void*>(rounded), size, MEM_COMMIT, PAGE_READWRITE);
+  if (result != reinterpret_cast<void*>(rounded)) FATAL("use_virtual_memory");
   return true;
 }
 

src/process.h

@@ -72,9 +72,9 @@ class Process : public ProcessListFromProcessGroup::Element,
   void mark_as_priviliged() { _is_privileged = true; }
 
   // Garbage collection operation for runtime objects.
-  int gc() {
+  int gc(bool try_hard) {
     if (program() == null) return 0;
-    int result = object_heap()->gc();
+    int result = object_heap()->gc(try_hard);
     _memory_usage = object_heap()->usage("object heap after gc");
     return result;
   }

src/scheduler.cc

@@ -459,7 +459,7 @@ void Scheduler::gc(Process* process, bool malloc_failed, bool try_hard) {
     }
 
     for (Process* target : targets) {
-      target->gc();
+      target->gc(try_hard);
       gcs++;
     }
 
@@ -473,7 +473,7 @@ void Scheduler::gc(Process* process, bool malloc_failed, bool try_hard) {
     }
   }
 
-  process->gc();
+  process->gc(try_hard);
 
   if (doing_cross_process_gc) {
     Locker locker(_mutex);

src/third_party/dartino/object_memory.cc

@@ -19,27 +19,23 @@ namespace toit {
 
 #ifndef LEGACY_GC
 
-Chunk::Chunk(Space* owner, uword start, uword size, bool external)
+Chunk::Chunk(Space* owner, uword start, uword size)
       : owner_(owner),
         start_(start),
         end_(start + size),
-        external_(external),
         scavenge_pointer_(start_) {
   if (!GcMetadata::in_metadata_range(start)) {
     FATAL("Not in metadata range: %p\n", (void*)start);
   }
 }
 
 Chunk::~Chunk() {
-  // If the memory for this chunk is external we leave it alone
-  // and let the embedder deallocate it.
-  if (is_external()) return;
   GcMetadata::mark_pages_for_chunk(this, UNKNOWN_SPACE_PAGE);
   OS::free_pages(reinterpret_cast<void*>(start_), size());
 }
 
 Space::~Space() {
-  // TODO(erik): Call finalizers.
+  // ObjectHeap destructor already called all finalizers.
   free_all_chunks();
 }
 
@@ -251,9 +247,10 @@ Chunk* ObjectMemory::allocate_chunk(Space* owner, uword size) {
   uword lowest = GcMetadata::lowest_old_space_address();
   USE(lowest);
   if (memory == null) return null;
-  ASSERT(reinterpret_cast<uword>(memory) >= lowest);
-  ASSERT(reinterpret_cast<uword>(memory) - lowest + size <=
-         GcMetadata::heap_extent());
+  if (reinterpret_cast<uword>(memory) < lowest ||
+      reinterpret_cast<uword>(memory) - lowest + size > GcMetadata::heap_extent()) {
+    FATAL("Toit heap outside expected range");
+  }
 
   uword base = reinterpret_cast<uword>(memory);
   Chunk* chunk = _new Chunk(owner, base, size);
@@ -291,8 +288,7 @@ void Chunk::initialize_metadata() const {
 
 void ObjectMemory::free_chunk(Chunk* chunk) {
 #ifdef DEBUG
-  // Do not touch external memory. It might be read-only.
-  if (!chunk->is_external()) chunk->scramble();
+  chunk->scramble();
 #endif
   allocated_ -= chunk->size();
   delete chunk;

src/third_party/dartino/object_memory.h

@@ -74,9 +74,6 @@ class Chunk : public ChunkList::Element {
   // Returns the size of this chunk in bytes.
   uword size() const { return end_ - start_; }
 
-  // Is the chunk externally allocated by the embedder.
-  bool is_external() const { return external_; }
-
   // Test for inclusion.
   bool includes(uword address) {
     return (address >= start_) && (address < end_);
@@ -99,17 +96,16 @@ class Chunk : public ChunkList::Element {
   void find(uword word, const char* name);
 #endif
 
-  ~Chunk();
-
  private:
+  ~Chunk();  // Use ObjectMemory::free_chunk().
+
   Space* owner_;
   const uword start_;
   const uword end_;
-  const bool external_;
   uword scavenge_pointer_;
   uword compaction_top_;
 
-  Chunk(Space* owner, uword start, uword size, bool external = false);
+  Chunk(Space* owner, uword start, uword size);
 
   friend class ObjectMemory;
 };

src/third_party/dartino/object_memory_mark_sweep.cc

@@ -105,6 +105,7 @@ Chunk* OldSpace::allocate_and_use_chunk(uword size) {
 }
 
 uword OldSpace::allocate_in_new_chunk(uword size) {
+  if (allocation_budget_ < 0) return 0;
   ASSERT(top_ == 0);  // Space is flushed.
   // Allocate new chunk.  After a certain heap size we start allocating
   // multi-page chunks to improve fragmentation.
@@ -126,10 +127,13 @@ uword OldSpace::allocate_in_new_chunk(uword size) {
     }
     if (chunk != null) {
       return allocate(size);
+    } else {
+      heap_->report_malloc_failed();
     }
   }
 
-  allocation_budget_ = -1;  // Trigger GC.
+  // Speed up later attempts during this scavenge to promote objects.
+  allocation_budget_ = -1;
   return 0;
 }
 
@@ -402,9 +406,12 @@ void OldSpace::end_scavenge() {
 void OldSpace::clear_free_list() { free_list_.clear(); }
 
 void OldSpace::mark_chunk_ends_free() {
-  chunk_list_.delete_wherever([&](Chunk* chunk) -> bool {
+  chunk_list_.remove_wherever([&](Chunk* chunk) -> bool {
     uword top = chunk->compaction_top();
-    if (top == chunk->start()) return true;  // Remove empty chunks from list.
+    if (top == chunk->start()) {
+      ObjectMemory::free_chunk(chunk);
+      return true;  // Remove empty chunks from list.
+    }
     uword end = chunk->usable_end();
     if (top != end) free_list_.add_region(top, end - top);
     top = Utils::round_up(top, GcMetadata::CARD_SIZE);

src/third_party/dartino/two_space_heap.cc

@@ -32,6 +32,10 @@ uword TwoSpaceHeap::max_expansion() {
   return old_space()->used() - limit;
 }
 
+Process* TwoSpaceHeap::process() {
+  return process_heap_->owner();
+}
+
 TwoSpaceHeap::~TwoSpaceHeap() {
   // TODO(erik): Call all finalizers.
 }
@@ -113,15 +117,19 @@ void SemiSpace::start_scavenge() {
 
 #ifndef LEGACY_GC
 
-void TwoSpaceHeap::collect_new_space() {
+void TwoSpaceHeap::collect_new_space(bool try_hard) {
   SemiSpace* from = new_space();
 
   uint64 start = OS::get_monotonic_time();
 
+  // Might get set during scavenge if we fail to promote to a full old-space
+  // that can't be expanded.
+  malloc_failed_ = false;
+
   total_bytes_allocated_ += from->used();
 
   if (has_empty_new_space()) {
-    collect_old_space_if_needed(false);
+    collect_old_space_if_needed(try_hard, try_hard);
     if (Flags::tracegc) {
       uint64 end = OS::get_monotonic_time();
       printf("Old-space-only GC: %dus\n", static_cast<int>(end - start));
@@ -184,14 +192,22 @@ void TwoSpaceHeap::collect_new_space() {
     int f = from_used;
     int t = to_used;
     int old = old_space()->used();
-    printf("%p Scavenge: %d%c->%d%c (old-gen %d%c) %dus\n",
+
+    uword overhead = old_space()->size() - old;
+
+    char buffer[30];
+    buffer[sizeof(buffer) - 1] = '\0';
+    snprintf(buffer, sizeof(buffer) - 1, " +%dk overhead", static_cast<int>(overhead) >> 10);
+
+    printf("%p Scavenge: %d%c->%d%c (old-gen %d%c%s) %dus\n",
         process_heap_->owner(),
         (f >> 10) ? (f >> 10) : f,
         (f >> 10) ? 'k' : 'b',
         (t >> 10) ? (t >> 10) : t,
         (t >> 10) ? 'k' : 'b',
         (old >> 10) ? (old >> 10) : old,
         (old >> 10) ? 'k' : 'b',
+        (overhead >= TOIT_PAGE_SIZE) ? buffer : "",
         static_cast<int>(end - start));
   }
 
@@ -206,7 +222,7 @@ void TwoSpaceHeap::collect_new_space() {
     old_space()->report_new_space_progress(progress);
   }
 
-  collect_old_space_if_needed(trigger_old_space_gc);
+  collect_old_space_if_needed(try_hard, trigger_old_space_gc);
 }
 
 uword TwoSpaceHeap::total_bytes_allocated() {
@@ -215,18 +231,18 @@ uword TwoSpaceHeap::total_bytes_allocated() {
   return result;
 }
 
-void TwoSpaceHeap::collect_old_space_if_needed(bool force) {
+void TwoSpaceHeap::collect_old_space_if_needed(bool force_compact, bool force) {
 #ifdef DEBUG
   if (Flags::validate_heap) {
     validate();
     old_space()->validate_before_mark_sweep(OLD_SPACE_PAGE, false);
     new_space()->validate_before_mark_sweep(NEW_SPACE_PAGE, true);
   }
 #endif
-  if (force || old_space()->needs_garbage_collection()) {
+  if (force || force_compact || old_space()->needs_garbage_collection()) {
     ASSERT(old_space()->is_flushed());
     ASSERT(new_space()->is_flushed());
-    collect_old_space();
+    collect_old_space(force_compact);
   }
 }
 
@@ -237,24 +253,31 @@ void TwoSpaceHeap::validate() {
 }
 #endif
 
-void TwoSpaceHeap::collect_old_space() {
+void TwoSpaceHeap::collect_old_space(bool force_compact) {
 
   uint64 start = OS::get_monotonic_time();
-  uword old_size = old_space()->used();
+  uword old_used = old_space()->used();
 
-  bool compacted = perform_garbage_collection();
+  bool compacted = perform_garbage_collection(force_compact);
 
   if (Flags::tracegc) {
     uint64 end = OS::get_monotonic_time();
-    int f = old_size;
+    int f = old_used;
     int t = old_space()->used();
-    printf("%p Mark-sweep%s: %d%c->%d%c, %dus\n",
+    uword overhead = old_space()->size() - t;
+
+    char buffer[30];
+    buffer[sizeof(buffer) - 1] = '\0';
+    snprintf(buffer, sizeof(buffer) - 1, " +%dk overhead", static_cast<int>(overhead) >> 10);
+
+    printf("%p Mark-sweep%s: %d%c->%d%c%s %dus\n",
         process_heap_->owner(),
         compacted ? "-compact" : "",
         (f >> 10) ? (f >> 10) : f,
         (f >> 10) ? 'k' : 'b',
         (t >> 10) ? (t >> 10) : t,
         (t >> 10) ? 'k' : 'b',
+        (overhead >= TOIT_PAGE_SIZE) ? buffer : "",
         static_cast<int>(end - start));
   }
 
@@ -271,7 +294,7 @@ void TwoSpaceHeap::collect_old_space() {
   old_space()->adjust_allocation_budget(0);
 }
 
-bool TwoSpaceHeap::perform_garbage_collection() {
+bool TwoSpaceHeap::perform_garbage_collection(bool force_compact) {
   // Mark all reachable objects.  We mark all live objects in new-space too, to
   // detect liveness paths that go through new-space, but we just clear the
   // mark bits afterwards.  Dead objects in new-space are only cleared in a
@@ -292,7 +315,7 @@ bool TwoSpaceHeap::perform_garbage_collection() {
 
   stack.process(&marking_visitor, old_space(), semi_space);
 
-  bool compact = !old_space()->compacting();
+  bool compact = force_compact || !old_space()->compacting();
 
   if (!compact) {
     // If the last GC was compacting we don't have fragmentation, so it