Skip to content

Commit 85d7b42

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 4cf3007 commit 85d7b42

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
@@ -854,7 +854,7 @@ STATIC_INLINE void gc_setmark_pool_(jl_ptls_t ptls, jl_taggedvalue_t *o,
854854
if (mark_mode == GC_OLD_MARKED) {
855855
ptls->gc_cache.perm_scanned_bytes += page->osize;
856856
static_assert(sizeof(_Atomic(uint16_t)) == sizeof(page->nold), "");
857-
jl_atomic_fetch_add_relaxed((_Atomic(uint16_t)*)&page->nold, 1);
857+
page->nold++;
858858
}
859859
else {
860860
ptls->gc_cache.scanned_bytes += page->osize;
@@ -1374,20 +1374,27 @@ static jl_taggedvalue_t **gc_sweep_page(jl_gc_pool_t *p, jl_gc_pagemeta_t **allo
13741374
nfree = (GC_PAGE_SZ - GC_PAGE_OFFSET) / osize;
13751375
goto done;
13761376
}
1377-
// For quick sweep, we might be able to skip the page if the page doesn't
1378-
// have any young live cell before marking.
1379-
if (!sweep_full && !pg->has_young) {
1380-
assert(!prev_sweep_full || pg->prev_nold >= pg->nold);
1381-
if (!prev_sweep_full || pg->prev_nold == pg->nold) {
1382-
// the position of the freelist begin/end in this page
1383-
// is stored in its metadata
1384-
if (pg->fl_begin_offset != (uint16_t)-1) {
1385-
*pfl = page_pfl_beg(pg);
1386-
pfl = (jl_taggedvalue_t**)page_pfl_end(pg);
1377+
// note that `pg->nold` may not be accurate with multithreaded marking since
1378+
// two threads may race when trying to set the mark bit in `gc_try_setmark_tag`.
1379+
// We're basically losing a bit of precision in the sweep phase at the cost of
1380+
// making the mark phase considerably cheaper.
1381+
// See issue #50419
1382+
if (jl_n_markthreads == 0) {
1383+
// For quick sweep, we might be able to skip the page if the page doesn't
1384+
// have any young live cell before marking.
1385+
if (!sweep_full && !pg->has_young) {
1386+
assert(!prev_sweep_full || pg->prev_nold >= pg->nold);
1387+
if (!prev_sweep_full || pg->prev_nold == pg->nold) {
1388+
// the position of the freelist begin/end in this page
1389+
// is stored in its metadata
1390+
if (pg->fl_begin_offset != (uint16_t)-1) {
1391+
*pfl = page_pfl_beg(pg);
1392+
pfl = (jl_taggedvalue_t**)page_pfl_end(pg);
1393+
}
1394+
freedall = 0;
1395+
nfree = pg->nfree;
1396+
goto done;
13871397
}
1388-
freedall = 0;
1389-
nfree = pg->nfree;
1390-
goto done;
13911398
}
13921399
}
13931400

0 commit comments

Comments
 (0)