Skip to content

gh-112529: Use GC heaps for GC allocations in free-threaded builds #114157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 20, 2024
Merged

gh-112529: Use GC heaps for GC allocations in free-threaded builds #114157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b8183e3
gh-112529: Use GC heaps for GC allocations in free-threaded builds
colesbury Jan 16, 2024
b6c3506
Refactor out setting heap based on type
colesbury Jan 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions Include/internal/pycore_object_alloc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
#ifndef Py_INTERNAL_OBJECT_ALLOC_H
#define Py_INTERNAL_OBJECT_ALLOC_H

#include "pycore_object.h" // _PyType_HasFeature()
#include "pycore_pystate.h" // _PyThreadState_GET()
#include "pycore_tstate.h" // _PyThreadStateImpl

#ifdef __cplusplus
extern "C" {
#endif

#ifndef Py_BUILD_CORE
# error "this header requires Py_BUILD_CORE define"
#endif

#ifdef Py_GIL_DISABLED
static inline mi_heap_t *
_PyObject_GetAllocationHeap(_PyThreadStateImpl *tstate, PyTypeObject *tp)
{
struct _mimalloc_thread_state *m = &tstate->mimalloc;
if (_PyType_HasFeature(tp, Py_TPFLAGS_PREHEADER)) {
return &m->heaps[_Py_MIMALLOC_HEAP_GC_PRE];
}
else if (_PyType_IS_GC(tp)) {
return &m->heaps[_Py_MIMALLOC_HEAP_GC];
}
else {
return &m->heaps[_Py_MIMALLOC_HEAP_OBJECT];
}
}
#endif

// Sets the heap used for PyObject_Malloc(), PyObject_Realloc(), etc. calls in
// Py_GIL_DISABLED builds. We use different heaps depending on if the object
// supports GC and if it has a pre-header. We smuggle the choice of heap
// through the _mimalloc_thread_state. In the default build, this simply
// calls PyObject_Malloc().
static inline void *
_PyObject_MallocWithType(PyTypeObject *tp, size_t size)
{
#ifdef Py_GIL_DISABLED
_PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET();
struct _mimalloc_thread_state *m = &tstate->mimalloc;
m->current_object_heap = _PyObject_GetAllocationHeap(tstate, tp);
#endif
void *mem = PyObject_Malloc(size);
#ifdef Py_GIL_DISABLED
m->current_object_heap = &m->heaps[_Py_MIMALLOC_HEAP_OBJECT];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, true, I hadn't considered that the pre-header implies GC, that makes sense!

#endif
return mem;
}

static inline void *
_PyObject_ReallocWithType(PyTypeObject *tp, void *ptr, size_t size)
{
#ifdef Py_GIL_DISABLED
_PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET();
struct _mimalloc_thread_state *m = &tstate->mimalloc;
m->current_object_heap = _PyObject_GetAllocationHeap(tstate, tp);
#endif
void *mem = PyObject_Realloc(ptr, size);
#ifdef Py_GIL_DISABLED
m->current_object_heap = &m->heaps[_Py_MIMALLOC_HEAP_OBJECT];
#endif
return mem;
}

#ifdef __cplusplus
}
#endif
#endif // !Py_INTERNAL_OBJECT_ALLOC_H
1 change: 1 addition & 0 deletions Makefile.pre.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1852,6 +1852,7 @@ PYTHON_HEADERS= \
$(srcdir)/Include/internal/pycore_moduleobject.h \
$(srcdir)/Include/internal/pycore_namespace.h \
$(srcdir)/Include/internal/pycore_object.h \
$(srcdir)/Include/internal/pycore_object_alloc.h \
$(srcdir)/Include/internal/pycore_object_state.h \
$(srcdir)/Include/internal/pycore_obmalloc.h \
$(srcdir)/Include/internal/pycore_obmalloc_init.h \
Expand Down
3 changes: 2 additions & 1 deletion Objects/typeobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "pycore_modsupport.h" // _PyArg_NoKwnames()
#include "pycore_moduleobject.h" // _PyModule_GetDef()
#include "pycore_object.h" // _PyType_HasFeature()
#include "pycore_object_alloc.h" // _PyObject_MallocWithType()
#include "pycore_pyerrors.h" // _PyErr_Occurred()
#include "pycore_pystate.h" // _PyThreadState_GET()
#include "pycore_symtable.h" // _Py_Mangle()
Expand Down Expand Up @@ -1729,7 +1730,7 @@ _PyType_AllocNoTrack(PyTypeObject *type, Py_ssize_t nitems)
const size_t size = _PyObject_VAR_SIZE(type, nitems+1);

