From e6b7b2cc1799bb0ea4fd170dae51daace94f0124 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Tue, 4 Oct 2022 16:55:52 -0700 Subject: [PATCH 1/6] Run the GC only on eval breaker --- Include/internal/pycore_gc.h | 2 ++ Include/internal/pycore_interp.h | 2 ++ Modules/gcmodule.c | 21 ++++++++++++++++++--- Modules/signalmodule.c | 13 +++++++++++++ Python/ceval_gil.c | 17 ++++++++++++----- 5 files changed, 47 insertions(+), 8 deletions(-) diff --git a/Include/internal/pycore_gc.h b/Include/internal/pycore_gc.h index bfab0adfffc9ff..b3abe2030a03da 100644 --- a/Include/internal/pycore_gc.h +++ b/Include/internal/pycore_gc.h @@ -202,6 +202,8 @@ extern void _PyList_ClearFreeList(PyInterpreterState *interp); extern void _PyDict_ClearFreeList(PyInterpreterState *interp); extern void _PyAsyncGen_ClearFreeLists(PyInterpreterState *interp); extern void _PyContext_ClearFreeList(PyInterpreterState *interp); +extern void _Py_ScheduleGC(PyInterpreterState *interp); +extern void _Py_RunGC(PyThreadState *tstate); #ifdef __cplusplus } diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 8cecd5ab3e541e..c11e897305d42b 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -49,6 +49,8 @@ struct _ceval_state { _Py_atomic_int eval_breaker; /* Request for dropping the GIL */ _Py_atomic_int gil_drop_request; + /* The GC is ready to be executed */ + _Py_atomic_int gc_scheduled; struct _pending_calls pending; }; diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index 97cb6e6e1efb1f..e8689dc83dd075 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -2252,6 +2252,14 @@ PyObject_IS_GC(PyObject *obj) return _PyObject_IS_GC(obj); } +void +_Py_ScheduleGC(PyInterpreterState *interp) +{ + struct _ceval_state *ceval = &interp->ceval; + _Py_atomic_store_relaxed(&ceval->gc_scheduled, 1); + _Py_atomic_store_relaxed(&ceval->eval_breaker, 1); +} + void _PyObject_GC_Link(PyObject *op) { @@ -2269,12 +2277,19 @@ _PyObject_GC_Link(PyObject *op) !gcstate->collecting && !_PyErr_Occurred(tstate)) { - gcstate->collecting = 1; - gc_collect_generations(tstate); - gcstate->collecting = 0; + _Py_ScheduleGC(tstate->interp); } } +void +_Py_RunGC(PyThreadState *tstate) +{ + GCState *gcstate = &tstate->interp->gc; + gcstate->collecting = 1; + gc_collect_generations(tstate); + gcstate->collecting = 0; +} + static PyObject * gc_alloc(size_t basicsize, size_t presize) { diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index 0f30b4da036313..b85d6d19e8cd05 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -1798,6 +1798,19 @@ int PyErr_CheckSignals(void) { PyThreadState *tstate = _PyThreadState_GET(); + + /* Opportunistically check if the GC is scheduled to run and run it + if we have a request. This is done here because native code needs + to call this API if is going to run for some time without executing + Python code to ensure signals are handled. Checking for the GC here + allows long running native code to clean cycles created using the C-API + even if it doesn't run the evaluation loop */ + struct _ceval_state *interp_ceval_state = &tstate->interp->ceval; + if (_Py_atomic_load_relaxed(&interp_ceval_state->gc_scheduled)) { + _Py_atomic_store_relaxed(&interp_ceval_state->gc_scheduled, 0); + _Py_RunGC(tstate); + } + if (!_Py_ThreadCanHandleSignals(tstate->interp)) { return 0; } diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index fd737b5738e889..6ee4360d95cec4 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -5,6 +5,7 @@ #include "pycore_pyerrors.h" // _PyErr_Fetch() #include "pycore_pylifecycle.h" // _PyErr_Print() #include "pycore_initconfig.h" // _PyStatus_OK() +#include "pycore_interp.h" // _Py_RunGC() #include "pycore_pymem.h" // _PyMem_IsPtrFreed() /* @@ -938,6 +939,13 @@ _Py_HandlePending(PyThreadState *tstate) { _PyRuntimeState * const runtime = &_PyRuntime; struct _ceval_runtime_state *ceval = &runtime->ceval; + struct _ceval_state *interp_ceval_state = &tstate->interp->ceval; + + /* GC scheduled to run */ + if (_Py_atomic_load_relaxed_int32(&interp_ceval_state->gc_scheduled)) { + _Py_atomic_store_relaxed(&interp_ceval_state->gc_scheduled, 0); + _Py_RunGC(tstate); + } /* Pending signals */ if (_Py_atomic_load_relaxed_int32(&ceval->signals_pending)) { @@ -947,20 +955,19 @@ _Py_HandlePending(PyThreadState *tstate) } /* Pending calls */ - struct _ceval_state *ceval2 = &tstate->interp->ceval; - if (_Py_atomic_load_relaxed_int32(&ceval2->pending.calls_to_do)) { + if (_Py_atomic_load_relaxed_int32(&interp_ceval_state->pending.calls_to_do)) { if (make_pending_calls(tstate->interp) != 0) { return -1; } } /* GIL drop request */ - if (_Py_atomic_load_relaxed_int32(&ceval2->gil_drop_request)) { + if (_Py_atomic_load_relaxed_int32(&interp_ceval_state->gil_drop_request)) { /* Give another thread a chance */ if (_PyThreadState_Swap(&runtime->gilstate, NULL) != tstate) { Py_FatalError("tstate mix-up"); } - drop_gil(ceval, ceval2, tstate); + drop_gil(ceval, interp_ceval_state, tstate); /* Other threads may run now */ @@ -989,7 +996,7 @@ _Py_HandlePending(PyThreadState *tstate) // value. It prevents to interrupt the eval loop at every instruction if // the current Python thread cannot handle signals (if // _Py_ThreadCanHandleSignals() is false). - COMPUTE_EVAL_BREAKER(tstate->interp, ceval, ceval2); + COMPUTE_EVAL_BREAKER(tstate->interp, ceval, interp_ceval_state); #endif return 0; From 52f3855362ae036d3d3295072e2826020ec76598 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Wed, 5 Oct 2022 11:38:08 -0700 Subject: [PATCH 2/6] Add news entry Signed-off-by: Pablo Galindo --- Doc/whatsnew/3.12.rst | 7 +++++++ .../2022-10-05-11-37-15.gh-issue-97922.Zu9Bge.rst | 5 +++++ Modules/gcmodule.c | 10 ++++++++-- Python/ceval_gil.c | 13 ++++++++----- 4 files changed, 28 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-10-05-11-37-15.gh-issue-97922.Zu9Bge.rst diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 405de11e716b44..faac75d7d1699e 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -93,6 +93,13 @@ Other Language Changes when parsing source code containing null bytes. (Contributed by Pablo Galindo in :gh:`96670`.) +* The Garbage Collector now runs only on the eval breaker mechanism of the + Python bytecode evaluation loop instead on object allocations. The GC can + also run when :c:func:`PyErr_CheckSignals` is called so C extensions that + need to run for a long time without executing any Python code also have a + chance to execute the GC periodically. (Contributed by Pablo Galindo in + :gh:`97922`.) + New Modules =========== diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-10-05-11-37-15.gh-issue-97922.Zu9Bge.rst b/Misc/NEWS.d/next/Core and Builtins/2022-10-05-11-37-15.gh-issue-97922.Zu9Bge.rst new file mode 100644 index 00000000000000..bf78709362f464 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-10-05-11-37-15.gh-issue-97922.Zu9Bge.rst @@ -0,0 +1,5 @@ +The Garbage Collector now runs only on the eval breaker mechanism of the +Python bytecode evaluation loop instead on object allocations. The GC can +also run when :c:func:`PyErr_CheckSignals` is called so C extensions that +need to run for a long time without executing any Python code also have a +chance to execute the GC periodically. diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index e8689dc83dd075..75832e9dd3da63 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -2255,9 +2255,15 @@ PyObject_IS_GC(PyObject *obj) void _Py_ScheduleGC(PyInterpreterState *interp) { + GCState *gcstate = &interp->gc; + if (gcstate->collecting == 1) { + return; + } struct _ceval_state *ceval = &interp->ceval; - _Py_atomic_store_relaxed(&ceval->gc_scheduled, 1); - _Py_atomic_store_relaxed(&ceval->eval_breaker, 1); + if (!_Py_atomic_load_relaxed(&ceval->gc_scheduled)) { + _Py_atomic_store_relaxed(&ceval->gc_scheduled, 1); + _Py_atomic_store_relaxed(&ceval->eval_breaker, 1); + } } void diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index 6ee4360d95cec4..9abbc97d4a839d 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -70,7 +70,8 @@ COMPUTE_EVAL_BREAKER(PyInterpreterState *interp, && _Py_ThreadCanHandleSignals(interp)) | (_Py_atomic_load_relaxed_int32(&ceval2->pending.calls_to_do) && _Py_ThreadCanHandlePendingCalls()) - | ceval2->pending.async_exc); + | ceval2->pending.async_exc + | _Py_atomic_load_relaxed_int32(&ceval2->gc_scheduled)); } @@ -945,6 +946,7 @@ _Py_HandlePending(PyThreadState *tstate) if (_Py_atomic_load_relaxed_int32(&interp_ceval_state->gc_scheduled)) { _Py_atomic_store_relaxed(&interp_ceval_state->gc_scheduled, 0); _Py_RunGC(tstate); + COMPUTE_EVAL_BREAKER(tstate->interp, ceval, interp_ceval_state); } /* Pending signals */ @@ -988,16 +990,17 @@ _Py_HandlePending(PyThreadState *tstate) return -1; } -#ifdef MS_WINDOWS - // bpo-42296: On Windows, _PyEval_SignalReceived() can be called in a - // different thread than the Python thread, in which case + + // It is possible that some of the conditions that trigger the eval breaker + // are called in a different thread than the Python thread. An example of + // this is bpo-42296: On Windows, _PyEval_SignalReceived() can be called in + // a different thread than the Python thread, in which case // _Py_ThreadCanHandleSignals() is wrong. Recompute eval_breaker in the // current Python thread with the correct _Py_ThreadCanHandleSignals() // value. It prevents to interrupt the eval loop at every instruction if // the current Python thread cannot handle signals (if // _Py_ThreadCanHandleSignals() is false). COMPUTE_EVAL_BREAKER(tstate->interp, ceval, interp_ceval_state); -#endif return 0; } From 0bbc48812ee54a967464d7278f69a59f48e15f20 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Fri, 7 Oct 2022 16:12:08 -0700 Subject: [PATCH 3/6] fixup! Add news entry --- Lib/test/test_frame.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_frame.py b/Lib/test/test_frame.py index 4b86a60d2f4c36..4b5bb7f94ac469 100644 --- a/Lib/test/test_frame.py +++ b/Lib/test/test_frame.py @@ -277,7 +277,7 @@ def callback(phase, info): frame! """ nonlocal sneaky_frame_object - sneaky_frame_object = sys._getframe().f_back + sneaky_frame_object = sys._getframe().f_back.f_back # We're done here: gc.callbacks.remove(callback) From 28426c2f652ad02016d70ec72dc6f84b2bc21e91 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Salgado Date: Fri, 7 Oct 2022 16:12:25 -0700 Subject: [PATCH 4/6] Update Python/ceval_gil.c Co-authored-by: Brandt Bucher --- Python/ceval_gil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index 9abbc97d4a839d..20628a9fbd8fb8 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -945,8 +945,8 @@ _Py_HandlePending(PyThreadState *tstate) /* GC scheduled to run */ if (_Py_atomic_load_relaxed_int32(&interp_ceval_state->gc_scheduled)) { _Py_atomic_store_relaxed(&interp_ceval_state->gc_scheduled, 0); - _Py_RunGC(tstate); COMPUTE_EVAL_BREAKER(tstate->interp, ceval, interp_ceval_state); + _Py_RunGC(tstate); } /* Pending signals */ From 9a1592a24ce1adbeeaa97bc921f8e221cad174ca Mon Sep 17 00:00:00 2001 From: Pablo Galindo Date: Fri, 7 Oct 2022 16:15:10 -0700 Subject: [PATCH 5/6] fixup! fixup! Add news entry --- Python/ceval_gil.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index 9abbc97d4a839d..594afd4670c27a 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -942,13 +942,6 @@ _Py_HandlePending(PyThreadState *tstate) struct _ceval_runtime_state *ceval = &runtime->ceval; struct _ceval_state *interp_ceval_state = &tstate->interp->ceval; - /* GC scheduled to run */ - if (_Py_atomic_load_relaxed_int32(&interp_ceval_state->gc_scheduled)) { - _Py_atomic_store_relaxed(&interp_ceval_state->gc_scheduled, 0); - _Py_RunGC(tstate); - COMPUTE_EVAL_BREAKER(tstate->interp, ceval, interp_ceval_state); - } - /* Pending signals */ if (_Py_atomic_load_relaxed_int32(&ceval->signals_pending)) { if (handle_signals(tstate) != 0) { @@ -963,6 +956,13 @@ _Py_HandlePending(PyThreadState *tstate) } } + /* GC scheduled to run */ + if (_Py_atomic_load_relaxed_int32(&interp_ceval_state->gc_scheduled)) { + _Py_atomic_store_relaxed(&interp_ceval_state->gc_scheduled, 0); + _Py_RunGC(tstate); + COMPUTE_EVAL_BREAKER(tstate->interp, ceval, interp_ceval_state); + } + /* GIL drop request */ if (_Py_atomic_load_relaxed_int32(&interp_ceval_state->gil_drop_request)) { /* Give another thread a chance */ From 0411480e6f9d59c9b8e29a3044064b7dbf92c621 Mon Sep 17 00:00:00 2001 From: Pablo Galindo Salgado Date: Fri, 7 Oct 2022 17:07:50 -0700 Subject: [PATCH 6/6] Update Python/ceval_gil.c Co-authored-by: Brandt Bucher --- Python/ceval_gil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index 594afd4670c27a..9b9d7dc1d1af1e 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -959,8 +959,8 @@ _Py_HandlePending(PyThreadState *tstate) /* GC scheduled to run */ if (_Py_atomic_load_relaxed_int32(&interp_ceval_state->gc_scheduled)) { _Py_atomic_store_relaxed(&interp_ceval_state->gc_scheduled, 0); - _Py_RunGC(tstate); COMPUTE_EVAL_BREAKER(tstate->interp, ceval, interp_ceval_state); + _Py_RunGC(tstate); } /* GIL drop request */