From 0d2c14a9afe68f9c65f12376964dfeade2680379 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Wed, 21 Oct 2026 15:37:14 -0700 Subject: [PATCH 1/2] Drain each thread's page list during GC Free pages awaiting QSBR are added to each thread's `mimalloc.page_list`. The list is only drained if the owning thread needs new pages, performs an allocation larger than mimalloc's medium-object limit, or the thread exits. This means that we don't free pages if a thread's allocation pattern quiesces or falls into a pattern that can be serviced by existing pages. This can be problematic for threads that allocate a bunch of memory and then settle into the previous pattern. --- Include/internal/pycore_pymem.h | 4 +++ Objects/obmalloc.c | 50 ++++++++++++++++++++++++++++----- Python/gc_free_threading.c | 2 ++ 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h index 05484e847f1195a..8f5e4b64960ea8e 100644 --- a/Include/internal/pycore_pymem.h +++ b/Include/internal/pycore_pymem.h @@ -129,6 +129,10 @@ extern void _PyMem_AbandonDelayed(PyThreadState *tstate); // On interpreter shutdown, frees all delayed free requests. extern void _PyMem_FiniDelayed(PyInterpreterState *interp); +// Free every empty page parked on each thread's page_list that has reached its +// QSBR goal +extern void _PyMem_FreeParkedQSBRPages(PyInterpreterState *interp); + #ifdef __cplusplus } #endif diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 502c3eea6806cfa..7a60252c7ad5430 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -226,16 +226,26 @@ _PyMem_mi_page_reclaimed(mi_page_t *page) #endif } +#ifdef Py_GIL_DISABLED +// Free the empty pages on `target`'s QSBR page list whose QSBR goal has been +// reached. +// +// `cur` must be the calling thread's state: _Py_qsbr_poll() must be called +// with the caller's own QSBR state. The goals themselves are global, so `cur` +// can check pages parked by any thread. +// +// `target` is the thread whose page list is drained. It must be `cur`, or a +// thread that is suspended (e.g., during a stop-the-world pause), because +// _mi_page_free() modifies the heap and segment state of the thread that owns +// the page. static void -_PyMem_mi_heap_collect_qsbr(mi_heap_t *heap) +_PyMem_mi_collect_tstate_page_list(_PyThreadStateImpl *cur, _PyThreadStateImpl *target) { -#ifdef Py_GIL_DISABLED - if (!heap->page_use_qsbr) { + if (!_Py_atomic_load_int(&target->mimalloc.initialized)) { + // The thread may not have called tstate_mimalloc_bind() yet. return; } - - _PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET(); - struct llist_node *head = &tstate->mimalloc.page_list; + struct llist_node *head = &target->mimalloc.page_list; if (llist_empty(head)) { return; } @@ -249,13 +259,26 @@ _PyMem_mi_heap_collect_qsbr(mi_heap_t *heap) continue; } - if (!_Py_qsbr_poll(tstate->qsbr, page->qsbr_goal)) { + if (!_Py_qsbr_poll(cur->qsbr, page->qsbr_goal)) { return; } _PyMem_mi_page_clear_qsbr(page); _mi_page_free(page, mi_page_queue_of(page), false); } +} +#endif + +static void +_PyMem_mi_heap_collect_qsbr(mi_heap_t *heap) +{ +#ifdef Py_GIL_DISABLED + if (!heap->page_use_qsbr) { + return; + } + + _PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET(); + _PyMem_mi_collect_tstate_page_list(tstate, tstate); #endif } @@ -1702,6 +1725,19 @@ _PyMem_FiniDelayed(PyInterpreterState *interp) } } +#ifdef Py_GIL_DISABLED +void +_PyMem_FreeParkedQSBRPages(PyInterpreterState *interp) +{ + _PyThreadStateImpl *tstate = (_PyThreadStateImpl *) _PyThreadState_GET(); + _Py_FOR_EACH_TSTATE_BEGIN(interp, p) { + _PyMem_mi_collect_tstate_page_list(tstate, (_PyThreadStateImpl *) p); + } + _Py_FOR_EACH_TSTATE_END(interp); +} +#endif + + /**************************/ /* the "object" allocator */ /**************************/ diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index e8ed866312c94bf..2dc8d222b06b72b 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -2110,6 +2110,8 @@ gc_collect_internal(PyInterpreterState *interp, struct collection_state *state, process_delayed_frees(interp, state); + _PyMem_FreeParkedQSBRPages(interp); + #ifdef GC_ENABLE_MARK_ALIVE // If gc.freeze() was used, it seems likely that doing this "mark alive" // pass will not be a performance win. Typically the majority of alive From d689f540bbb7bef2f3fdd9f9a1d9ea8a913b806c Mon Sep 17 00:00:00 2001 From: Matt Page Date: Wed, 21 Oct 2026 16:09:51 -0700 Subject: [PATCH 2/2] Add tests for draining QSBR page lists during GC Add _testinternalcapi.get_qsbr_page_list_length(), which counts the pages on all threads' QSBR page lists, and use it to check that gc.collect() frees empty pages parked by the calling thread and by an idle thread. Also check that the GC skips thread states that have not initialized their mimalloc state yet. --- Lib/test/test_free_threading/test_gc.py | 85 +++++++++++++++++++++++++ Modules/_testinternalcapi.c | 23 +++++++ 2 files changed, 108 insertions(+) diff --git a/Lib/test/test_free_threading/test_gc.py b/Lib/test/test_free_threading/test_gc.py index 30282a111345f86..d4d536009876529 100644 --- a/Lib/test/test_free_threading/test_gc.py +++ b/Lib/test/test_free_threading/test_gc.py @@ -10,6 +10,11 @@ from test import support from test.support import threading_helper +try: + import _testinternalcapi +except ImportError: + _testinternalcapi = None + class MyObj: pass @@ -210,5 +215,85 @@ def reader(): pass +def allocate_and_free(): + # Free enough small objects to leave many mimalloc pages empty. Empty + # pages in the object heaps wait on QSBR before they are freed. + junk = [[] for _ in range(100_000)] + del junk + + +@threading_helper.requires_working_threading() +@unittest.skipIf(_testinternalcapi is None, "requires _testinternalcapi") +class TestQSBRPageList(TestCase): + # gh-158972: gc.collect() should free empty pages waiting on QSBR, even + # if the thread that parked them is idle. + + def parked_pages(self): + return _testinternalcapi.get_qsbr_page_list_length() + + def test_gc_collect_frees_parked_pages(self): + gc.collect() + allocate_and_free() + parked = self.parked_pages() + if parked < 10: + self.skipTest(f"only {parked} pages were parked") + gc.collect() + self.assertLess(self.parked_pages(), parked // 4) + + def test_gc_collect_frees_pages_parked_by_idle_thread(self): + done = threading.Lock() + done.acquire() + resume = threading.Lock() + resume.acquire() + + def worker(): + allocate_and_free() + done.release() + # Block without allocating so the worker doesn't free its parked + # pages itself. + resume.acquire() + + t = Thread(target=worker) + t.start() + try: + done.acquire() + parked = self.parked_pages() + if parked < 10: + self.skipTest(f"only {parked} pages were parked") + gc.collect() + self.assertLess(self.parked_pages(), parked // 4) + finally: + resume.release() + t.join() + + def test_gc_collect_while_starting_threads(self): + # A new thread is added to the interpreter before it initializes its + # mimalloc state, so the GC can see thread states without a usable + # page list. + stop = threading.Event() + + def collector(): + while not stop.is_set(): + gc.collect() + + deadline = time.monotonic() + 2.0 + + def starter(): + while time.monotonic() < deadline: + threads = [Thread(target=lambda: None) for _ in range(16)] + with threading_helper.start_threads(threads): + pass + + c = Thread(target=collector) + c.start() + try: + starters = [Thread(target=starter) for _ in range(4)] + with threading_helper.start_threads(starters): + pass + finally: + stop.set() + c.join() + + if __name__ == "__main__": unittest.main() diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 479297a996c2502..279d0f34b72508c 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -2885,6 +2885,28 @@ get_long_lived_total(PyObject *self, PyObject *Py_UNUSED(ignored)) return PyLong_FromInt64(PyInterpreterState_Get()->gc.long_lived_total); } +// Return the total number of mimalloc pages on all threads' QSBR page lists. +static PyObject * +get_qsbr_page_list_length(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyInterpreterState *interp = _PyInterpreterState_GET(); + Py_ssize_t count = 0; + _PyEval_StopTheWorld(interp); + _Py_FOR_EACH_TSTATE_BEGIN(interp, p) { + struct _mimalloc_thread_state *m = &((_PyThreadStateImpl *)p)->mimalloc; + if (!_Py_atomic_load_int(&m->initialized)) { + continue; + } + struct llist_node *node; + llist_for_each(node, &m->page_list) { + count++; + } + } + _Py_FOR_EACH_TSTATE_END(interp); + _PyEval_StartTheWorld(interp); + return PyLong_FromSsize_t(count); +} + #endif static PyObject * @@ -3442,6 +3464,7 @@ static PyMethodDef module_functions[] = { {"get_tlbc", get_tlbc, METH_O, NULL}, {"get_tlbc_id", get_tlbc_id, METH_O, NULL}, {"get_long_lived_total", get_long_lived_total, METH_NOARGS}, + {"get_qsbr_page_list_length", get_qsbr_page_list_length, METH_NOARGS}, #endif #ifdef _Py_TIER2 {"uop_symbols_test", _Py_uop_symbols_test, METH_NOARGS},