const size_t presize = _PyType_PreHeaderSize(type);
char *alloc = PyObject_Malloc(size + presize);
char *alloc = _PyObject_MallocWithType(type, size + presize);
if (alloc == NULL) {
return PyErr_NoMemory();
}
Expand Down
1 change: 1 addition & 0 deletions PCbuild/pythoncore.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@
<ClInclude Include="..\Include\internal\pycore_moduleobject.h" />
<ClInclude Include="..\Include\internal\pycore_namespace.h" />
<ClInclude Include="..\Include\internal\pycore_object.h" />
<ClInclude Include="..\Include\internal\pycore_object_alloc.h" />
<ClInclude Include="..\Include\internal\pycore_object_state.h" />
<ClInclude Include="..\Include\internal\pycore_obmalloc.h" />
<ClInclude Include="..\Include\internal\pycore_obmalloc_init.h" />
Expand Down
3 changes: 3 additions & 0 deletions PCbuild/pythoncore.vcxproj.filters
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,9 @@
<ClInclude Include="..\Include\internal\pycore_object.h">
<Filter>Include\internal</Filter>
</ClInclude>
<ClInclude Include="..\Include\internal\pycore_object_alloc.h">
<Filter>Include\internal</Filter>
</ClInclude>
<ClInclude Include="..\Include\internal\pycore_object_state.h">
<Filter>Include\internal</Filter>
</ClInclude>
Expand Down
13 changes: 7 additions & 6 deletions Python/gc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "pycore_initconfig.h"
#include "pycore_interp.h" // PyInterpreterState.gc
#include "pycore_object.h"
#include "pycore_object_alloc.h" // _PyObject_MallocWithType()
#include "pycore_pyerrors.h"
#include "pycore_pystate.h" // _PyThreadState_GET()
#include "pycore_weakref.h" // _PyWeakref_ClearRef()
Expand Down Expand Up @@ -1795,14 +1796,14 @@ _Py_RunGC(PyThreadState *tstate)
}

static PyObject *
gc_alloc(size_t basicsize, size_t presize)
gc_alloc(PyTypeObject *tp, size_t basicsize, size_t presize)
{
PyThreadState *tstate = _PyThreadState_GET();
if (basicsize > PY_SSIZE_T_MAX - presize) {
return _PyErr_NoMemory(tstate);
}
size_t size = presize + basicsize;
char *mem = PyObject_Malloc(size);
char *mem = _PyObject_MallocWithType(tp, size);
if (mem == NULL) {
return _PyErr_NoMemory(tstate);
}
Expand All @@ -1817,7 +1818,7 @@ PyObject *
_PyObject_GC_New(PyTypeObject *tp)
{
size_t presize = _PyType_PreHeaderSize(tp);
PyObject *op = gc_alloc(_PyObject_SIZE(tp), presize);
PyObject *op = gc_alloc(tp, _PyObject_SIZE(tp), presize);
if (op == NULL) {
return NULL;
}
Expand All @@ -1836,7 +1837,7 @@ _PyObject_GC_NewVar(PyTypeObject *tp, Py_ssize_t nitems)
}
size_t presize = _PyType_PreHeaderSize(tp);
size_t size = _PyObject_VAR_SIZE(tp, nitems);
op = (PyVarObject *)gc_alloc(size, presize);
op = (PyVarObject *)gc_alloc(tp, size, presize);
if (op == NULL) {
return NULL;
}
Expand All @@ -1848,7 +1849,7 @@ PyObject *
PyUnstable_Object_GC_NewWithExtraData(PyTypeObject *tp, size_t extra_size)
{
size_t presize = _PyType_PreHeaderSize(tp);
PyObject *op = gc_alloc(_PyObject_SIZE(tp) + extra_size, presize);
PyObject *op = gc_alloc(tp, _PyObject_SIZE(tp) + extra_size, presize);
if (op == NULL) {
return NULL;
}
Expand All @@ -1867,7 +1868,7 @@ _PyObject_GC_Resize(PyVarObject *op, Py_ssize_t nitems)
return (PyVarObject *)PyErr_NoMemory();
}
char *mem = (char *)op - presize;
mem = (char *)PyObject_Realloc(mem, presize + basicsize);
mem = (char *)_PyObject_ReallocWithType(Py_TYPE(op), mem, presize + basicsize);
if (mem == NULL) {
return (PyVarObject *)PyErr_NoMemory();
}
Expand Down