Fix several significant bugs in the generational collector.

No known issues remaining at this time.

gc.c

@@ -369,7 +369,7 @@ static void update_weak_box_list(void);
 static void gc_gen0_init(size_t gen0_size);
 static void gc_gen1_init(size_t min_size, size_t max_size);
 
-static int gen1_gc_range_of(void *object) __attribute__ ((const));
+static int gc_gen1_range_of(void *object) __attribute__ ((const));
 static size_t gc_gen1_block_of(void *obj) __attribute__ ((const));
 static inline size_t gc_gen1_free_space(void);
 
@@ -453,9 +453,6 @@ 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
@@ -478,9 +475,9 @@ static void collect_gen0_garbage(void)
     {
       char *object_ptr = gc_gen1_free_ptr;
       const size_t used_bytes = gc_gen1_free_ptr - gc_gen1_ranges[gc_gen1_current_range];
-      const int current_block_groups = (used_bytes + GC_DIRTY_BLOCK_SIZE - 1) / GC_DIRTY_BLOCK_SIZE;
+      const int current_blocks = (used_bytes + GC_DIRTY_BLOCK_SIZE - 1) / GC_DIRTY_BLOCK_SIZE;
+      const int current_block_groups = (current_blocks + 31) / 32;
       int group;
-      int bit;
 
       gc_in_gen0_collection = true;
 
@@ -490,23 +487,34 @@ static void collect_gen0_garbage(void)
       /* 2. Locate and transfer Gen-0 references from dirty Gen-1 blocks. */
       for (group = 0; group < current_block_groups; ++group)
       {
-        for (bit = 0; bit < 32; ++bit)
+        uint32_t bits = gc_gen1_dirty_bits[group];
-        {
-          if (gc_gen1_dirty_bits[group] & (1UL << bit))
-          {
-            /* Find first object in block */
-            const int block = group * 32 + bit;
-            char *block_obj = gc_gen1_block_starts[block];
 
-            assert(block_obj);
+        if (bits)
+        {
+          int block;
+
+          for (block = group * 32; bits; bits >>= 1, ++block)
+          {
+            if (bits & 1)
+            {
+              /* Find first object in the block */
+              char *block_obj = gc_gen1_block_starts[block];
+              char *block_end = gc_gen1_ranges[gc_gen1_current_range]
+                              + ((block+1) * GC_DIRTY_BLOCK_SIZE);
+
+              assert(block_obj && (gc_gen1_block_of(block_obj) == block));
 
               /* For each object in block: transfer children */
               do {
                 block_obj += gc_align(transfer_children((object_t*)block_obj));
-            } while (gen1_gc_range_of(block_obj) == block);
+                assert(gc_gen1_range_of(block_obj) == gc_gen1_current_range);
+              } while (block_obj < block_end);
             }
           }
         }
+      }
+
+      gc_gen1_clear_dirty_bits();
 
       /* Transfer Gen-0 children of objects newly moved to Gen-1 */
       while (object_ptr < gc_gen1_free_ptr)
@@ -518,11 +526,13 @@ static void collect_gen0_garbage(void)
       process_weak_boxes();
       process_wills();
 
-      /* Keep transferring until no more objects in the new range refer to the old one.
+      /*
+       * Keep transferring until no more objects in the new range refer to the old one.
        * This is so that values which are otherwise unreachable, but have finalizers which
        * may be able to reach them, are not collected prematurely. process_wills() transfers
        * the value of any will newly placed on the active list. Note that these values may
-      * be finalized in any order, and that any weak references have already been cleared. */
+       * be finalized in any order, and that any weak references have already been cleared.
+       */
       while (object_ptr < gc_gen1_free_ptr)
       {
         object_ptr += gc_align(transfer_children((object_t*)object_ptr));
@@ -536,7 +546,6 @@ static void collect_gen0_garbage(void)
 #endif
 
       /* 4. Reset Gen-0 range to 'empty' state. */
-      gc_gen1_clear_dirty_bits();
       gc_gen0_free_ptr = gc_gen0_range;
 
 #ifndef NO_STATS
@@ -601,21 +610,22 @@ void *gc_alloc(size_t nbytes)
 
 /****************************** Gen-1 Collector *****************************/
 
-static int gen1_gc_range_of(void *object)
+static int gc_gen1_range_of(void *object)
 {
-  if (((value_t)object >= (value_t)gc_gen1_ranges[0]) &&
+  const char *const ptr = (char*)object;
-      ((value_t)object <  (value_t)(gc_gen1_ranges[0] + gc_gen1_max_size)))
+
+  if ((ptr >= gc_gen1_ranges[0]) && (ptr < (gc_gen1_ranges[0] + gc_gen1_max_size)))
     return 0;
-  if (((value_t)object >= (value_t)gc_gen1_ranges[1]) &&
+  else if ((ptr >= gc_gen1_ranges[1]) && (ptr < (gc_gen1_ranges[1] + gc_gen1_max_size)))
-      ((value_t)object <  (value_t)(gc_gen1_ranges[1] + gc_gen1_max_size)))
     return 1;
+  else
     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);
+         (gc_gen1_range_of(_get_object(_get_object(v)->forward)) == gc_gen1_current_range);
 }
 
 /* Only useful AFTER all reachable objects have been processed. */
@@ -624,11 +634,11 @@ 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));
+         (gc_gen1_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)) !=
+         (!is_broken_heart(v) || (gc_gen1_range_of(_get_object(_get_object(v)->forward)) !=
                                   gc_gen1_current_range));
 }
 
