Skip to content

Commit 846e343

Browse files
committed
Remove non-beneficial optimisations, consolidate benchmarks
Remove three optimisations that showed no measurable benefit in realistic benchmarks: - Cleanup workers (cw1+): parallel async cleanup showed no improvement - BRC sharding: added overhead without measurable benefit - Fast decref (Py_BRC_FAST_DECREF): no benefit in realistic workloads Keep parallel GC scanning which provides proven benefits: - ~4-12% throughput improvement on realistic workloads - ~70-75% reduction in STW pause time Consolidate four benchmark files into single gc_perf_benchmark.py: - Focuses on realistic pyperformance-style workloads - Reports best/mean/worst with clear +/- percentages - Optional synthetic stress tests (--include-synthetic) - Markdown output by default, JSON optional Net change: -3000 lines of code
1 parent 80e8aa6 commit 846e343

16 files changed

Lines changed: 1082 additions & 4076 deletions

Include/internal/pycore_brc.h

Lines changed: 2 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -15,42 +15,10 @@ extern "C" {
1515

1616
#ifdef Py_GIL_DISABLED
1717

18-
// Enable sharded BRC for reduced contention on cross-thread decrefs.
19-
// When enabled, each bucket has multiple shards indexed by the decrefing
20-
// thread's ID, reducing mutex contention by a factor of _Py_BRC_NUM_SHARDS.
21-
#define Py_BRC_SHARDED 1
22-
23-
// Enable fast decref path using atomic ADD when object is already queued/merged.
24-
// When disabled, always uses CAS loop for all shared decrefs.
25-
// This optimisation avoids CAS retry loops for the common case.
26-
#define Py_BRC_FAST_DECREF 1
27-
2818
// Prime number to avoid correlations with memory addresses.
2919
#define _Py_BRC_NUM_BUCKETS 257
3020

31-
#ifdef Py_BRC_SHARDED
32-
// Prime number for shard count to avoid correlations with thread IDs.
33-
#define _Py_BRC_NUM_SHARDS 11
34-
35-
// Per-shard mutex within a bucket
36-
struct _brc_shard {
37-
PyMutex mutex;
38-
};
39-
40-
// Hash table bucket with sharding
41-
struct _brc_bucket {
42-
// Array of shards, each with its own mutex.
43-
// Shard index = decrefing_thread_id % _Py_BRC_NUM_SHARDS
44-
struct _brc_shard shards[_Py_BRC_NUM_SHARDS];
45-
46-
// Linked list of _PyThreadStateImpl objects hashed to this bucket.
47-
// Protected by ALL shard mutexes (must hold all for modification).
48-
struct llist_node root;
49-
};
50-
51-
#else /* !Py_BRC_SHARDED */
52-
53-
// Hash table bucket (original non-sharded design)
21+
// Hash table bucket
5422
struct _brc_bucket {
5523
// Mutex protects both the bucket and thread state queues in this bucket.
5624
PyMutex mutex;
@@ -59,40 +27,14 @@ struct _brc_bucket {
5927
struct llist_node root;
6028
};
6129

62-
#endif /* Py_BRC_SHARDED */
63-
6430
// Per-interpreter biased reference counting state
6531
struct _brc_state {
6632
// Hash table of thread states by thread-id. Thread states within a bucket
6733
// are chained using a doubly-linked list.
6834
struct _brc_bucket table[_Py_BRC_NUM_BUCKETS];
6935
};
7036

71-
#ifdef Py_BRC_SHARDED
72-
73-
// Per-thread biased reference counting state (sharded)
74-
struct _brc_thread_state {
75-
// Linked-list of thread states per hash bucket
76-
struct llist_node bucket_node;
77-
78-
// Thread-id as determined by _PyThread_Id()
79-
uintptr_t tid;
80-
81-
// Objects with refcounts to be merged, one queue per shard.
82-
// Queue[i] is protected by bucket->shards[i].mutex
83-
_PyObjectStack queues[_Py_BRC_NUM_SHARDS];
84-
85-
// Bitmap tracking which queues are non-empty.
86-
// Bit i set iff queues[i] is non-empty.
87-
uint16_t non_empty_shards;
88-
89-
// Local stack of objects to be merged (not accessed by other threads)
90-
_PyObjectStack local_objects_to_merge;
91-
};
92-
93-
#else /* !Py_BRC_SHARDED */
94-
95-
// Per-thread biased reference counting state (original)
37+
// Per-thread biased reference counting state
9638
struct _brc_thread_state {
9739
// Linked-list of thread states per hash bucket
9840
struct llist_node bucket_node;
@@ -107,8 +49,6 @@ struct _brc_thread_state {
10749
_PyObjectStack local_objects_to_merge;
10850
};
10951

110-
#endif /* Py_BRC_SHARDED */
111-
11252
// Initialize/finalize the per-thread biased reference counting state
11353
void _Py_brc_init_thread(PyThreadState *tstate);
11454
void _Py_brc_remove_thread(PyThreadState *tstate);

Include/internal/pycore_gc_ft_parallel.h

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@ typedef enum {
140140
_PyGC_WORK_UPDATE_REFS, // Initialize gc_refs on heap (deduce_unreachable phase 1)
141141
_PyGC_WORK_MARK_HEAP, // Find roots and mark reachable (deduce_unreachable phase 2)
142142
_PyGC_WORK_SCAN_HEAP, // Collect unreachable objects (deduce_unreachable phase 3)
143-
_PyGC_WORK_ASYNC_CLEANUP, // Single-threaded concurrent cleanup (runs in background)
144143
_PyGC_WORK_SHUTDOWN // Shutdown workers
145144
} _PyGCWorkType;
146145

@@ -193,13 +192,6 @@ typedef struct {
193192
// Shared worker state (deques, etc.)
194193
_PyGCWorkerState *workers; // Now uses the full typedef
195194

196-
// For ASYNC_CLEANUP work (parallel concurrent background cleanup)
197-
PyObject **async_cleanup_objects; // Array of objects to clean up (owned)
198-
Py_ssize_t async_cleanup_count; // Number of objects
199-
struct _gc_runtime_state *gcstate; // GC state for clearing collecting flag
200-
int async_cleanup_workers; // Number of workers participating in cleanup (1..N)
201-
_Atomic(int) workers_remaining; // Counter for completion tracking (last to 0 clears collecting)
202-
203195
// Result
204196
volatile int error_flag; // Set if any worker encounters an error
205197

@@ -235,7 +227,6 @@ typedef struct _PyGCThreadPool {
235227
_PyGCBarrier mark_barrier; // Workers wait here for work
236228
_PyGCBarrier done_barrier; // All workers wait here when done
237229
_PyGCBarrier phase_barrier; // For multi-phase operations
238-
_PyGCBarrier async_done_barrier; // For concurrent work (N-1 workers, excludes main thread)
239230

240231
// Worker control
241232
volatile int shutdown; // 1 = pool is shutting down
@@ -345,17 +336,6 @@ PyAPI_FUNC(int) _PyGC_ParallelScanHeapWithPool(
345336
_PyGCFTParState *state,
346337
struct _PyGCScanHeapResult *result);
347338

348-
// Start concurrent cleanup in background worker (single-threaded).
349-
// Objects array is copied by the function (caller frees original).
350-
// The background worker will set gcstate->collecting = 0 when done.
351-
// Pool must exist (guaranteed if parallel_gc_enabled is true).
352-
// num_workers: how many workers to use for cleanup (1..pool_size, already clamped by caller)
353-
PyAPI_FUNC(void) _PyGC_StartAsyncCleanup(
354-
PyInterpreterState *interp,
355-
PyObject **objects,
356-
Py_ssize_t count,
357-
int num_workers);
358-
359339
//-----------------------------------------------------------------------------
360340
// Ad-hoc thread versions (spawn threads per-collection instead of using pool)
361341
// These duplicate the pool-based functions but create threads on each call.

Include/internal/pycore_interp_structs.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,6 @@ struct _gc_runtime_state {
263263
/* Parallel GC configuration for FTP */
264264
int parallel_gc_enabled; /* 1 = enabled, 0 = disabled (default) */
265265
int parallel_gc_num_workers; /* Number of workers, 0 = auto (based on CPU count) */
266-
int cleanup_workers; /* Requested max cleanup workers (0=serial, N=parallel) */
267-
int cleanup_workers_active; /* Actual cleanup workers for current cycle (set after STW) */
268266
struct _PyGCThreadPool *thread_pool; /* Persistent thread pool for parallel GC */
269267

270268
/* Phase timing for benchmarking (nanoseconds) */

0 commit comments

Comments
 (0)