Skip to content

Commit 0189543

Browse files
luke-gruberluke-gruber
committed
gc: change some atomic memory orderings
1 parent e7a7070 commit 0189543

File tree

1 file changed

+29
-45
lines changed

1 file changed

+29
-45
lines changed

‎gc/default/default.c‎

Lines changed: 29 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1594,7 +1594,7 @@ RVALUE_MARKED_ATOMIC(rb_objspace_t *objspace, VALUE obj)
15941594
{
15951595
bits_t *bits = GET_HEAP_MARK_BITS(obj);
15961596
struct heap_page *page = GET_HEAP_PAGE(obj);
1597-
bits_t word = __atomic_load_n(&bits[SLOT_BITMAP_INDEX(page, obj)], __ATOMIC_SEQ_CST);
1597+
bits_t word = rbimpl_atomic_value_load((VALUE*)&bits[SLOT_BITMAP_INDEX(page, obj)], RBIMPL_ATOMIC_ACQUIRE);
15981598
return (word & SLOT_BITMAP_BIT(page, obj)) != 0;
15991599
}
16001600

@@ -1946,7 +1946,7 @@ rb_gc_impl_garbage_object_p(void *objspace_ptr, VALUE ptr)
19461946
bool dead = false;
19471947

19481948
// Set to false/true by the ruby GC thread when entering/exiting GC, so shouldn't change throughout this call.
1949-
rb_atomic_t use_sweep_thread = RUBY_ATOMIC_LOAD(objspace->use_background_sweep_thread);
1949+
rb_atomic_t use_sweep_thread = rbimpl_atomic_load(&objspace->use_background_sweep_thread, RBIMPL_ATOMIC_RELAXED);
19501950