@@ -646,22 +656,21 @@ static void gc_gen1_init(size_t min_size, size_t max_size)
 
   gc_gen1_min_size   = min_size;
   gc_gen1_max_size   = max_size;
-  gc_gen1_soft_limit = 2*gc_gen1_min_size;
+  gc_gen1_soft_limit = 2*min_size;
   gc_gen1_range_end  = gc_gen1_free_ptr + gc_gen1_soft_limit;
 
-  {
   gc_gen1_max_blocks = ((size_t)2 << 30) / GC_DIRTY_BLOCK_SIZE;
-    gc_gen1_dirty_bits = (uint32_t*)malloc(4 * ((gc_gen1_max_blocks + 31) / 32));
+
+  gc_gen1_dirty_bits = (uint32_t*)calloc((gc_gen1_max_blocks + 31) / 32, sizeof(uint32_t));
   release_assert(gc_gen1_dirty_bits);
-    gc_gen1_block_starts = (char**)malloc(gc_gen1_max_blocks * sizeof(char*));
+
-    gc_gen1_clear_dirty_bits();
+  gc_gen1_block_starts = (char**)calloc(gc_gen1_max_blocks, sizeof(char*));
-    memset(gc_gen1_block_starts, 0, gc_gen1_max_blocks * sizeof(char*));
+  release_assert(gc_gen1_block_starts);
-  }
 }
 
 static void gc_gen1_clear_dirty_bits(void)
 {
-  memset(gc_gen1_dirty_bits, 0, 4 * ((gc_gen1_max_blocks + 31) / 32));
+  memset(gc_gen1_dirty_bits, 0, sizeof(uint32_t) * ((gc_gen1_max_blocks + 31) / 32));
 }
 
 static void *gc_alloc_gen1(size_t nbytes)
