Skip to content

Commit 74ce6cf

Browse files
authored
minor NFC in GC codebase (#50991)
- Use `GC_PAGE_ALLOCATED`instead of hardcoded constant - Remove stale eytzinger tree comment from `gc_mark_outrefs` - Add some comments to explain why push_lf_page_metadata_back/pop_lf_page_metadata_back can only use a CAS and not be vulnerable to ABA
1 parent 8be469e commit 74ce6cf

File tree

3 files changed

+8
-2
lines changed

3 files changed

+8
-2
lines changed

src/gc-pages.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ NOINLINE jl_gc_pagemeta_t *jl_gc_alloc_page(void) JL_NOTSAFEPOINT
127127
meta = pop_lf_page_metadata_back(&global_page_pool_clean);
128128
if (meta != NULL) {
129129
uv_mutex_unlock(&gc_perm_lock);
130-
gc_alloc_map_set(meta->data, 1);
130+
gc_alloc_map_set(meta->data, GC_PAGE_ALLOCATED);
131131
goto exit;
132132
}
133133
// must map a new set of pages

src/gc.c

-1
Original file line numberDiff line numberDiff line change
@@ -2423,7 +2423,6 @@ FORCE_INLINE void gc_mark_outrefs(jl_ptls_t ptls, jl_gc_markqueue_t *mq, void *_
24232423
uint8_t bits = (gc_old(o->header) && !mark_reset_age) ? GC_OLD_MARKED : GC_MARKED;
24242424
int update_meta = __likely(!meta_updated && !gc_verifying);
24252425
int foreign_alloc = 0;
2426-
// directly point at eyt_obj_in_img to encourage inlining
24272426
if (update_meta && o->bits.in_image) {
24282427
foreign_alloc = 1;
24292428
update_meta = 0;

src/gc.h

+7
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,8 @@ typedef struct _mallocarray_t {
146146

147147
// pool page metadata
148148
typedef struct _jl_gc_pagemeta_t {
149+
// next metadata structre in per-thread list
150+
// or in one of the `jl_gc_global_page_pool_t`
149151
struct _jl_gc_pagemeta_t *next;
150152
// index of pool that owns this page
151153
uint8_t pool_n;
@@ -203,6 +205,11 @@ STATIC_INLINE void gc_backoff(int *i) JL_NOTSAFEPOINT
203205

204206
// Lock-free stack implementation taken
205207
// from Herlihy's "The Art of Multiprocessor Programming"
208+
// XXX: this is not a general-purpose lock-free stack. We can
209+
// get away with just using a CAS and not implementing some ABA
210+
// prevention mechanism since once a node is popped from the
211+
// `jl_gc_global_page_pool_t`, it may only be pushed back to them
212+
// in the sweeping phase, which is serial
206213

207214
STATIC_INLINE void push_lf_page_metadata_back(jl_gc_global_page_pool_t *pool, jl_gc_pagemeta_t *elt) JL_NOTSAFEPOINT
208215
{

0 commit comments

Comments
 (0)