gh-112529: Use GC heaps for GC allocations in free-threaded builds #114157
Conversation
The free-threaded build's garbage collector implementation will need to find GC objects by traversing mimalloc heaps. This hooks up the allocation calls with the correct heaps by using a thread-local "current_obj_heap" variable.
There was a problem hiding this comment.
LGTM
_PyObject_MallocWithType and _PyObject_ReallocWithType looks very redundent.
We can extract them as something like this, but I am not sure whether it is worth doing this or if it will cause any performance issues. If possible, extracting them as macros will be beneficial to understanding the code.
# psudo
switch_heap_per_type(tp);
malloc or realloc
switch_heap(_Py_MIMALLOC_HEAP_OBJECT);
There was a problem hiding this comment.
Still not a big fan of the smuggling through thread state, but I don't have a better solution other than extending the existing PyMemAllocator API in some way or another.
I'd love it if we could use ctx here but that's not how ctx works and if someone intercepts the allocator we won't get our ctx. I wonder if it'd be worth adding a GC domain that would take an extra void* (which would avoid the problem with "what do we do if we need another heap in the future" problem). Pinging @vstinner who I believe wasn't around the last time this was discussed to see if he might have further opinions.
| #endif | ||
| void *mem = PyObject_Malloc(size); | ||
| #ifdef Py_GIL_DISABLED | ||
| m->current_object_heap = &m->heaps[_Py_MIMALLOC_HEAP_OBJECT]; |
There was a problem hiding this comment.
It seems like this shouldn't be strictly necessary? We should always be going through this API to allocate otherwise we might get the wrong heap. We could keep this NULL in debug builds and assert that it's not NULL when we hit the allocator.
There was a problem hiding this comment.
You can call PyObject_Malloc() directly -- that doesn't go through this API. Only calls that might allocate GC objects need to go through this. So we have to restore current_object_heap to the non-GC object heap.
You could structure this a bit differently and have PyObject_Malloc() also go through this sort of thing, but I think that would end up being more complicated.
There was a problem hiding this comment.
But if you call PyObject_Malloc directly aren't things now broken with the pre-header? It seems that Py_TPFLAGS_MANAGED_DICT and Py_TPFLAGS_MANAGED_WEAKREF are both documented as being available to extension objects which would be the primary callers of PyObject_Malloc. Maybe we at least need to document that if you're using these flags you need to call PyType_GenericAlloc in Doc/c-api/typeobj.rst.
I suppose before this you'd be probably broken if you called PyObject_Malloc, but it would have crashed and burned pretty obviously, and you could figure out to call _PyType_PreHeaderSize. Now it seems critical that you call PyType_GenericAlloc or _PyObject_MallocWithType or you'll get really subtle crashes in multi-threaded scenarios.
There was a problem hiding this comment.
You can call PyObject_Malloc() for non-GC objects, but not for GC-objects. GC objects have had "pre-headers" (PyGC_Head) before Py_TPFLAGS_MANAGED_DICT was introduced.
These restrictions have been in place since Python 2.2a3:
https://github.com/python/cpython/blob/72abb8c5d487ead9eb115fec8132ccef5ba189e5/Misc/HISTORY#L24391-L24398
There was a problem hiding this comment.
Ahh, true, I hadn't considered that the pre-header implies GC, that makes sense!
|
@colesbury Please let me know if the PR is ready to be merged. cc @DinoV |
|
Yes, it's ready to be merged |
The free-threaded build's garbage collector implementation will need to find GC objects by traversing mimalloc heaps. This hooks up the allocation calls with the correct heaps by using a thread-local "current_obj_heap" variable. There is no effect on the default (with GIL) build.
--disable-gilbuilds #112529