19511951
if (!use_sweep_thread) {
19521952
// It's not safe to read flags on an object if the sweep thread is running
@@ -1971,13 +1971,12 @@ rb_gc_impl_garbage_object_p(void *objspace_ptr, VALUE ptr)
19711971
if (!use_sweep_thread) {
19721972
// The ruby GC thread or a user thread called us
19731973
bool marked = RVALUE_MARKED(objspace, ptr);
1974-
GC_ASSERT(marked == RVALUE_MARKED_ATOMIC(objspace, ptr));
1975-
return during_lazy_sweep && !marked && RUBY_ATOMIC_LOAD(page->before_sweep);
1974+
return during_lazy_sweep && !marked && rbimpl_atomic_load(&page->before_sweep, RBIMPL_ATOMIC_RELAXED);
19761975
}
19771976
else if (during_lazy_sweep) {
19781977
// we're currently lazy sweeping with the sweep thread
19791978
bool marked = RVALUE_MARKED_ATOMIC(objspace, ptr); // load it atomically so it can't be re-ordered past the next atomic load
1980-
bool before_sweep = RUBY_ATOMIC_LOAD(page->before_sweep);
1979+
rb_atomic_t before_sweep = rbimpl_atomic_load(&page->before_sweep, RBIMPL_ATOMIC_ACQUIRE);
19811980
bool is_garbage = !marked && before_sweep;
19821981
if (is_garbage) return true;
19831982
if (marked && before_sweep) return false;
@@ -4132,7 +4131,7 @@ gc_post_sweep_page(rb_objspace_t *objspace, rb_heap_t *heap, struct heap_page *s
41324131
GC_ASSERT(RUBY_ATOMIC_LOAD(sweep_page->before_sweep));
41334132
}
41344133
#endif
4135-
RUBY_ATOMIC_SET(sweep_page->before_sweep, 0);
4134+
rbimpl_atomic_store(&sweep_page->before_sweep, 0, RBIMPL_ATOMIC_RELEASE);
41364135

41374136
bits = sweep_page->mark_bits;
41384137

@@ -4186,7 +4185,7 @@ gc_sweep_page(rb_objspace_t *objspace, rb_heap_t *heap, struct gc_sweep_context
41864185
GC_ASSERT(RUBY_ATOMIC_LOAD(sweep_page->before_sweep));
41874186
}
41884187
#endif
4189-
RUBY_ATOMIC_SET(sweep_page->before_sweep, 0);
4188+
rbimpl_atomic_store(&sweep_page->before_sweep, 0, RBIMPL_ATOMIC_RELEASE);
41904189
sweep_page->free_slots = 0;
41914190

41924191
p = (uintptr_t)sweep_page->start;
@@ -4607,7 +4606,7 @@ gc_pre_sweep_page(rb_objspace_t *objspace, rb_heap_t *heap, struct heap_page *pa
46074606
static inline bool
46084607
done_worker_incremental_sweep_steps_p(rb_objspace_t *objspace, rb_heap_t *heap)
46094608
{
4610-
if (ATOMIC_LOAD_RELAXED(heap->foreground_sweep_steps) != heap->background_sweep_steps) {
4609+
if (rbimpl_atomic_load(&heap->foreground_sweep_steps, RBIMPL_ATOMIC_ACQUIRE) != heap->background_sweep_steps) {
46114610
GC_ASSERT(ATOMIC_LOAD_RELAXED(heap->foreground_sweep_steps) > heap->background_sweep_steps);
46124611
return true;
46134612
}
@@ -4942,7 +4941,7 @@ gc_sweep_start(rb_objspace_t *objspace)
49424941
(objspace->profile.latest_gc_info & GPR_FLAG_METHOD) == 0 &&
49434942
!(objspace->hook_events & RUBY_INTERNAL_EVENT_FREEOBJ)) {
49444943

4945-
RUBY_ATOMIC_SET(objspace->use_background_sweep_thread, true);
4944+
rbimpl_atomic_store(&objspace->use_background_sweep_thread, true, RBIMPL_ATOMIC_RELEASE);
49464945
psweep_debug(-1, "[gc] gc_sweep_start: requesting sweep thread\n");
49474946
sweep_lock_lock(&objspace->sweep_lock);
49484947
{
@@ -4952,7 +4951,7 @@ gc_sweep_start(rb_objspace_t *objspace)
49524951
sweep_lock_unlock(&objspace->sweep_lock);
49534952
}
49544953
else {
4955-
RUBY_ATOMIC_SET(objspace->use_background_sweep_thread, false);
4954+
rbimpl_atomic_store(&objspace->use_background_sweep_thread, false, RBIMPL_ATOMIC_RELEASE);
49564955
psweep_debug(-1, "[gc] gc_sweep_start: not using background sweep thread\n");
49574956
}
49584957
}
@@ -5011,17 +5010,16 @@ gc_sweep_finish(rb_objspace_t *objspace)
50115010
gc_report(1, objspace, "gc_sweep_finish\n");
50125011
psweep_debug(-1, "[gc] gc_sweep_finish\n");
50135012

5014-
RUBY_ATOMIC_SET(objspace->use_background_sweep_thread, false);
5013+
rbimpl_atomic_store(&objspace->use_background_sweep_thread, false, RBIMPL_ATOMIC_RELEASE);
50155014

50165015
gc_prof_set_heap_info(objspace);
50175016
heap_pages_free_unused_pages(objspace);
50185017

50195018
for (int i = 0; i < HEAP_COUNT; i++) {
50205019
rb_heap_t *heap = &heaps[i];
50215020

5022-
#ifdef DEBUG_SWEEP_BOOKKEEPING
5021+
#if VM_CHECK_MODE > 0
50235022
{
5024-
/* Assert that every page in this heap was swept. */
50255023
struct heap_page *page;
50265024
ccan_list_for_each(&heap->pages, page, page_node) {
50275025
if (RUBY_ATOMIC_LOAD(page->before_sweep)) {
@@ -5177,10 +5175,10 @@ gc_sweep_step(rb_objspace_t *objspace, rb_heap_t *heap)
51775175
#endif
51785176
psweep_debug(-2, "[gc] gc_sweep_step heap:%p (%ld) use_sweep_thread:%d\n", heap, heap - heaps, objspace->use_background_sweep_thread);
51795177
bool sweep_rest = objspace->sweep_rest;
5180-
bool use_bg_thread = objspace->use_background_sweep_thread;
5178+
bool use_sweep_thread = objspace->use_background_sweep_thread;
51815179

51825180
while (1) {
5183-
bool free_in_user_thread_p = !use_bg_thread;
5181+
bool free_in_user_thread_p = !use_sweep_thread;
51845182
bool dequeued_unswept_page = false;
51855183
// NOTE: pages we dequeue from the sweep thread need to be AFTER the list of heap->free_pages so we don't free from pages
51865184
// we've allocated from since sweep started.
@@ -5219,39 +5217,25 @@ gc_sweep_step(rb_objspace_t *objspace, rb_heap_t *heap)
52195217
if (deferred_to_free > 0) {
52205218
uintptr_t p = (uintptr_t)sweep_page->start;
52215219
bits_t *deferred_bits = sweep_page->deferred_free_bits;
5220+
int total_slots = sweep_page->total_slots;
52225221
short slot_size = sweep_page->slot_size;
5223-
short slot_bits = slot_size / BASE_SLOT_SIZE;
5224-
bits_t slot_mask = heap->slot_bits_mask;
5225-
5226-
int page_rvalue_count = sweep_page->total_slots * slot_bits;
5227-
int bitmap_plane_count = CEILDIV(NUM_IN_PAGE(p) + page_rvalue_count, BITS_BITLENGTH);
5228-
5229-
// First plane: skip out-of-range slots at head of page
5230-
bits_t bitset = deferred_bits[0];
5231-
bitset >>= NUM_IN_PAGE(p);
5232-
bitset &= slot_mask;
5233-
while (bitset) {
5234-
if (bitset & 1) {
5235-
VALUE obj = (VALUE)p;
5236-
GC_ASSERT(GET_HEAP_PAGE(obj) == sweep_page);
5237-
if (deferred_free(objspace, obj)) {
5238-
deferred_free_freed++;
5239-
}
5240-
else {
5241-
deferred_free_final_slots++;
5242-
}
5243-
}
5244-
p += slot_size;
5245-
bitset >>= slot_bits;
5222+
5223+
int bitmap_plane_count = CEILDIV(total_slots, BITS_BITLENGTH);
5224+
int out_of_range_bits = total_slots % BITS_BITLENGTH;
5225+
bits_t bitset;
5226+
5227+
if (out_of_range_bits != 0) {
5228+
deferred_bits[bitmap_plane_count - 1] &= (((bits_t)1 << out_of_range_bits) - 1);
52465229
}
5247-
p = (uintptr_t)sweep_page->start + (BITS_BITLENGTH - NUM_IN_PAGE((uintptr_t)sweep_page->start)) * BASE_SLOT_SIZE;
52485230

5249-
for (int i = 1; i < bitmap_plane_count; i++) {
5250-
bitset = deferred_bits[i] & slot_mask;
5231+
for (int i = 0; i < bitmap_plane_count; i++) {
5232+
bitset = deferred_bits[i];
5233+
p = (uintptr_t)sweep_page->start + (i * BITS_BITLENGTH * slot_size);
52515234
while (bitset) {
52525235
if (bitset & 1) {
52535236
VALUE obj = (VALUE)p;
52545237
GC_ASSERT(GET_HEAP_PAGE(obj) == sweep_page);
5238+
GC_ASSERT(!RVALUE_MARKED(objspace, obj));
52555239
if (deferred_free(objspace, obj)) {
52565240
deferred_free_freed++;
52575241
}
@@ -5260,11 +5244,11 @@ gc_sweep_step(rb_objspace_t *objspace, rb_heap_t *heap)
52605244
}
52615245
}
52625246
p += slot_size;
5263-
bitset >>= slot_bits;
5247+
bitset >>= 1;
52645248
}
5265-
p = (uintptr_t)sweep_page->start + (BITS_BITLENGTH * (i + 1) - NUM_IN_PAGE((uintptr_t)sweep_page->start)) * BASE_SLOT_SIZE;
52665249
}
52675250
}
5251+
GC_ASSERT(deferred_to_free == (deferred_free_freed + deferred_free_final_slots));
52685252

52695253
ctx.final_slots = sweep_page->pre_final_slots + deferred_free_final_slots;
52705254
ctx.freed_slots = sweep_page->pre_freed_slots + deferred_free_freed;
@@ -5369,8 +5353,8 @@ gc_sweep_step(rb_objspace_t *objspace, rb_heap_t *heap)
53695353
heap_add_freepage(heap, sweep_page, "gc_sweep_step");
53705354
swept_slots += free_slots;
53715355
if (swept_slots > GC_INCREMENTAL_SWEEP_SLOT_COUNT) {
5372-
if (!sweep_rest && use_bg_thread) {
5373-
RUBY_ATOMIC_INC(heap->foreground_sweep_steps); // signal sweep thread to move on
5356+
if (!sweep_rest && use_sweep_thread) {
5357+
rbimpl_atomic_inc(&heap->foreground_sweep_steps, RBIMPL_ATOMIC_RELEASE); // signal sweep thread to move on
53745358
}
53755359
psweep_debug(0, "[gc] gc_sweep_step got to SWEEP_SLOT_COUNT, break\n");
53765360
break;

0 commit comments

Comments
 (0)