Skip to content

Commit 9c77265

Browse files
committed
Revert FTP _PyGC_GetParallelWorkers returning adaptive_workers
The earlier change (Phase 6 step 3, commit 5501b73 in pre-rebase SHAs) made _PyGC_GetParallelWorkers return pool->adaptive_workers so FTP collections would scale per-collection like the GIL build. That broke _PyGC_ParallelPropagateAliveWithPool: assert(pool->num_workers == num_workers); The pool dispatch path requires num_workers == pool->num_workers because its internal phase_barrier is sized pool->num_workers at pool init and broadcasts dispatch to all of them. Returning a smaller count makes the assertion fire (and would deadlock the phase_barrier if the assertion were removed). Plumbing adaptive_workers through the FTP pool dispatch is genuinely larger work: replace mark_barrier/done_barrier broadcast with per-worker condvars (mirror of Phase 5.3 on the GIL side), and either dynamically size the per-dispatch phase_barrier or remove it in favour of an alternative sync mechanism. That's the FTP-side equivalent of Phase 5.3 + later cleanup. Deferred to a follow-up. In the meantime the FTP controller state (b176751 in pre-rebase SHAs / 5ef3a61 currently) is still being updated after each collection — same logic as the GIL build via the shared helper — but adaptive_workers does not yet take effect on FTP. The header comment on _PyGC_GetParallelWorkers documents the path to enable it. Verified: FTP build, all 5 parallel-GC test files pass (177 tests, 7 skips). The 3-file regression from the previous build (test_gc_parallel, test_gc_ft_parallel, test_gc_parallel_properties — all failing on the assertion) is now resolved.
1 parent dbe0604 commit 9c77265

1 file changed

Lines changed: 15 additions & 14 deletions

File tree

Include/internal/pycore_gc_ft_parallel.h

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -529,28 +529,29 @@ struct _PyGCScanHeapResult {
529529
};
530530

531531
// Get number of parallel GC workers for the next collection.
532-
// Returns 0 if parallel GC is disabled, otherwise the adaptive worker count
533-
// chosen by the random-walk controller (see pycore_gc_random_walk.h).
532+
// Returns 0 if parallel GC is disabled, otherwise the worker count.
534533
//
535-
// The thread pool persists with the maximum num_workers; this returns the
536-
// CURRENT adaptive count, which varies per collection. If the pool isn't
537-
// initialised yet (first call before enable_parallel) the configured
538-
// num_workers is used as a fall-through.
534+
// Currently returns pool->num_workers (the configured maximum), NOT the
535+
// adaptive count. The pool dispatch path
536+
// (_PyGC_ParallelPropagateAliveWithPool and similar) asserts that the
537+
// passed num_workers matches pool->num_workers and that the internal
538+
// phase_barrier (sized num_workers) receives that many participants.
539+
// Returning adaptive_workers here would break both invariants.
540+
//
541+
// Plumbing the adaptive count through requires the pool dispatch refactor:
542+
// per-worker condvars instead of mark_barrier broadcast, dynamically-sized
543+
// (or per-dispatch) phase_barriers. Once that lands, change the body to:
544+
// _PyGCThreadPool *pool = gc->thread_pool;
545+
// return pool ? (int)pool->adaptive_workers : gc->parallel_gc_num_workers;
546+
// and FTP collections will scale per-collection like the GIL build does.
539547
static inline int
540548
_PyGC_GetParallelWorkers(PyInterpreterState *interp)
541549
{
542550
struct _gc_runtime_state *gc = &interp->gc;
543551
if (!gc->parallel_gc_enabled) {
544552
return 0;
545553
}
546-
_PyGCThreadPool *pool = gc->thread_pool;
547-
if (pool != NULL) {
548-
// Adaptive worker count, updated by _PyGC_RandomWalkUpdate after
549-
// each collection. Guaranteed in [2, pool->num_workers].
550-
return (int)pool->adaptive_workers;
551-
}
552-
// No pool yet (e.g. during initial enable_parallel before pool init):
553-
// fall back to the configured max.
554+
// num_workers is required and validated by gc.enable_parallel()
554555
return gc->parallel_gc_num_workers;
555556
}
556557

0 commit comments

Comments
 (0)