From 8e877cbb01d2c26a3fdcb7b9302f4d3ead229f9f Mon Sep 17 00:00:00 2001
From: Diogo Netto <61364108+d-netto@users.noreply.github.com>
Date: Sun, 9 Jul 2023 21:40:14 -0300
Subject: [PATCH] relax assertion involving pg->nold to reflect that it may be
 a bit inaccurate with parallel marking (#50466)

---
 src/gc.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/src/gc.c b/src/gc.c
index cb86d5d4535a7..1b14452603239 100644
--- a/src/gc.c
+++ b/src/gc.c
@@ -854,7 +854,7 @@ STATIC_INLINE void gc_setmark_pool_(jl_ptls_t ptls, jl_taggedvalue_t *o,
     if (mark_mode == GC_OLD_MARKED) {
         ptls->gc_cache.perm_scanned_bytes += page->osize;
         static_assert(sizeof(_Atomic(uint16_t)) == sizeof(page->nold), "");
-        jl_atomic_fetch_add_relaxed((_Atomic(uint16_t)*)&page->nold, 1);
+        page->nold++;
     }
     else {
         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
         nfree = (GC_PAGE_SZ - GC_PAGE_OFFSET) / osize;
         goto done;
     }
-    // For quick sweep, we might be able to skip the page if the page doesn't
-    // have any young live cell before marking.
-    if (!sweep_full && !pg->has_young) {
-        assert(!prev_sweep_full || pg->prev_nold >= pg->nold);
-        if (!prev_sweep_full || pg->prev_nold == pg->nold) {
-            // the position of the freelist begin/end in this page
-            // is stored in its metadata
-            if (pg->fl_begin_offset != (uint16_t)-1) {
-                *pfl = page_pfl_beg(pg);
-                pfl = (jl_taggedvalue_t**)page_pfl_end(pg);
+    // note that `pg->nold` may not be accurate with multithreaded marking since
+    // two threads may race when trying to set the mark bit in `gc_try_setmark_tag`.
+    // We're basically losing a bit of precision in the sweep phase at the cost of
+    // making the mark phase considerably cheaper.
+    // See issue #50419
+    if (jl_n_markthreads == 0) {
+        // For quick sweep, we might be able to skip the page if the page doesn't
+        // have any young live cell before marking.
+        if (!sweep_full && !pg->has_young) {
+            assert(!prev_sweep_full || pg->prev_nold >= pg->nold);
+            if (!prev_sweep_full || pg->prev_nold == pg->nold) {
+                // the position of the freelist begin/end in this page
+                // is stored in its metadata
+                if (pg->fl_begin_offset != (uint16_t)-1) {
+                    *pfl = page_pfl_beg(pg);
+                    pfl = (jl_taggedvalue_t**)page_pfl_end(pg);
+                }
+                freedall = 0;
+                nfree = pg->nfree;
+                goto done;
             }
-            freedall = 0;
-            nfree = pg->nfree;
-            goto done;
         }
     }