Fix a bug which was cryptically preventing Gen-1 size from increasing.

gc.c

@@ -411,6 +411,15 @@ void clear_gc_stats(void)
   gc_stats.max_ns      = 0;
 }
 
+static void gc_poison_region(void *start, size_t size, value_t tag)
+{
+  size_t count = size / GC_ALIGNMENT;
+  object_t *obj = (object_t*)start;
+
+  while (count--)
+    *obj++ = (object_t){ .tag = tag, .forward = tag };
+}
+
 /****************************** Gen-0 Collector *****************************/
 
 /* These private variables are exported ONLY for use by is_gen0_object(). */
@@ -441,6 +450,11 @@ static void gc_gen0_init(size_t gen0_size)
 }
 
 static void collect_gen0_garbage(void)
+{
+  collect_gen1_garbage(0);
+  return;
+
+  if (gc_enabled)
   {
 #ifndef NO_STATS
     size_t initial_free_space;
@@ -448,17 +462,14 @@ static void collect_gen0_garbage(void)
     struct timespec start_time;
     clock_gettime(TIMING_CLOCK, &start_time);
 #endif
-
     initial_free_space = gc_gen0_free_space() + gc_gen1_free_space();
 #endif
 
-  //debug(("Performing Gen-0 garbage collection pass...\n"));
+    debug(("Performing Gen-0 garbage collection pass...\n"));
 
     assert(!gc_in_gen0_collection);
     assert(!gc_in_gen1_collection);
 
-  gc_in_gen0_collection = true;
-
     /* If we trigger a Gen-1 collection at any point then we are done. */
     /* Full collection will pull in any current Gen-0 objects. */
     if (setjmp(gc_gen0_end_ctx) == 0)
@@ -469,6 +480,8 @@ static void collect_gen0_garbage(void)
       int group;
       int bit;
 
+      gc_in_gen0_collection = true;
+
       /* 1. Transfer Gen-0 roots (ignore Gen-1). */
       transfer_roots();
 
@@ -483,6 +496,8 @@ static void collect_gen0_garbage(void)
             const int block = group * 32 + bit;
             char *block_obj = gc_gen1_block_starts[block];
 
+            assert(block_obj);
+
             /* For each object in block: transfer children */
             do {
               block_obj += gc_align(transfer_children((object_t*)block_obj));
@@ -513,6 +528,11 @@ static void collect_gen0_garbage(void)
 
       update_weak_box_list();
 
+#ifndef NDEBUG
+      /* Clear old range, to make it easier to detect bugs. */
+      gc_poison_region(gc_gen0_range, gc_gen0_size, GC_GEN0_POISON);
+#endif
+
       /* 4. Reset Gen-0 range to 'empty' state. */
       gc_gen1_clear_dirty_bits();
       gc_gen0_free_ptr = gc_gen0_range;
@@ -540,18 +560,10 @@ static void collect_gen0_garbage(void)
       ++gc_stats.gen0_passes;
 #endif
     }
-  else
-  {
-    //debug(("Gen-0 pass was interrupted by Gen-1 (full) collection.\n"));
-  }
-
-#ifndef NDEBUG
-  /* Clear old range, to make it easier to detect bugs. */
-  memset(gc_gen0_range, 0, gc_gen0_size);
-#endif
 
     gc_in_gen0_collection = false;
   }
+}
 
 void *gc_alloc(size_t nbytes)
 {
@@ -568,7 +580,12 @@ void *gc_alloc(size_t nbytes)
   else
   {
     if (nbytes > gc_gen0_free_space())
+    {
+      if (gc_enabled)
         collect_gen0_garbage();
+      else
+        return gc_alloc_gen1(nbytes);
+    }
 
     assert(nbytes <= gc_gen0_free_space());
 
@@ -593,6 +610,26 @@ static int gen1_gc_range_of(void *object)
   return -1;
 }
 
+static inline bool gc_object_has_moved(value_t v)
+{
+  return is_broken_heart(v) &&
+         (gen1_gc_range_of(_get_object(_get_object(v)->forward)) == gc_gen1_current_range);
+}
+
+/* Only useful AFTER all reachable objects have been processed. */
+static inline bool gc_object_left_behind(value_t v)
+{
+  /* Must provide a reference to the original location, not the new one (if moved). */
+  assert(!is_object(v) ||
+         is_gen0_object(v) ||
+         (gen1_gc_range_of(_get_object(v)) != gc_gen1_current_range));
+
+  return is_object(v) &&
+         (gc_in_gen1_collection || is_gen0_object(v)) &&
+         (!is_broken_heart(v) || (gen1_gc_range_of(_get_object(_get_object(v)->forward)) !=
+                                  gc_gen1_current_range));
+}
+
 static void gc_gen1_init(size_t min_size, size_t max_size)
 {
   release_assert(min_size <= ((max_size+1)/2));
@@ -627,9 +664,14 @@ static void gc_gen1_clear_dirty_bits(void)
 
 static void *gc_alloc_gen1(size_t nbytes)
 {
-  nbytes = gc_align(nbytes);
+  size_t min_free;
 
-  if ((nbytes + gc_gen0_size) > gc_gen1_free_space())
+  min_free = nbytes = gc_align(nbytes);
+
+  if (!gc_in_gen1_collection)
+    min_free += gc_gen0_size;
+
+  if (gc_gen1_free_space() < min_free)
     collect_gen1_garbage(nbytes);
 
   assert(nbytes <= gc_gen1_free_space());
@@ -666,6 +708,8 @@ static void transfer_object(value_t *value)
         transfer_object(&obj->forward);
       }
 
+      assert(gen1_gc_range_of(_get_object(obj->forward)) == gc_gen1_current_range);
+
       /* Object has already been moved; just update the reference */
       *value = obj->forward;
       return;
@@ -801,8 +845,8 @@ static void transfer_roots(void)
   transfer_object(&gc_will_list);
 
   /* The values associated with active wills are also roots */
-  for (value_t *will = &gc_will_active_list; !is_nil(*will); will = &_get_will(*will)->next)
+  for (value_t will = gc_will_active_list; !is_nil(will); will = _get_will(will)->next)
-    transfer_object(&get_will(*will)->value);
+    transfer_object(&get_will(will)->value);
 
   /* Ensure active list itself is transferred */
   transfer_object(&gc_will_active_list);
@@ -816,7 +860,7 @@ static void process_weak_boxes(void)
   {
     weak_box_t *box;
 
-    if (is_broken_heart(wb))
+    if (gc_object_has_moved(wb))
     {
       /* Box has been moved; get a pointer to the new location, but don't update list yet. */
       value_t fw = _get_object(wb)->forward;
@@ -830,12 +874,7 @@ static void process_weak_boxes(void)
       box = _get_weak_box(wb);
     }
 
-    if (is_broken_heart(box->value))
+    if (gc_object_left_behind(box->value))
-    {
-      /* The value in the box is reachable; update w/ new location. */
-      box->value = _get_object(box->value)->forward;
-    }
-    else if (is_object(box->value) && (gc_in_gen1_collection || is_gen0_object(box->value)))
     {
       /* The value in the box is an unreachable object; change to #f. */
 
@@ -853,6 +892,11 @@ static void process_weak_boxes(void)
 
       box->value = FALSE_VALUE;
     }
+    else if (gc_object_has_moved(box->value))
+    {
+      /* The value in the box is reachable; update w/ new location. */
+      box->value = _get_object(box->value)->forward;
+    }
 
     /* Move on to this box's 'next' pointer */
     wb = box->next;
@@ -874,18 +918,8 @@ static void process_wills(void)
   {
     will_t *w = get_will(*will);
 
-    if (is_broken_heart(w->value))
+    if (gc_object_left_behind(w->value))
     {
-      /* The value associated with the will is still reachable; update w/ new location. */
-      w->value = _get_object(w->value)->forward;
-
-      /* Move on to this will's 'next' pointer */
-      will = &w->next;
-    }
-    else if (gc_in_gen1_collection || is_gen0_object(w->value))
-    {
-      assert(is_object(w->value));
-
       /*
        * The will is associated with an unreachable object; activate it.
        */
@@ -900,6 +934,17 @@ static void process_wills(void)
       w->next = gc_will_active_list;
       gc_will_active_list = object_value(w);
     }
+    else
+    {
+      /* The value associated with the will is still reachable; update w/ new location. */
+      if (gc_object_has_moved(w->value))
+      {
+        w->value = _get_object(w->value)->forward;
+      }
+
+      /* Move on to this will's 'next' pointer */
+      will = &w->next;
+    }
   }
 }
 
@@ -909,23 +954,24 @@ static void update_weak_box_list(void)
 
   while (!is_nil(*wb))
   {
-    if (is_broken_heart(*wb))
+    if (gc_object_left_behind(*wb))
-    {
-      assert(gen1_gc_range_of(_get_object(_get_object(*wb)->forward)) == gc_gen1_current_range);
-
-      /* The box itself is reachable; need to update 'next' pointer to new location */
-      *wb = _get_object(*wb)->forward;
-
-      /* Move on to next box's 'next' pointer */
-      assert(is_weak_box(*wb));
-      wb = &_get_weak_box(*wb)->next;
-    }
-    else
     {
       /* Box is no longer reachable; remove it from the list by updating 'next' pointer. */
       assert(is_weak_box(*wb));
       *wb = _get_weak_box(*wb)->next;
     }
+    else
+    {
+      /* The box itself is reachable; need to update 'next' pointer to new location */
+      if (gc_object_has_moved(*wb))
+      {
+        *wb = _get_object(*wb)->forward;
+      }
+
+      /* Move on to next box's 'next' pointer */
+      assert(is_weak_box(*wb));
+      wb = &_get_weak_box(*wb)->next;
+    }
   }
 }
 
@@ -945,10 +991,10 @@ static void update_soft_limit(size_t min_free)
       new_limit = deflate_limit;
   }
 
