gc: remove atomic operations on bitmaps

Author: luke-gruber

gc/default/default.c

@@ -827,7 +827,6 @@ struct heap_page {
         unsigned int has_remembered_objects : 1;
         unsigned int has_uncollectible_wb_unprotected_objects : 1;
     } flags;
-    bool needs_setup_mark_bits;
     rb_atomic_t before_sweep; // bool
 
     rb_heap_t *heap;
@@ -920,12 +919,6 @@ heap_page_in_global_empty_pages_pool(rb_objspace_t *objspace, struct heap_page *
 #define MARK_IN_BITMAP(bits, p)      ((bits)[BITMAP_INDEX(p)] = (bits)[BITMAP_INDEX(p)] | BITMAP_BIT(p))
 #define CLEAR_IN_BITMAP(bits, p)     ((bits)[BITMAP_INDEX(p)] = (bits)[BITMAP_INDEX(p)] & ~BITMAP_BIT(p))
 
-/* Atomic bitmap operations for use during parallel sweep, where the sweep
- * thread and mutator write barriers may modify different bits in the same
- * bitmap word concurrently. */
-#define ATOMIC_MARK_IN_BITMAP(bits, p)   RUBY_ATOMIC_VALUE_OR((bits)[BITMAP_INDEX(p)], BITMAP_BIT(p))
-#define ATOMIC_CLEAR_IN_BITMAP(bits, p)  RUBY_ATOMIC_VALUE_AND((bits)[BITMAP_INDEX(p)], ~BITMAP_BIT(p))
-
 /* getting bitmap */
 #define GET_HEAP_MARK_BITS(x)           (&GET_HEAP_PAGE(x)->mark_bits[0])
 #define GET_HEAP_PINNED_BITS(x)         (&GET_HEAP_PAGE(x)->pinned_bits[0])
@@ -959,10 +952,10 @@ RVALUE_AGE_SET_BITMAP(VALUE obj, int age)
 
     /* Use atomic operations because the sweep thread may concurrently clear
      * age bits for dead objects in the same bitmap word. */
-    if (age & 1) { RUBY_ATOMIC_VALUE_OR(age_bits[idx], mask); }
-    else         { RUBY_ATOMIC_VALUE_AND(age_bits[idx], ~mask); }
-    if (age & 2) { RUBY_ATOMIC_VALUE_OR(age_bits[idx + 1], mask); }
-    else         { RUBY_ATOMIC_VALUE_AND(age_bits[idx + 1], ~mask); }
+    if (age & 1) { age_bits[idx] |= mask; }
+    else         { age_bits[idx] &= ~mask; }
+    if (age & 2) { age_bits[idx + 1] |= mask; }
+    else         { age_bits[idx + 1] &= ~mask; }
 }
 
 static void
