Fix GC compaction

Shopify · Aug 19, 2020 · d3554d3 · d3554d3
1 parent 8946b0c
commit d3554d3
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 21 deletions.
diff --git a/gc.c b/gc.c
@@ -2058,6 +2058,16 @@ remove_obj_from_freelist(rb_heap_t *heap, VALUE obj)
         GC_ASSERT(prev == NULL);
         heap->freelist = next;
     }
+
+    if (p == GET_HEAP_PAGE(p)->freelist) {
+        GC_ASSERT(prev == NULL);
+        GET_HEAP_PAGE(p)->freelist = next;
+    }
+
+    if (p == GET_HEAP_PAGE(p)->freelist_tail) {
+        GC_ASSERT(next == NULL);
+        GET_HEAP_PAGE(p)->freelist_tail = prev;
+    }
 }
 
 static inline VALUE
@@ -4443,6 +4453,8 @@ gc_page_sweep(rb_objspace_t *objspace, rb_heap_t *heap, struct heap_page *sweep_
         RVALUE *p = pstart + i;
         VALUE vp = (VALUE)p;
 
+        GC_ASSERT(!RVALUE_PAGE_MARKING(sweep_page, p) || sweep_page->flags.has_remembered_objects);
+
         bitset = (~bits[BITMAP_INDEX(p)] >> BITMAP_OFFSET(p)) & 1;
 
         asan_unpoison_object(vp, false);
@@ -4452,7 +4464,6 @@ gc_page_sweep(rb_objspace_t *objspace, rb_heap_t *heap, struct heap_page *sweep_
                 gc_report(2, objspace, "page_sweep: free %p\n", (void *)p);
 #if RGENGC_CHECK_MODE
                 if (!is_full_marking(objspace)) {
-                    // if (RVALUE_OLD_P(vp)) rb_bug("page_sweep: %p - old while minor GC.", (void *)p);
                     if (RVALUE_OLD_P(vp)) rb_bug("page_sweep: old while minor GC: %s.", obj_info(p));
                     if (rgengc_remembered_sweep(objspace, vp)) rb_bug("page_sweep: %p - remembered.", (void *)p);
                 }
@@ -7843,6 +7854,7 @@ gc_is_moveable_obj(rb_objspace_t *objspace, VALUE obj)
       case T_NIL:
       case T_MOVED:
       case T_ZOMBIE:
+      case T_GARBAGE:
         return FALSE;
       case T_SYMBOL:
         if (DYNAMIC_SYM_P(obj) && (RSYMBOL(obj)->id & ~ID_SCOPE_MASK)) {
@@ -7925,6 +7937,8 @@ gc_move(rb_objspace_t *objspace, VALUE scan, VALUE free, struct RMoved * moved_l
         st_insert(objspace->obj_to_id_tbl, (st_data_t)dest, id);
     }
 
+    remove_obj_from_freelist(heap_eden, (VALUE)dest);
+
     /* Move the object */
     memcpy(dest, src, sizeof(RVALUE));
     memset(src, 0, sizeof(RVALUE));
@@ -7978,12 +7992,12 @@ static void
 advance_cursor(struct heap_cursor *free, struct heap_page **page_list)
 {
     if (free->slot == free->page->start + free->page->total_slots - 1) {
-        free->index--;
+        free->index++;
         free->page = page_list[free->index];
         free->slot = free->page->start;
     }
     else {
-        free->slot--;
+        free->slot++;
     }
 }
 
@@ -8027,7 +8041,7 @@ init_cursors(rb_objspace_t *objspace, struct heap_cursor *free, struct heap_curs
     page = page_list[total_pages - 1];
     scan->index = total_pages - 1;
     scan->page = page;
-    scan->slot = page->start + page->total_slots * 2 - 2;
+    scan->slot = page->start + page->total_slots - 1;
     scan->objspace = objspace;
 }
 
@@ -8703,16 +8717,10 @@ gc_ref_update(void *vstart, void *vend, size_t stride, void * data)
 {
     rb_objspace_t * objspace;
     struct heap_page *page;
-    short free_slots = 0;
 
     VALUE v = (VALUE)vstart;
     objspace = (rb_objspace_t *)data;
     page = GET_HEAP_PAGE(v);
-    asan_unpoison_memory_region(&page->freelist, sizeof(RVALUE*), false);
-    page->freelist = NULL;
-    asan_poison_memory_region(&page->freelist, sizeof(RVALUE*));
-    page->flags.has_uncollectible_shady_objects = FALSE;
-    page->flags.has_remembered_objects = FALSE;
 
     /* For each object on the page */
     for (; v != (VALUE)vend; v += stride) {
@@ -8722,11 +8730,7 @@ gc_ref_update(void *vstart, void *vend, size_t stride, void * data)
 
             switch (BUILTIN_TYPE(v)) {
               case T_NONE:
-                heap_page_add_freeobj(objspace, page, v);
-                free_slots++;
-                break;
               case T_MOVED:
-                break;
               case T_ZOMBIE:
                 break;
               default:
@@ -8746,7 +8750,6 @@ gc_ref_update(void *vstart, void *vend, size_t stride, void * data)
         }
     }
 
-    page->free_slots = free_slots;
     return 0;
 }
 

diff --git a/test/ruby/test_gc_compact.rb b/test/ruby/test_gc_compact.rb
@@ -35,8 +35,6 @@ def find_object_in_recycled_slot(addresses)
   end
 
   def test_complex_hash_keys
-    skip
-
     list_of_objects = big_list
     hash = list_of_objects.hash
     GC.verify_compaction_references(toward: :empty)
@@ -54,16 +52,12 @@ def walk_ast ast
   end
 
   def test_ast_compacts
-    skip
-
     ast = RubyVM::AbstractSyntaxTree.parse_file __FILE__
     assert GC.compact
     walk_ast ast
   end
 
   def test_compact_count
-    skip
-
     count = GC.stat(:compact_count)
     GC.compact
     assert_equal count + 1, GC.stat(:compact_count)