FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-146527, gh-155811: Backport out-of-process GC statistics to 3.14 · Kronuz/cpython@3453253 · GitHub

Repository navigation

Commit 3453253

Browse files
committed
pythongh-146527, pythongh-155811: Backport out-of-process GC statistics to 3.14
1 parent 8e6e75d commit 3453253

12 files changed

Lines changed: 1823 additions & 55 deletions

File tree

‎Include/internal/pycore_debug_offsets.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ typedef struct _Py_DebugOffsets {
233233
uint64_t debugger_script_path;
234234
uint64_t debugger_script_path_size;
235235
} debugger_support;
236+
236237
} _Py_DebugOffsets;
237238

238239

‎Include/internal/pycore_interp_structs.h‎

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,27 +178,65 @@ struct gc_generation {
178178
generations */
179179
};
180180

181-
struct gc_collection_stats {
182-
/* number of collected objects */
181+
/* Stock 3.14's three counters. The generation_counters[] array below keeps
182+
this type and therefore its original 72 bytes at its original offset, so
183+
that _gc_runtime_state's layout is byte-identical to an unpatched build.
184+
It is still maintained, so an extension compiled against stock headers
185+
that reads it keeps reading correct values. */
186+
struct gc_generation_counters {
187+
/* total number of collections */
188+
Py_ssize_t collections;
189+
/* total number of collected objects */
183190
Py_ssize_t collected;
184191
/* total number of uncollectable objects (put into gc.garbage) */
185192
Py_ssize_t uncollectable;
186193
};
187194

188195
/* Running stats per generation */
189196
struct gc_generation_stats {
197+
PyTime_t ts_start;
198+
PyTime_t ts_stop;
190199
/* total number of collections */
191200
Py_ssize_t collections;
192201
/* total number of collected objects */
193202
Py_ssize_t collected;
194203
/* total number of uncollectable objects (put into gc.garbage) */
195204
Py_ssize_t uncollectable;
205+
// Total number of objects considered for collection and traversed:
206+
Py_ssize_t candidates;
207+
// Total duration of the collection in seconds:
208+
double duration;
209+
/* heap_size on the start of the collection */
210+
Py_ssize_t heap_size;
211+
};
212+
213+
#ifdef Py_GIL_DISABLED
214+
#define GC_YOUNG_STATS_SIZE 1
215+
#define GC_OLD_STATS_SIZE 1
216+
#else
217+
#define GC_YOUNG_STATS_SIZE 11
218+
#define GC_OLD_STATS_SIZE 3
219+
#endif
220+
struct gc_young_stats_buffer {
221+
struct gc_generation_stats items[GC_YOUNG_STATS_SIZE];
222+
int8_t index;
223+
};
224+
225+
struct gc_old_stats_buffer {
226+
struct gc_generation_stats items[GC_OLD_STATS_SIZE];
227+
int8_t index;
196228
};
197229

198230
/* If we change this, we need to change the default value in the
199231
signature of gc.collect. */
200232
#define NUM_GENERATIONS 3
201233

234+
struct gc_stats {
235+
struct gc_young_stats_buffer young;
236+
struct gc_old_stats_buffer old[2];
237+
uint32_t update_seq;
238+
};
239+
202240
struct _gc_runtime_state {
203241
/* List of objects that still need to be cleaned up, singly linked
204242
* via their gc headers' gc_prev pointers. */
@@ -219,7 +257,7 @@ struct _gc_runtime_state {
219257

220258
/* a permanent generation which won't be collected */
221259
struct gc_generation permanent_generation;
222-
struct gc_generation_stats generation_stats[NUM_GENERATIONS];
260+
struct gc_generation_counters generation_counters[NUM_GENERATIONS];
223261
/* true if we are currently running the collector */
224262
int collecting;
225263
/* list of uncollectable objects */
@@ -230,8 +268,15 @@ struct _gc_runtime_state {
230268
/* The number of live objects. */
231269
Py_ssize_t heap_size;
232270

233-
/* dummy members to preserve other offsets */
234-
Py_ssize_t dummy1; /* was work_to_do */
271+
/* Ring buffers of per-collection statistics, published to external
272+
readers through _Py_DebugOffsets. This occupies the slot 3.14
273+
reserved as dummy1 when it reverted the incremental collector, so it
274+
costs no layout change: a pointer and a Py_ssize_t are both eight
275+
bytes with eight-byte alignment here. */
276+
/* Upstream 3.15 spells this PyInterpreterState.gc.generation_stats
277+
and reaches it the same way; see the Provenance note in
278+
patches/gc-stats.patch for the one placement difference. */
279+
struct gc_stats *generation_stats; /* was dummy1, was work_to_do */
235280
int dummy2; /* was visited_space */
236281
int dummy3; /* was phase */
237282

@@ -260,6 +305,13 @@ struct _gc_runtime_state {
260305

261306
/* Mutex held for gc_should_collect_mem_usage(). */
262307
PyMutex mutex;
308+
/* Serializes gc.get_stats() (gc_get_stats_impl, the reader) against
309+
gc_collect_main()'s stats writer on the free-threaded build so a
310+
reader never observes a half-updated record (gh-151646). Placed
311+
after the existing mutex so it lands in this struct's trailing
312+
padding: no member moves and _gc_runtime_state stays byte-identical
313+
to a stock build. */
314+
PyMutex stats_mutex;
263315
#else
264316
PyGC_Head *generation0;
265317
#endif
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
# Reproduction for the gc.get_stats() data race under free-threading.
2+
#
3+
# CPython issue gh-151646: in free-threading builds (``--disable-gil``) the
4+
# concurrent collector and ``gc.get_stats()`` touch the same per-generation
5+
# statistics struct (``gcstate->generation_stats``) without any synchronisation:
6+
#
7+
# * READER -- ``gc_get_stats_impl()`` in ``Modules/gcmodule.c`` copies the
8+
# ``struct gc_generation_stats`` for each generation (``collections``,
9+
# ``collected``, ``uncollectable``, ``candidates``, ``duration``) with a
10+
# plain struct assignment and no lock.
11+
#
12+
# * WRITER -- at the end of every collection cycle ``gc_collect_main()`` in
13+
# ``Python/gc_free_threading.c`` mutates that very struct in place
14+
# (``stats->collections++``, ``stats->collected += m``, ...), again with no
15+
# lock.
16+
#
17+
# When one thread is collecting while several others read the stats, the
18+
# unsynchronised read/write to the same memory is a data race. The values are
19+
# only statistics, so the race is benign at the Python level (no crash), which
20+
# is exactly what lets this script double as a regression test: once the fix
21+
# adds proper synchronisation, ThreadSanitizer should stay quiet while the
22+
# script keeps exiting cleanly.
23+
#
24+
# Running this script under a free-threading CPython build compiled with
25+
# ThreadSanitizer (``./configure --disable-gil --with-thread-sanitizer``) is
26+
# expected to print a ``WARNING: ThreadSanitizer: data race`` report whose stack
27+
# traces point at ``gc_get_stats_impl`` (gcmodule.c) and ``gc_collect_main``
28+
# (gc_free_threading.c).
29+
#
30+
# Standalone usage:
31+
#
32+
# ./python Lib/test/test_free_threading/test_gc_get_stats_race.py
33+
#
34+
# or as part of the test suite (only meaningful under a TSAN build):
35+
#
36+
# ./python -m test test_free_threading.test_gc_get_stats_race
37+
38+
import gc
39+
import threading
40+
import unittest
41+
42+
from test.support import threading_helper
43+
44+
45+
# One thread hammers gc.collect() (the writer side); enough reader threads run
46+
# gc.get_stats() concurrently to make the race easy to observe. Readers run
47+
# until the writer is done so the test duration is controlled by the collection
48+
# count rather than by reader loop counts.
49+
NUM_READERS = 4
50+
ITERATIONS = 50
51+
52+
53+
def _stress_get_stats_race(num_readers=NUM_READERS, iterations=ITERATIONS):
54+
"""Race gc.collect() against gc.get_stats()."""
55+
56+
# Synchronise the start so the writer and readers overlap for as long as
57+
# possible, maximising the chance of the read and write landing on the
58+
# statistics struct at the same time.
59+
done = threading.Event()
60+
61+
def collector():
62+
try:
63+
for _ in range(iterations):
64+
# Writer: each full collection updates gcstate->generation_stats.
65+
gc.collect()
66+
finally:
67+
done.set()
68+
69+
def reader():
70+
while not done.is_set():
71+
# Reader: copies the per-generation stats structs with no lock.
72+
gc.get_stats()
73+
74+
threading_helper.run_concurrently([collector] + [reader] * num_readers)
75+
76+
77+
@threading_helper.requires_working_threading()
78+
class TestGCGetStatsRace(unittest.TestCase):
79+
def test_get_stats_collect_race(self):
80+
_stress_get_stats_race()
81+
82+
# The race is benign at the Python level: gc.get_stats() must still
83+
# return well-formed data and the interpreter must not crash.
84+
stats = gc.get_stats()
85+
self.assertEqual(len(stats), 3)
86+
for generation in stats:
87+
self.assertIn("collections", generation)
88+
self.assertIn("collected", generation)
89+
self.assertIn("uncollectable", generation)
90+
91+
92+
if __name__ == "__main__":
93+
# Standalone reproduction: run the race and exit cleanly so the script can
94+
# be reused as a regression check once the fix lands.
95+
print(f"Racing 1 gc.collect() thread against "
96+
f"{NUM_READERS} gc.get_stats() thread(s), {ITERATIONS} collections...")
97+
_stress_get_stats_race()
98+
print("Done (no Python-level crash). "
99+
"Run under a free-threading + TSAN build to observe the data race.")

‎Lib/test/test_gc.py‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -835,11 +835,15 @@ def test_get_stats(self):
835835
self.assertEqual(len(stats), 3)
836836
for st in stats:
837837
self.assertIsInstance(st, dict)
838-
self.assertEqual(set(st),
839-
{"collected", "collections", "uncollectable"})
838+
self.assertEqual(
839+
set(st),
840+
{"collected", "collections", "uncollectable", "candidates", "duration"}
841+
)
840842
self.assertGreaterEqual(st["collected"], 0)
841843
self.assertGreaterEqual(st["collections"], 0)
842844
self.assertGreaterEqual(st["uncollectable"], 0)
845+
self.assertGreaterEqual(st["candidates"], 0)
846+
self.assertGreaterEqual(st["duration"], 0)
843847
# Check that collection counts are incremented correctly
844848
if gc.isenabled():
845849
self.addCleanup(gc.enable)
@@ -850,6 +854,13 @@ def test_get_stats(self):
850854
self.assertEqual(new[0]["collections"], old[0]["collections"] + 1)
851855
self.assertEqual(new[1]["collections"], old[1]["collections"])
852856
self.assertEqual(new[2]["collections"], old[2]["collections"])
857+
self.assertGreater(new[0]["duration"], old[0]["duration"])
858+
self.assertEqual(new[1]["duration"], old[1]["duration"])
859+
self.assertEqual(new[2]["duration"], old[2]["duration"])
860+
for stat in ["collected", "uncollectable", "candidates"]:
861+
self.assertGreaterEqual(new[0][stat], old[0][stat])
862+
self.assertEqual(new[1][stat], old[1][stat])
863+
self.assertEqual(new[2][stat], old[2][stat])
853864
gc.collect(2)
854865
new = gc.get_stats()
855866
self.assertEqual(new[0]["collections"], old[0]["collections"] + 1)

0 commit comments

Comments
 (0)

Back | FazBrowse Home | New Git URL