Skip to content

Commit dcafe91

Browse files
d-nettoKristofferC
authored andcommitted
relax assertion involving pg->nold to reflect that it may be a bit inaccurate with parallel marking (#50466)
(cherry picked from commit 8e877cb)
1 parent 076b68f commit dcafe91

File tree

1 file changed

+21
-14
lines changed

1 file changed

+21
-14
lines changed

src/gc.c

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -978,7 +978,7 @@ STATIC_INLINE void gc_setmark_pool_(jl_ptls_t ptls, jl_taggedvalue_t *o,
978978
if (mark_mode == GC_OLD_MARKED) {
979979
ptls->gc_cache.perm_scanned_bytes += page->osize;
980980
static_assert(sizeof(_Atomic(uint16_t)) == sizeof(page->nold), "");
981-
jl_atomic_fetch_add_relaxed((_Atomic(uint16_t)*)&page->nold, 1);
981+
page->nold++;
982982
}
983983
else {
984984
ptls->gc_cache.scanned_bytes += page->osize;
@@ -1549,20 +1549,27 @@ static jl_taggedvalue_t **sweep_page(jl_gc_pool_t *p, jl_gc_pagemeta_t *pg, jl_t
15491549
nfree = (GC_PAGE_SZ - GC_PAGE_OFFSET) / osize;
15501550
goto done;
15511551
}
1552-
// For quick sweep, we might be able to skip the page if the page doesn't
1553-
// have any young live cell before marking.
1554-
if (!sweep_full && !pg->has_young) {
1555-
assert(!prev_sweep_full || pg->prev_nold >= pg->nold);
1556-
if (!prev_sweep_full || pg->prev_nold == pg->nold) {
1557-
// the position of the freelist begin/end in this page
1558-
// is stored in its metadata
1559-
if (pg->fl_begin_offset != (uint16_t)-1) {
1560-
*pfl = page_pfl_beg(pg);
1561-
pfl = (jl_taggedvalue_t**)page_pfl_end(pg);
1552+
// note that `pg->nold` may not be accurate with multithreaded marking since
1553+
// two threads may race when trying to set the mark bit in `gc_try_setmark_tag`.
1554+
// We're basically losing a bit of precision in the sweep phase at the cost of
1555+
// making the mark phase considerably cheaper.
1556+
// See issue #50419
1557+
if (jl_n_markthreads == 0) {
1558+
// For quick sweep, we might be able to skip the page if the page doesn't
1559+
// have any young live cell before marking.
1560+
if (!sweep_full && !pg->has_young) {
1561+
assert(!prev_sweep_full || pg->prev_nold >= pg->nold);
1562+
if (!prev_sweep_full || pg->prev_nold == pg->nold) {
1563+
// the position of the freelist begin/end in this page
1564+
// is stored in its metadata
1565+
if (pg->fl_begin_offset != (uint16_t)-1) {
1566+
*pfl = page_pfl_beg(pg);
1567+
pfl = (jl_taggedvalue_t**)page_pfl_end(pg);
1568+
}
1569+
freedall = 0;
1570+
nfree = pg->nfree;
1571+
goto done;
15621572
}
1563-
freedall = 0;
1564-
nfree = pg->nfree;
1565-
goto done;
15661573
}
15671574
}
15681575

0 commit comments

Comments
 (0)