From 83eb827247dd28b13fd816936c74c162e9f52a2d Mon Sep 17 00:00:00 2001 From: Pablo Galindo Salgado Date: Sat, 8 Oct 2022 07:57:09 -0700 Subject: gh-97922: Run the GC only on eval breaker (#97920) --- Doc/whatsnew/3.12.rst | 7 +++++ Include/internal/pycore_gc.h | 2 ++ Include/internal/pycore_interp.h | 2 ++ Lib/test/test_frame.py | 2 +- .../2022-10-05-11-37-15.gh-issue-97922.Zu9Bge.rst | 5 ++++ Modules/gcmodule.c | 27 ++++++++++++++++--- Modules/signalmodule.c | 13 ++++++++++ Python/ceval_gil.c | 30 ++++++++++++++-------- 8 files changed, 74 insertions(+), 14 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 f873974..341e851 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/Include/internal/pycore_gc.h b/Include/internal/pycore_gc.h index bfab0ad..b3abe20 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 8cecd5a..c11e897 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/Lib/test/test_frame.py b/Lib/test/test_frame.py index 4b86a60..4b5bb7f 100644 --- a/Lib/test/test_frame.py +++ b/Lib/test/test_frame.py @@ -277,7 +277,7 @@ class TestIncompleteFrameAreInvisible(unittest.TestCase): 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) 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 0000000..bf78709 --- /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 97cb6e6..75832e9 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -2253,6 +2253,20 @@ 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; + 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 _PyObject_GC_Link(PyObject *op) { PyGC_Head *g = AS_GC(op); @@ -2269,12 +2283,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 0f30b4d..b85d6d1 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 fd737b5..9b9d7dc 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() /* @@ -69,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)); } @@ -938,6 +940,7 @@ _Py_HandlePending(PyThreadState *tstate) { _PyRuntimeState * const runtime = &_PyRuntime; struct _ceval_runtime_state *ceval = &runtime->ceval; + struct _ceval_state *interp_ceval_state = &tstate->interp->ceval; /* Pending signals */ if (_Py_atomic_load_relaxed_int32(&ceval->signals_pending)) { @@ -947,20 +950,26 @@ _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; } } + /* 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); + COMPUTE_EVAL_BREAKER(tstate->interp, ceval, interp_ceval_state); + _Py_RunGC(tstate); + } + /* 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 */ @@ -981,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, ceval2); -#endif + COMPUTE_EVAL_BREAKER(tstate->interp, ceval, interp_ceval_state); return 0; } -- cgit v0.12