gh-155725: Remove PyGILState_Ensure usage from tracemalloc - #156409
gh-155725: Remove PyGILState_Ensure usage from tracemalloc#156409kumaraditya303 wants to merge 4 commits into
Conversation
tracemalloc no longer acquires the GIL nor creates a temporary thread state when tracing memory allocations. A thread with no attached thread state now records the trace with the "<unknown>" traceback instead of attaching a thread state to capture the Python traceback. Threads without a thread state used to pay for a GIL acquisition plus a full thread state creation and destruction on every traced raw allocation, only to record an empty traceback anyway.
Documentation build overview
|
…e attached Store traceback frame filenames as interned NUL terminated UTF-8 strings instead of Python str objects, so that capturing a traceback no longer uses or modifies Python objects. Threads without an attached thread state now capture their Python traceback by walking the frames of the thread state most recently bound to the thread; only threads which never had a thread state record the traceback as "<unknown>".
ZeroIntensity
left a comment
ZeroIntensity
left a comment
There was a problem hiding this comment.
Thanks, this is a much better approach.
| filename_obj = PyUnicode_DecodeUTF8(filename, | ||
| (Py_ssize_t)strlen(filename), | ||
| "surrogatepass"); |
There was a problem hiding this comment.
This can call arbitrary Python code through the codec error handler, meaning this can encounter a re-entrancy deadlock with the tables lock. I'm okay with not fixing that since there's no reason an error handler should be invoking tracemalloc, but it's probably worth adding a comment.
There was a problem hiding this comment.
tracemalloc checks for re-entrancy before acquring the tables lock, so this shoudn't be a issue unless I am missing something.
There was a problem hiding this comment.
Yeah, it's not an issue for the allocator functions, but it would be an issue if the Python code called tracemalloc.clear_traces or something like that, which access the tables lock unconditionally.
| if (_PyInterpreterGuard_TryAcquire(_PyInterpreterState_Main(), | ||
| &guard) < 0) { | ||
| return tracemalloc_empty_traceback; | ||
| } |
There was a problem hiding this comment.
_PyInterpreterGuard_TryAcquire isn't thread safe in this manner, because the interpreter state can be deleted before the guard has been acquired. It's technically not an issue here because the main interpreter is statically allocated, but relying on that will make refactoring harder later.
It would be nicer to use the interpreter ID instead (the main interpreter ID is always zero), which has a thread-safe lookup.
There was a problem hiding this comment.
Looking up by interpreter id would require holding the runtime lock which I am trying to avoid here. FWIW main interpreter being statically allocated is relied upon in a lot of code anyways so avoiding that isn't very useful here.
There was a problem hiding this comment.
Why is the runtime lock a bad thing?
2 participants
tracemalloc no longer acquires the GIL nor creates a temporary thread state when tracing memory allocations.
tracemallocraw-domain allocator hook violatesPYMEM_DOMAIN_RAWcontract #155725