@@ -2599,7 +2592,7 @@ newobj_init(VALUE klass, VALUE flags, int wb_protected, rb_objspace_t *objspace,
 #endif
 
     if (RB_UNLIKELY(wb_protected == FALSE)) {
-        ATOMIC_MARK_IN_BITMAP(GET_HEAP_WB_UNPROTECTED_BITS(obj), obj);
+        MARK_IN_BITMAP(GET_HEAP_WB_UNPROTECTED_BITS(obj), obj);
     }
 
 #if RGENGC_PROFILE
@@ -4112,7 +4105,7 @@ deferred_free(rb_objspace_t *objspace, VALUE obj)
 
 // Clear bits for the page that was swept by the background thread.
 static inline void
-gc_post_sweep_page(rb_objspace_t *objspace, rb_heap_t *heap, struct heap_page *sweep_page, bool force_setup_mark_bits)
+gc_post_sweep_page(rb_objspace_t *objspace, rb_heap_t *heap, struct heap_page *sweep_page)
 {
     GC_ASSERT(sweep_page->heap == heap);
 
@@ -4154,23 +4147,14 @@ gc_post_sweep_page(rb_objspace_t *objspace, rb_heap_t *heap, struct heap_page *s
         bits_t *age_bits = sweep_page->age_bits;
         for (int i = 0; i < bitmap_plane_count; i++) {
             bits_t unmarked = ~bits[i] & slot_mask;
-            RUBY_ATOMIC_VALUE_AND(wb_unprotected_bits[i], ~unmarked);
-            RUBY_ATOMIC_VALUE_AND(age_bits[i * 2],     ~unmarked);
-            RUBY_ATOMIC_VALUE_AND(age_bits[i * 2 + 1], ~unmarked);
+            wb_unprotected_bits[i] &= ~unmarked;
+            age_bits[i * 2] &= ~unmarked;
+            age_bits[i * 2 + 1] &= ~unmarked;
         }
     }
 
     if (!heap->compact_cursor) {
-        if (objspace->background_sweep_mode && !force_setup_mark_bits) {
-            /* Defer gc_setup_mark_bits to gc_sweep_finish on the GC thread,
-             * because it overwrites mark_bits which would race with mutator
-             * write barriers for objects on the same page. */
-            sweep_page->needs_setup_mark_bits = true;
-        }
-        else {
-            gc_setup_mark_bits(sweep_page);
-            sweep_page->needs_setup_mark_bits = false;
-        }
+        gc_setup_mark_bits(sweep_page);
     }
 
     if (RUBY_ATOMIC_PTR_LOAD(heap_pages_deferred_final) && !finalizing) {
@@ -4228,9 +4212,9 @@ gc_sweep_page(rb_objspace_t *objspace, rb_heap_t *heap, struct gc_sweep_context
         bits_t *age_bits = sweep_page->age_bits;
         for (int i = 0; i < bitmap_plane_count; i++) {
             bits_t unmarked = ~bits[i] & slot_mask;
-            RUBY_ATOMIC_VALUE_AND(wb_unprotected_bits[i], ~unmarked);
-            RUBY_ATOMIC_VALUE_AND(age_bits[i * 2],     ~unmarked);
-            RUBY_ATOMIC_VALUE_AND(age_bits[i * 2 + 1], ~unmarked);
+            wb_unprotected_bits[i] &= ~unmarked;
+            age_bits[i * 2] &= ~unmarked;
+            age_bits[i * 2 + 1] &= ~unmarked;
         }
     }
 
@@ -4975,19 +4959,6 @@ gc_sweep_finish(rb_objspace_t *objspace)
 
     objspace->use_background_sweep_thread = false;
 
-    /* Run deferred gc_setup_mark_bits for pages swept by the background thread.
-     * This must run on the GC thread to avoid racing with mutator write barriers
-     * that modify mark_bits and uncollectible_bits. */
-    for (int i = 0; i < HEAP_COUNT; i++) {
-        struct heap_page *page;
-        ccan_list_for_each(&heaps[i].pages, page, page_node) {
-            if (page->needs_setup_mark_bits) {
-                gc_setup_mark_bits(page);
-                page->needs_setup_mark_bits = false;
-            }
-        }
-    }
-
     gc_prof_set_heap_info(objspace);
     heap_pages_free_unused_pages(objspace);
 
@@ -5172,7 +5143,7 @@ gc_sweep_step(rb_objspace_t *objspace, rb_heap_t *heap)
             GC_ASSERT(sweep_page->pre_deferred_free_slots == 0);
         }
         else {
-            gc_post_sweep_page(objspace, heap, sweep_page, false); // clear bits
+            gc_post_sweep_page(objspace, heap, sweep_page); // clear bits
             // Process deferred free objects
             unsigned short deferred_free_freed = 0;
             unsigned short deferred_to_free = sweep_page->pre_deferred_free_slots;
@@ -7653,7 +7624,7 @@ rb_gc_impl_writebarrier_unprotect(void *objspace_ptr, VALUE obj)
             }
 
             RB_DEBUG_COUNTER_INC(obj_wb_unprotect);
-            ATOMIC_MARK_IN_BITMAP(GET_HEAP_WB_UNPROTECTED_BITS(obj), obj);
+            MARK_IN_BITMAP(GET_HEAP_WB_UNPROTECTED_BITS(obj), obj);
         }
         RB_GC_VM_UNLOCK_NO_BARRIER(lev);
     }