diff options
author | Campbell Barton <campbell@blender.org> | 2022-09-28 09:09:12 +0300 |
---|---|---|
committer | Campbell Barton <campbell@blender.org> | 2022-09-28 10:53:30 +0300 |
commit | 5270ac5ed87fb5f9b5605fdc332f16ea0f7ee59d (patch) | |
tree | 4319932f4aee73faf1a56ec04a48b29c7055d9db /source/blender | |
parent | ada2b9f6e43e687acba8b7b86394d3cc20eed291 (diff) |
Fix GC tracking error for instances of mathutils types
Mathutils types were always GC tracked even when it wasn't intended.
Not having to track objects speeds up Python execution.
In an isolated benchmark created to stress test the GC
creating 4-million vectors (re-assigning them 100 times), this gives
an overall ~2.5x speedup, see: P3221.
Details:
Since [0] (which added support for sub-classed mathutils types)
tp_alloc was called which defaults to PyType_GenericAlloc which always
GC tracked the resulting object when Py_TPFLAGS_HAVE_GC was set.
Avoid using PyType_GenericAlloc unless the type is sub-classed,
in that case the object is un-tracked.
Add asserts that the tracked state is as expected before tracking &
un-tracking, to ensure changes to object creation don't cause objects
to be tracked unintentionally.
Also assign the PyTypeObject.tp_is_gc callback so types optionally GC
track objects only do so when an object is referenced.
[0]: fbd936494495d0de54eef24a97957e000306785f
Diffstat (limited to 'source/blender')
-rw-r--r-- | source/blender/python/generic/idprop_py_api.c | 14 | ||||
-rw-r--r-- | source/blender/python/gpu/gpu_py_batch.c | 7 | ||||
-rw-r--r-- | source/blender/python/gpu/gpu_py_buffer.c | 7 | ||||
-rw-r--r-- | source/blender/python/intern/bpy_props.c | 1 | ||||
-rw-r--r-- | source/blender/python/intern/bpy_rna.c | 1 | ||||
-rw-r--r-- | source/blender/python/intern/bpy_rna_data.c | 1 | ||||
-rw-r--r-- | source/blender/python/mathutils/mathutils.c | 47 | ||||
-rw-r--r-- | source/blender/python/mathutils/mathutils.h | 6 | ||||
-rw-r--r-- | source/blender/python/mathutils/mathutils_Color.c | 3 | ||||
-rw-r--r-- | source/blender/python/mathutils/mathutils_Euler.c | 3 | ||||
-rw-r--r-- | source/blender/python/mathutils/mathutils_Matrix.c | 3 | ||||
-rw-r--r-- | source/blender/python/mathutils/mathutils_Quaternion.c | 3 | ||||
-rw-r--r-- | source/blender/python/mathutils/mathutils_Vector.c | 5 |
13 files changed, 93 insertions, 8 deletions
diff --git a/source/blender/python/generic/idprop_py_api.c b/source/blender/python/generic/idprop_py_api.c index 333ab9487d1..3978f7f37cc 100644 --- a/source/blender/python/generic/idprop_py_api.c +++ b/source/blender/python/generic/idprop_py_api.c @@ -909,6 +909,11 @@ static int BPy_IDGroup_Iter_clear(BPy_IDGroup_Iter *self) return 0; } +static int BPy_IDGroup_Iter_is_gc(BPy_IDGroup_Iter *self) +{ + return (self->group != NULL); +} + static bool BPy_Group_Iter_same_size_or_raise_error(BPy_IDGroup_Iter *self) { if (self->len_init == self->group->prop->len) { @@ -1000,6 +1005,7 @@ static void IDGroup_Iter_init_type(void) SHARED_MEMBER_SET(tp_flags, Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC); SHARED_MEMBER_SET(tp_traverse, (traverseproc)BPy_IDGroup_Iter_traverse); SHARED_MEMBER_SET(tp_clear, (inquiry)BPy_IDGroup_Iter_clear); + SHARED_MEMBER_SET(tp_is_gc, (inquiry)BPy_IDGroup_Iter_is_gc); SHARED_MEMBER_SET(tp_iter, PyObject_SelfIter); #undef SHARED_MEMBER_SET @@ -1015,6 +1021,7 @@ static PyObject *IDGroup_Iter_New_WithType(BPy_IDProperty *group, iter->group = group; if (group != NULL) { Py_INCREF(group); + BLI_assert(!PyObject_GC_IsTracked((PyObject *)iter)); PyObject_GC_Track(iter); iter->cur = (reversed ? group->prop->data.group.last : group->prop->data.group.first); iter->len_init = group->prop->len; @@ -1086,6 +1093,11 @@ static int BPy_IDGroup_View_clear(BPy_IDGroup_View *self) return 0; } +static int BPy_IDGroup_View_is_gc(BPy_IDGroup_View *self) +{ + return (self->group != NULL); +} + /* View Specific API's (Key/Value/Items). */ static PyObject *BPy_Group_ViewKeys_iter(BPy_IDGroup_View *self) @@ -1233,6 +1245,7 @@ static void IDGroup_View_init_type(void) SHARED_MEMBER_SET(tp_flags, Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC); SHARED_MEMBER_SET(tp_traverse, (traverseproc)BPy_IDGroup_View_traverse); SHARED_MEMBER_SET(tp_clear, (inquiry)BPy_IDGroup_View_clear); + SHARED_MEMBER_SET(tp_is_gc, (inquiry)BPy_IDGroup_View_is_gc); SHARED_MEMBER_SET(tp_methods, BPy_IDGroup_View_methods); #undef SHARED_MEMBER_SET @@ -2087,6 +2100,7 @@ static BPy_IDGroup_View *IDGroup_View_New_WithType(BPy_IDProperty *group, PyType iter->group = group; if (group != NULL) { Py_INCREF(group); + BLI_assert(!PyObject_GC_IsTracked((PyObject *)iter)); PyObject_GC_Track(iter); } return iter; diff --git a/source/blender/python/gpu/gpu_py_batch.c b/source/blender/python/gpu/gpu_py_batch.c index 879e1c0ce8b..a36b1dfd1b5 100644 --- a/source/blender/python/gpu/gpu_py_batch.c +++ b/source/blender/python/gpu/gpu_py_batch.c @@ -111,6 +111,7 @@ static PyObject *pygpu_batch__tp_new(PyTypeObject *UNUSED(type), PyObject *args, Py_INCREF(py_indexbuf); } + BLI_assert(!PyObject_GC_IsTracked((PyObject *)ret)); PyObject_GC_Track(ret); #endif @@ -273,6 +274,11 @@ static int pygpu_batch__tp_clear(BPyGPUBatch *self) return 0; } +static int pygpu_batch__tp_is_gc(BPyGPUBatch *self) +{ + return self->references != NULL; +} + #endif static void pygpu_batch__tp_dealloc(BPyGPUBatch *self) @@ -313,6 +319,7 @@ PyTypeObject BPyGPUBatch_Type = { .tp_doc = pygpu_batch__tp_doc, .tp_traverse = (traverseproc)pygpu_batch__tp_traverse, .tp_clear = (inquiry)pygpu_batch__tp_clear, + .tp_is_gc = (inquiry)pygpu_batch__tp_is_gc, #else .tp_flags = Py_TPFLAGS_DEFAULT, #endif diff --git a/source/blender/python/gpu/gpu_py_buffer.c b/source/blender/python/gpu/gpu_py_buffer.c index 551eb4451c2..30a434f8667 100644 --- a/source/blender/python/gpu/gpu_py_buffer.c +++ b/source/blender/python/gpu/gpu_py_buffer.c @@ -153,6 +153,7 @@ static BPyGPUBuffer *pygpu_buffer_make_from_data(PyObject *parent, if (parent) { Py_INCREF(parent); buffer->parent = parent; + BLI_assert(!PyObject_GC_IsTracked((PyObject *)buffer)); PyObject_GC_Track(buffer); } return buffer; @@ -422,6 +423,11 @@ static PyObject *pygpu_buffer__tp_new(PyTypeObject *UNUSED(type), PyObject *args return (PyObject *)buffer; } +static int pygpu_buffer__tp_is_gc(BPyGPUBuffer *self) +{ + return self->parent != NULL; +} + /* BPyGPUBuffer sequence methods */ static int pygpu_buffer__sq_length(BPyGPUBuffer *self) @@ -677,6 +683,7 @@ PyTypeObject BPyGPU_BufferType = { .tp_methods = pygpu_buffer__tp_methods, .tp_getset = pygpu_buffer_getseters, .tp_new = pygpu_buffer__tp_new, + .tp_is_gc = (inquiry)pygpu_buffer__tp_is_gc, }; static size_t pygpu_buffer_calc_size(const int format, diff --git a/source/blender/python/intern/bpy_props.c b/source/blender/python/intern/bpy_props.c index ff84a5c75d7..b6c75f7a793 100644 --- a/source/blender/python/intern/bpy_props.c +++ b/source/blender/python/intern/bpy_props.c @@ -307,6 +307,7 @@ static PyObject *bpy_prop_deferred_data_CreatePyObject(PyObject *fn, PyObject *k Py_INCREF(kw); } self->kw = kw; + BLI_assert(!PyObject_GC_IsTracked((PyObject *)self)); PyObject_GC_Track(self); return (PyObject *)self; } diff --git a/source/blender/python/intern/bpy_rna.c b/source/blender/python/intern/bpy_rna.c index 8cc66854c77..56ad05d5501 100644 --- a/source/blender/python/intern/bpy_rna.c +++ b/source/blender/python/intern/bpy_rna.c @@ -1174,6 +1174,7 @@ static void pyrna_struct_reference_set(BPy_StructRNA *self, PyObject *reference) if (reference) { self->reference = reference; Py_INCREF(reference); + BLI_assert(!PyObject_GC_IsTracked((PyObject *)self)); PyObject_GC_Track(self); } } diff --git a/source/blender/python/intern/bpy_rna_data.c b/source/blender/python/intern/bpy_rna_data.c index a3e865f4846..3c7ec3bddc3 100644 --- a/source/blender/python/intern/bpy_rna_data.c +++ b/source/blender/python/intern/bpy_rna_data.c @@ -183,6 +183,7 @@ static PyObject *bpy_rna_data_context_enter(BPy_DataContext *self) self->data_rna = (BPy_StructRNA *)pyrna_struct_CreatePyObject(&ptr); + BLI_assert(!PyObject_GC_IsTracked((PyObject *)self)); PyObject_GC_Track(self); return (PyObject *)self->data_rna; diff --git a/source/blender/python/mathutils/mathutils.c b/source/blender/python/mathutils/mathutils.c index 2bf608fd1d6..c4c0659100b 100644 --- a/source/blender/python/mathutils/mathutils.c +++ b/source/blender/python/mathutils/mathutils.c @@ -697,6 +697,18 @@ int BaseMathObject_clear(BaseMathObject *self) return 0; } +/** Only to validate assumptions when debugging. */ +#ifndef NDEBUG +static bool BaseMathObject_is_tracked(BaseMathObject *self) +{ + PyObject *cb_user = self->cb_user; + self->cb_user = (void *)(uintptr_t)-1; + bool is_tracked = PyObject_GC_IsTracked((PyObject *)self); + self->cb_user = cb_user; + return is_tracked; +} +#endif /* NDEBUG */ + void BaseMathObject_dealloc(BaseMathObject *self) { /* only free non wrapped */ @@ -705,13 +717,48 @@ void BaseMathObject_dealloc(BaseMathObject *self) } if (self->cb_user) { + BLI_assert(BaseMathObject_is_tracked(self) == true); PyObject_GC_UnTrack(self); BaseMathObject_clear(self); } + else if (!BaseMathObject_CheckExact(self)) { + /* Sub-classed types get an extra track (in Pythons internal `subtype_dealloc` function). */ + BLI_assert(BaseMathObject_is_tracked(self) == true); + PyObject_GC_UnTrack(self); + BLI_assert(BaseMathObject_is_tracked(self) == false); + } Py_TYPE(self)->tp_free(self); // PyObject_DEL(self); /* breaks sub-types. */ } +int BaseMathObject_is_gc(BaseMathObject *self) +{ + return self->cb_user != NULL; +} + +PyObject *_BaseMathObject_new_impl(PyTypeObject *root_type, PyTypeObject *base_type) +{ + PyObject *obj; + if (ELEM(base_type, NULL, root_type)) { + obj = _PyObject_GC_New(root_type); + if (obj) { + BLI_assert(BaseMathObject_is_tracked((BaseMathObject *)obj) == false); + } + } + else { + /* Calls Generic allocation function which always tracks + * (because `root_type` is flagged for GC). */ + obj = base_type->tp_alloc(base_type, 0); + if (obj) { + BLI_assert(BaseMathObject_is_tracked((BaseMathObject *)obj) == true); + PyObject_GC_UnTrack(obj); + BLI_assert(BaseMathObject_is_tracked((BaseMathObject *)obj) == false); + } + } + + return obj; +} + /*----------------------------MODULE INIT-------------------------*/ static struct PyMethodDef M_Mathutils_methods[] = { {NULL, NULL, 0, NULL}, diff --git a/source/blender/python/mathutils/mathutils.h b/source/blender/python/mathutils/mathutils.h index 27d84a80601..f4beaf92ad4 100644 --- a/source/blender/python/mathutils/mathutils.h +++ b/source/blender/python/mathutils/mathutils.h @@ -17,9 +17,10 @@ extern char BaseMathObject_is_frozen_doc[]; extern char BaseMathObject_is_valid_doc[]; extern char BaseMathObject_owner_doc[]; +PyObject *_BaseMathObject_new_impl(PyTypeObject *root_type, PyTypeObject *base_type); + #define BASE_MATH_NEW(struct_name, root_type, base_type) \ - ((struct_name *)((base_type ? (base_type)->tp_alloc(base_type, 0) : \ - _PyObject_GC_New(&(root_type))))) + ((struct_name *)_BaseMathObject_new_impl(&root_type, base_type)) /** #BaseMathObject.flag */ enum { @@ -76,6 +77,7 @@ PyObject *BaseMathObject_freeze(BaseMathObject *self); int BaseMathObject_traverse(BaseMathObject *self, visitproc visit, void *arg); int BaseMathObject_clear(BaseMathObject *self); void BaseMathObject_dealloc(BaseMathObject *self); +int BaseMathObject_is_gc(BaseMathObject *self); PyMODINIT_FUNC PyInit_mathutils(void); diff --git a/source/blender/python/mathutils/mathutils_Color.c b/source/blender/python/mathutils/mathutils_Color.c index a66178baefc..432b1709d82 100644 --- a/source/blender/python/mathutils/mathutils_Color.c +++ b/source/blender/python/mathutils/mathutils_Color.c @@ -1156,7 +1156,7 @@ PyTypeObject color_Type = { NULL, /* tp_alloc */ Color_new, /* tp_new */ NULL, /* tp_free */ - NULL, /* tp_is_gc */ + (inquiry)BaseMathObject_is_gc, /* tp_is_gc */ NULL, /* tp_bases */ NULL, /* tp_mro */ NULL, /* tp_cache */ @@ -1235,6 +1235,7 @@ PyObject *Color_CreatePyObject_cb(PyObject *cb_user, uchar cb_type, uchar cb_sub self->cb_user = cb_user; self->cb_type = cb_type; self->cb_subtype = cb_subtype; + BLI_assert(!PyObject_GC_IsTracked((PyObject *)self)); PyObject_GC_Track(self); } diff --git a/source/blender/python/mathutils/mathutils_Euler.c b/source/blender/python/mathutils/mathutils_Euler.c index 78c98d8f482..8da2b20d9a5 100644 --- a/source/blender/python/mathutils/mathutils_Euler.c +++ b/source/blender/python/mathutils/mathutils_Euler.c @@ -821,7 +821,7 @@ PyTypeObject euler_Type = { NULL, /* tp_alloc */ Euler_new, /* tp_new */ NULL, /* tp_free */ - NULL, /* tp_is_gc */ + (inquiry)BaseMathObject_is_gc, /* tp_is_gc */ NULL, /* tp_bases */ NULL, /* tp_mro */ NULL, /* tp_cache */ @@ -904,6 +904,7 @@ PyObject *Euler_CreatePyObject_cb(PyObject *cb_user, self->cb_user = cb_user; self->cb_type = cb_type; self->cb_subtype = cb_subtype; + BLI_assert(!PyObject_GC_IsTracked((PyObject *)self)); PyObject_GC_Track(self); } diff --git a/source/blender/python/mathutils/mathutils_Matrix.c b/source/blender/python/mathutils/mathutils_Matrix.c index af01c571fe9..51362ec595f 100644 --- a/source/blender/python/mathutils/mathutils_Matrix.c +++ b/source/blender/python/mathutils/mathutils_Matrix.c @@ -3366,7 +3366,7 @@ PyTypeObject matrix_Type = { NULL, /*tp_alloc*/ Matrix_new, /*tp_new*/ NULL, /*tp_free*/ - NULL, /*tp_is_gc*/ + (inquiry)BaseMathObject_is_gc, /*tp_is_gc*/ NULL, /*tp_bases*/ NULL, /*tp_mro*/ NULL, /*tp_cache*/ @@ -3474,6 +3474,7 @@ PyObject *Matrix_CreatePyObject_cb( self->cb_user = cb_user; self->cb_type = cb_type; self->cb_subtype = cb_subtype; + BLI_assert(!PyObject_GC_IsTracked((PyObject *)self)); PyObject_GC_Track(self); } return (PyObject *)self; diff --git a/source/blender/python/mathutils/mathutils_Quaternion.c b/source/blender/python/mathutils/mathutils_Quaternion.c index 892f38d22ae..d5519676081 100644 --- a/source/blender/python/mathutils/mathutils_Quaternion.c +++ b/source/blender/python/mathutils/mathutils_Quaternion.c @@ -1724,7 +1724,7 @@ PyTypeObject quaternion_Type = { NULL, /* tp_alloc */ Quaternion_new, /* tp_new */ NULL, /* tp_free */ - NULL, /* tp_is_gc */ + (inquiry)BaseMathObject_is_gc, /* tp_is_gc */ NULL, /* tp_bases */ NULL, /* tp_mro */ NULL, /* tp_cache */ @@ -1800,6 +1800,7 @@ PyObject *Quaternion_CreatePyObject_cb(PyObject *cb_user, uchar cb_type, uchar c self->cb_user = cb_user; self->cb_type = cb_type; self->cb_subtype = cb_subtype; + BLI_assert(!PyObject_GC_IsTracked((PyObject *)self)); PyObject_GC_Track(self); } diff --git a/source/blender/python/mathutils/mathutils_Vector.c b/source/blender/python/mathutils/mathutils_Vector.c index 161f05deafd..654ddcb1237 100644 --- a/source/blender/python/mathutils/mathutils_Vector.c +++ b/source/blender/python/mathutils/mathutils_Vector.c @@ -3265,8 +3265,8 @@ PyTypeObject vector_Type = { /* Low-level free-memory routine */ NULL, /* freefunc tp_free; */ /* For PyObject_IS_GC */ - NULL, /* inquiry tp_is_gc; */ - NULL, /* PyObject *tp_bases; */ + (inquiry)BaseMathObject_is_gc, /* inquiry tp_is_gc; */ + NULL, /* PyObject *tp_bases; */ /* method resolution order */ NULL, /* PyObject *tp_mro; */ NULL, /* PyObject *tp_cache; */ @@ -3357,6 +3357,7 @@ PyObject *Vector_CreatePyObject_cb(PyObject *cb_user, int vec_num, uchar cb_type self->cb_user = cb_user; self->cb_type = cb_type; self->cb_subtype = cb_subtype; + BLI_assert(!PyObject_GC_IsTracked((PyObject *)self)); PyObject_GC_Track(self); } |