@@ -700,17 +709,17 @@ static void transfer_object(value_t *value)
     void *newobj;
 
     assert(is_gen0_object(*value) ||
-           (gen1_gc_range_of(obj) == gc_gen1_other_range()));
+           (gc_gen1_range_of(obj) == gc_gen1_other_range()));
 
     if (obj->tag == BROKEN_HEART)
     {
-      if (gen1_gc_range_of(_get_object(obj->forward)) != gc_gen1_current_range)
+      if (gc_gen1_range_of(_get_object(obj->forward)) != gc_gen1_current_range)
       {
         /* Gen-0 object was transferred into old range; needs to move to current range */
         transfer_object(&obj->forward);
       }
 
-      assert(gen1_gc_range_of(_get_object(obj->forward)) == gc_gen1_current_range);
+      assert(gc_gen1_range_of(_get_object(obj->forward)) == gc_gen1_current_range);
 
       /* Object has already been moved; just update the reference */
       *value = obj->forward;
@@ -1013,16 +1022,16 @@ static void update_soft_limit(size_t min_free)
 
 static size_t gc_gen1_block_of(void *obj)
 {
-  const intptr_t offset = (intptr_t)obj
+  const intptr_t offset = (uintptr_t)obj
-                        - (intptr_t)gc_gen1_ranges[gc_gen1_current_range];
+                        - (uintptr_t)gc_gen1_ranges[gc_gen1_current_range];
 
-  return (offset & ((intptr_t)2 << 30)) / GC_DIRTY_BLOCK_SIZE;
+  return (offset & (((uintptr_t)2 << 30) - 1)) / GC_DIRTY_BLOCK_SIZE;
 }
 
 void _gc_mark_updated_gen1_object(value_t v)
 {
   const size_t block = gc_gen1_block_of(_get_object(v));
-  assert(is_object(v) && !is_gen0_object(v));
+  assert(is_object(v) && !is_gen0_object(v) && gc_gen1_block_starts[block]);
   gc_gen1_dirty_bits[block / 32] |= (1UL << (block % 32));
 }
 
@@ -1035,6 +1044,7 @@ static void _out_of_memory(void)
 
 static void collect_gen1_garbage(size_t min_free)
 {
+  bool was_in_gen0_collection = gc_in_gen0_collection;
   bool collected_garbage = false;
 
   /* If Gen-1 free space falls below used portion of Gen-0, chaos may ensue. */
@@ -1056,11 +1066,13 @@ static void collect_gen1_garbage(size_t min_free)
     debug(("Performing Gen-1 garbage collection pass...\n"));
 
     gc_enabled = false;
+    gc_in_gen0_collection = false;
     gc_in_gen1_collection = true;
 
+    gc_gen1_clear_dirty_bits();
     swap_gen1_gc_ranges();
 
-    /* Record the start of each Gen-1 block as we go. */
+    /* Record the start of each Gen-1 block as objects are moved to the new range. */
     memset(gc_gen1_block_starts, 0, gc_gen1_max_blocks * sizeof(char*));
 
     /* New "current" range is initially empty, old one is full */
@@ -1074,6 +1086,7 @@ static void collect_gen1_garbage(size_t min_free)
     while (object_ptr < gc_gen1_free_ptr)
     {
       object_ptr += gc_align(transfer_children((object_t*)object_ptr));
+      assert(gc_gen1_range_of(object_ptr) == gc_gen1_current_range);
     }
 
     /* These have to be examined after normal reachability has been determined */
@@ -1088,6 +1101,7 @@ static void collect_gen1_garbage(size_t min_free)
     while (object_ptr < gc_gen1_free_ptr)
     {
       object_ptr += gc_align(transfer_children((object_t*)object_ptr));
+      assert(gc_gen1_range_of(object_ptr) == gc_gen1_current_range);
     }
 
     update_weak_box_list();
@@ -1103,7 +1117,6 @@ static void collect_gen1_garbage(size_t min_free)
      * have been moved to the Gen-1 memory region.
      */
 
-    gc_gen1_clear_dirty_bits();
     gc_gen0_free_ptr = gc_gen0_range;
     collected_garbage = true;
     
@@ -1141,7 +1154,7 @@ static void collect_gen1_garbage(size_t min_free)
   if (gc_gen1_free_space() < min_free)
   {
     size_t bytes_used = gc_gen1_free_ptr - gc_gen1_ranges[gc_gen1_current_range];
-    size_t need_bytes = bytes_used + min_free;
+    size_t need_bytes = bytes_used + min_free + gc_gen0_size;
 
     /* If GC is disabled then we can't move anything, so reallocating is impossible. */
     if (!gc_enabled)
@@ -1183,7 +1196,7 @@ static void collect_gen1_garbage(size_t min_free)
   }
 
   /* If Gen-1 was invoked within Gen-0, skip the rest: Gen-0 is empty, we're done. */
-  if (gc_in_gen0_collection && collected_garbage)
+  if (was_in_gen0_collection && collected_garbage)
     longjmp(gc_gen0_end_ctx, 1);
 }
 

rosella.c

@@ -47,7 +47,7 @@ int main(int argc, char **argv)
   }
 #endif
 
-  gc_init(12*1024*1024, 64*1024*1024);
+  gc_init(8*1024*1024, 16*1024*1024);
   builtin_init();
   interpreter_init();