-  if (new_limit > gc_gen1_max_size)
+  if (new_limit < gc_gen1_min_size)
-    new_limit = gc_gen1_max_size;
-  else if (new_limit < gc_gen1_min_size)
     new_limit = gc_gen1_min_size;
+  else if (new_limit > gc_gen1_max_size)
+    new_limit = gc_gen1_max_size;
 
   gc_gen1_soft_limit = new_limit;
 
@@ -989,7 +1035,9 @@ static void collect_gen1_garbage(size_t min_free)
 {
   bool collected_garbage = false;
 
-  gc_in_gen1_collection = true;
+  /* If Gen-1 free space falls below used portion of Gen-0, chaos may ensue. */
+  assert(gc_gen1_free_space() >= (gc_gen0_free_ptr - gc_gen0_range));
+  min_free += gc_gen0_size;
 
   if (gc_enabled)
   {
@@ -1003,9 +1051,11 @@ static void collect_gen1_garbage(size_t min_free)
 #endif
 #endif
 
-    //debug(("Performing Gen-1 garbage collection pass...\n"));
+    debug(("Performing Gen-1 garbage collection pass...\n"));
 
     gc_enabled = false;
+    gc_in_gen1_collection = true;
+
     swap_gen1_gc_ranges();
 
     /* Record the start of each Gen-1 block as we go. */
@@ -1042,7 +1092,8 @@ static void collect_gen1_garbage(size_t min_free)
 
 #ifndef NDEBUG
     /* Clear old range, to make it easier to detect bugs. */
-    memset(gc_gen1_ranges[gc_gen1_other_range()], 0, gc_gen1_soft_limit);
+    gc_poison_region(gc_gen1_ranges[gc_gen1_other_range()], gc_gen1_soft_limit, GC_GEN1_POISON);
+    gc_poison_region(gc_gen0_range, gc_gen0_size, GC_GEN0_POISON);
 #endif
 
     /*
@@ -1056,6 +1107,7 @@ static void collect_gen1_garbage(size_t min_free)
     
     //debug(("Finished collection with %d bytes to spare (out of %d bytes).\n", gc_gen1_free_space(), gc_gen1_soft_limit));
 
+    gc_in_gen1_collection = false;
     gc_enabled = true;
 
 #ifndef NO_STATS
@@ -1082,7 +1134,7 @@ static void collect_gen1_garbage(size_t min_free)
 #endif
   }
 
-  update_soft_limit(min_free + gc_gen0_size);
+  update_soft_limit(min_free);
 
   if (gc_gen1_free_space() < min_free)
   {
@@ -1128,9 +1180,7 @@ static void collect_gen1_garbage(size_t min_free)
       _out_of_memory();
   }
 
-  gc_in_gen1_collection = false;
+  /* If Gen-1 was invoked within Gen-0, skip the rest: Gen-0 is empty, we're done. */
-
-  /* If Gen-1 was invoked within Gen-0, skip the rest: we're done. */
   if (gc_in_gen0_collection && collected_garbage)
     longjmp(gc_gen0_end_ctx, 1);
 }

gc.h

@@ -59,6 +59,8 @@ typedef void (builtin_fn_t)(struct interp_state *state);
 #define FALSE_VALUE       SPECIAL_VALUE(1)
 #define TRUE_VALUE        SPECIAL_VALUE(2)
 #define UNDEFINED         SPECIAL_VALUE(3)
+#define GC_GEN0_POISON    SPECIAL_VALUE(4)
+#define GC_GEN1_POISON    SPECIAL_VALUE(5)
 
 #define TYPE_TAG_BOX      TYPE_TAG(0)
 #define TYPE_TAG_VECTOR   TYPE_TAG(1)