Skip to content

Commit dbe0604

Browse files
committed
Remove unused mark_barrier and done_barrier struct fields
Phase 5.3 replaced barrier-based dispatch with per-worker condvar dispatch (see _PyGC_DispatchAndWait and the per-worker wake_mutex/ wake_cond plus pool-level done_mutex/done_cond). The mark_barrier and done_barrier _PyGCBarrier fields stayed in _PyParallelGCState with their Init/Fini calls but were no longer waited on. Drop the two unused fields, their Init/Fini calls, and update the struct + worker-loop documentation comments. startup_barrier stays — still used for the once-per-pool worker-ready handshake during ParallelStart. Verified: - Full distclean rebuild required (struct layout changed; the Makefile doesn't track header changes through all .o files — without distclean, stale .o files with the old struct offsets hit an assertion at runtime). - GIL test suite: 124 tests pass (test_gc, test_gc_parallel_mark_alive, test_gc_ws_deque) — same as before the cleanup. Part of post-Phase-5 cleanup. The corresponding cleanup of FTP's mark_barrier/done_barrier/phase_barrier is deferred until FTP's pool dispatch is also converted to per-worker condvars (a follow-up to the adaptive-controller forward-port).
1 parent e2335fd commit dbe0604

2 files changed

Lines changed: 18 additions & 22 deletions

File tree

Include/internal/pycore_gc_parallel.h

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,10 @@ typedef struct {
204204
// =============================================================================
205205
// Worker Thread Phases
206206
// =============================================================================
207-
// Workers wait on mark_barrier, then dispatch based on current phase.
208-
// The main GC thread sets the phase before signalling the barrier.
207+
// Workers wait on per-worker (wake_mutex, wake_cond) for dispatch, then
208+
// switch on current phase. The main GC thread sets the phase, then sets
209+
// wake_flag and signals wake_cond for each active worker.
210+
// See _PyGC_DispatchAndWait in gc_parallel.c.
209211

210212
typedef enum {
211213
_PyGC_PHASE_IDLE, // Waiting for work
@@ -308,15 +310,15 @@ struct _PyParallelGCState {
308310
// Populated during level-1 expansion, consumed by workers
309311
_PyGCWorkQueue work_queue;
310312

311-
// Synchronizes all workers before marking reachable objects
312-
_PyGCBarrier mark_barrier;
313-
314-
// Synchronizes all worker threads and the main thread at the end of
315-
// parallel collection
316-
_PyGCBarrier done_barrier;
317-
318313
// Synchronizes worker startup - ensures all workers are ready before
319-
// ParallelStart returns (prevents race condition in Stop)
314+
// ParallelStart returns (prevents race condition in Stop).
315+
//
316+
// The previous mark_barrier and done_barrier fields were retired in
317+
// Phase 5.3 when broadcast-barrier dispatch was replaced with per-worker
318+
// condvar dispatch (see _PyGC_DispatchAndWait in gc_parallel.c plus the
319+
// wake_mutex/wake_cond on each worker and done_mutex/done_cond at the
320+
// pool level). startup_barrier remains because the once-per-pool worker
321+
// handshake during ParallelStart is a natural fit for a barrier.
320322
_PyGCBarrier startup_barrier;
321323

322324
// Tracks the number of workers actively running. When this reaches zero

Python/gc_parallel.c

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -537,12 +537,9 @@ _PyGC_ParallelInit(PyInterpreterState *interp, size_t num_workers)
537537
return -1;
538538
}
539539

540-
// Initialize barriers
541-
// mark_barrier: all workers + main thread (to signal start)
542-
// done_barrier: all workers + main thread (to signal completion)
543-
// startup_barrier: all workers + main thread (to ensure workers are ready)
544-
_PyGCBarrier_Init(&par_gc->mark_barrier, (unsigned int)num_workers + 1);
545-
_PyGCBarrier_Init(&par_gc->done_barrier, (unsigned int)num_workers + 1);
540+
// Initialize startup_barrier (worker-ready handshake; only barrier still used).
541+
// mark_barrier/done_barrier dispatch was replaced with per-worker condvars
542+
// (see _PyGC_DispatchAndWait + per-worker wake_mutex/wake_cond).
546543
_PyGCBarrier_Init(&par_gc->startup_barrier, (unsigned int)num_workers + 1);
547544

548545
// Initialize locks
@@ -557,8 +554,6 @@ _PyGC_ParallelInit(PyInterpreterState *interp, size_t num_workers)
557554
PyCOND_FINI(&par_gc->workers_done_cond);
558555
PyMUTEX_FINI(&par_gc->active_lock);
559556
_PyGCBarrier_Fini(&par_gc->startup_barrier);
560-
_PyGCBarrier_Fini(&par_gc->done_barrier);
561-
_PyGCBarrier_Fini(&par_gc->mark_barrier);
562557
_PyGCWorkQueue_Fini(&par_gc->work_queue);
563558
_PyGCSplitVector_Fini(&par_gc->split_vector);
564559
PyMem_Free(par_gc);
@@ -655,9 +650,8 @@ _PyGC_ParallelFini(PyInterpreterState *interp)
655650
}
656651
}
657652

658-
// Clean up barriers
659-
_PyGCBarrier_Fini(&par_gc->mark_barrier);
660-
_PyGCBarrier_Fini(&par_gc->done_barrier);
653+
// Clean up startup_barrier (the only barrier still in use; mark/done
654+
// were replaced with per-worker condvars in Phase 5.3).
661655
_PyGCBarrier_Fini(&par_gc->startup_barrier);
662656

663657
// Clean up split vector
@@ -750,7 +744,7 @@ _PyGC_ParallelStop(PyInterpreterState *interp)
750744
// Wait for workers to finish (they exit after seeing should_exit)
751745
// After joining, clean up each worker's tstate from the main thread.
752746
// This avoids a race condition: if workers did cleanup themselves, there
753-
// would be a window between done_barrier and thread exit where cleanup
747+
// would be a window between worker exit and thread exit where cleanup
754748
// could race with re-initialization if enable_parallel() is called quickly.
755749
for (size_t i = 0; i < par_gc->num_workers; i++) {
756750
_PyParallelGCWorker *worker = &par_gc->workers[i];

0 commit comments

Comments
 (0)