Repository navigation
Tier two memory and reference "leaks" #114695
Description
Activity
Thanks for the report, I'll take a look tomorrow!
Reacted by Kirill Podoprigora, sunmy2019 and An LongSo, these aren't related to the JIT, and they aren't "real" reference/memory leaks (since the memory is indeed cleaned up with the interpreter is finalized).
The problem here is tier two in general... since it needs to "warm up" to actually start running, the normal refleak stabilization period of 3 runs isn't enough. It appears we're "leaking" two things: the executors (reference and memory block "leaks") and the arrays of executors managed by code objects (memory block "leaks").
So there are a few different routes we can take here... I'd like to hear others' thoughts (@gvanrossum, and Mark when he's back).
A: Crank up the number of warmups we do for refleak runs
This is probably the "right" thing to do, but it's fragile and could slow our buildbots to a crawl. With local experiments, it appears that using 9 warmups instead of 3 (
-R 9:3) is what's required to pass the whole test suite without leaks. So refleak runs will take roughly twice as long as they do now, and it could get worse.B: Disable the optimizer during refleak runs
Not a horrible option, but it does mean that we have no refleak coverage for all of tier two.
C: Expose a helper function to wipe out all executors
This is similar to how internal caches get around false "refleaks". Unfortunately, this doesn't really work for us the way things are set up now: while we have ways of globally invalidating executors, this doesn't immediately remove them from the bytecode or touch their refcounts. Getting this to work correctly would be tricky, and likely introduce nasty reference cycles between code objects and executors, or between different executors (once we have trace stitching).
It's a heavy lift with a real runtime cost just to appease the (arguably broken) refleak runners... but there's a chance somebody could want this functionality someday anyways. Currently an executor needs to be hit again in order to be removed after invalidation, but I could certainly imagine use cases for immediately purging JIT'ed code that isn't currently running (and may not ever run again).
D: Hack around the problem
Manually tweak the global refcount total when creating and destroying executors, so the initial reference in the code object isn't reflected (other refcount operations on executors will be tracked normally). This isn't pretty, but it works as expected.
However, this doesn't solve the memory leak problem.
diff --git a/Python/optimizer.c b/Python/optimizer.c index 0d04b09fef..c33bd87354 100644 --- a/Python/optimizer.c +++ b/Python/optimizer.c @@ -2,6 +2,7 @@ #include "opcode.h" #include "pycore_interp.h" #include "pycore_bitutils.h" // _Py_popcount32() +#include "pycore_object.h" #include "pycore_opcode_metadata.h" // _PyOpcode_OpName[] #include "pycore_opcode_utils.h" // MAX_REAL_OPCODE #include "pycore_optimizer.h" // _Py_uop_analyze_and_optimize() @@ -1103,6 +1104,9 @@ _Py_ExecutorInit(_PyExecutorObject *executor, _PyBloomFilter *dependency_set) executor->vm_data.bloom.bits[i] = dependency_set->bits[i]; } link_executor(executor); +#ifdef Py_REF_DEBUG + _Py_DecRefTotal(_PyInterpreterState_GET()); +#endif } /* This must be called by executors during dealloc */ @@ -1110,6 +1114,9 @@ void _Py_ExecutorClear(_PyExecutorObject *executor) { unlink_executor(executor); +#ifdef Py_REF_DEBUG + _Py_IncRefTotal(_PyInterpreterState_GET()); +#endif } void
- changed the title
[-]JIT reference leaks[/-][+]Tier two memory and reference "leaks"[/+]on Jan 30, 2024 So IIUC the solution is to have a non-refcounted backreference from an executor to its code object, which is cleared when the code object releases the executor (i.e., removes it from its list of executors).
Yep (we decided offline that that that's a not-too-difficult way to implement option C).
Bug report
Bug description:
CPython was built with
./configure --enable-experimental-jit --with-pydebugFull trace
trace.txt
CPython versions tested on:
CPython main branch
Operating systems tested on:
macOS
Linked PRs
sys._clear_internal_caches#115152