From ee5977214835d746242b3a54a669e8bfc35f8e11 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 5 Aug 2021 12:06:06 +0100 Subject: [PATCH 01/19] Remove unsafe _PyObject_GC_Calloc function. --- Include/cpython/objimpl.h | 1 - Modules/gcmodule.c | 23 +++-------------------- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/Include/cpython/objimpl.h b/Include/cpython/objimpl.h index d83700e2a4647f..00bbd0cad6d405 100644 --- a/Include/cpython/objimpl.h +++ b/Include/cpython/objimpl.h @@ -91,7 +91,6 @@ PyAPI_FUNC(int) PyObject_IS_GC(PyObject *obj); #endif PyAPI_FUNC(PyObject *) _PyObject_GC_Malloc(size_t size); -PyAPI_FUNC(PyObject *) _PyObject_GC_Calloc(size_t size); /* Test if a type supports weak references */ diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index 2592c397cf2f73..e045eb2170953f 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -2232,8 +2232,8 @@ PyObject_IS_GC(PyObject *obj) return _PyObject_IS_GC(obj); } -static PyObject * -_PyObject_GC_Alloc(int use_calloc, size_t basicsize) +PyObject * +_PyObject_GC_Malloc(size_t basicsize) { PyThreadState *tstate = _PyThreadState_GET(); GCState *gcstate = &tstate->interp->gc; @@ -2242,13 +2242,7 @@ _PyObject_GC_Alloc(int use_calloc, size_t basicsize) } size_t size = sizeof(PyGC_Head) + basicsize; - PyGC_Head *g; - if (use_calloc) { - g = (PyGC_Head *)PyObject_Calloc(1, size); - } - else { - g = (PyGC_Head *)PyObject_Malloc(size); - } + PyGC_Head *g = (PyGC_Head *)PyObject_Malloc(size); if (g == NULL) { return _PyErr_NoMemory(tstate); } @@ -2271,17 +2265,6 @@ _PyObject_GC_Alloc(int use_calloc, size_t basicsize) return op; } -PyObject * -_PyObject_GC_Malloc(size_t basicsize) -{ - return _PyObject_GC_Alloc(0, basicsize); -} - -PyObject * -_PyObject_GC_Calloc(size_t basicsize) -{ - return _PyObject_GC_Alloc(1, basicsize); -} PyObject * _PyObject_GC_New(PyTypeObject *tp) From 72b71cc7d64034b9c9960c4e4599920d15f3f4d0 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 5 Aug 2021 17:47:50 +0100 Subject: [PATCH 02/19] Place __dict__ immediately before GC header for variable sized objects and plain Python objects. Work in progress. --- Include/internal/pycore_object.h | 9 +++++ Include/object.h | 3 ++ Lib/test/test_capi.py | 10 ++---- Modules/_testcapimodule.c | 13 ++++---- Modules/gcmodule.c | 34 ++++++++++++++++--- Objects/object.c | 26 ++------------- Objects/typeobject.c | 57 ++++++++++++++++++-------------- 7 files changed, 86 insertions(+), 66 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 744b41ae5d90d6..6cdfc1faf33cfc 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -168,6 +168,15 @@ _PyObject_IS_GC(PyObject *obj) // Fast inlined version of PyType_IS_GC() #define _PyType_IS_GC(t) _PyType_HasFeature((t), Py_TPFLAGS_HAVE_GC) +static inline size_t +_PyType_PreHeaderSize(PyTypeObject *tp) +{ + return _PyType_IS_GC(tp) * sizeof(PyGC_Head) + + _PyType_HasFeature(tp, Py_TPFLAGS_MANAGED_DICT) * sizeof(PyObject *); +} + +void _PyObject_GC_Link(PyObject *op); + // Usage: assert(_Py_CheckSlotResult(obj, "__getitem__", result != NULL))); extern int _Py_CheckSlotResult( PyObject *obj, diff --git a/Include/object.h b/Include/object.h index 23ebad84ab4672..b8846d13a019ff 100644 --- a/Include/object.h +++ b/Include/object.h @@ -326,6 +326,9 @@ given type object has a specified feature. */ #ifndef Py_LIMITED_API + +#define Py_TPFLAGS_MANAGED_DICT (1 << 4) + /* Set if instances of the type object are treated as sequences for pattern matching */ #define Py_TPFLAGS_SEQUENCE (1 << 5) /* Set if instances of the type object are treated as mappings for pattern matching */ diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 9165f45db64f34..a054fdf6565192 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -508,14 +508,8 @@ def test_heaptype_with_dict(self): self.assertEqual({}, inst.__dict__) def test_heaptype_with_negative_dict(self): - inst = _testcapi.HeapCTypeWithNegativeDict() - inst.foo = 42 - self.assertEqual(inst.foo, 42) - self.assertEqual(inst.dictobj, inst.__dict__) - self.assertEqual(inst.dictobj, {"foo": 42}) - - inst = _testcapi.HeapCTypeWithNegativeDict() - self.assertEqual({}, inst.__dict__) + with self.assertRaises(TypeError): + _testcapi.negative_dictoffset() def test_heaptype_with_weakref(self): inst = _testcapi.HeapCTypeWithWeakref() diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index f338e89f426da0..e816a6d4381cc5 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -5576,6 +5576,7 @@ test_fatal_error(PyObject *self, PyObject *args) } +static PyObject *negative_dictoffset(PyObject *, PyObject *); static PyObject *test_buildvalue_issue38913(PyObject *, PyObject *); static PyObject *getargs_s_hash_int(PyObject *, PyObject *, PyObject*); @@ -5646,6 +5647,7 @@ static PyMethodDef TestMethods[] = { {"getbuffer_with_null_view", getbuffer_with_null_view, METH_O}, {"PyBuffer_SizeFromFormat", test_PyBuffer_SizeFromFormat, METH_VARARGS}, {"test_buildvalue_N", test_buildvalue_N, METH_NOARGS}, + {"negative_dictoffset", negative_dictoffset, METH_NOARGS}, {"test_buildvalue_issue38913", test_buildvalue_issue38913, METH_NOARGS}, {"get_args", get_args, METH_VARARGS}, {"test_get_statictype_slots", test_get_statictype_slots, METH_NOARGS}, @@ -7288,12 +7290,6 @@ PyInit__testcapi(void) } PyModule_AddObject(m, "HeapCTypeWithDict", HeapCTypeWithDict); - PyObject *HeapCTypeWithNegativeDict = PyType_FromSpec(&HeapCTypeWithNegativeDict_spec); - if (HeapCTypeWithNegativeDict == NULL) { - return NULL; - } - PyModule_AddObject(m, "HeapCTypeWithNegativeDict", HeapCTypeWithNegativeDict); - PyObject *HeapCTypeWithWeakref = PyType_FromSpec(&HeapCTypeWithWeakref_spec); if (HeapCTypeWithWeakref == NULL) { return NULL; @@ -7336,6 +7332,11 @@ PyInit__testcapi(void) return m; } +static PyObject * +negative_dictoffset(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + return PyType_FromSpec(&HeapCTypeWithNegativeDict_spec); +} /* Test the C API exposed when PY_SSIZE_T_CLEAN is not defined */ diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index e045eb2170953f..78d5c56222266a 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -69,10 +69,10 @@ module gc #define NEXT_MASK_UNREACHABLE (1) /* Get an object's GC head */ -#define AS_GC(o) ((PyGC_Head *)(o)-1) +#define AS_GC(o) ((PyGC_Head *)(((char *)(o))-sizeof(PyGC_Head))) /* Get the object given the GC head */ -#define FROM_GC(g) ((PyObject *)(((PyGC_Head *)g)+1)) +#define FROM_GC(g) ((PyObject *)(((char *)(g))+sizeof(PyGC_Head))) static inline int gc_is_collecting(PyGC_Head *g) @@ -2232,6 +2232,31 @@ PyObject_IS_GC(PyObject *obj) return _PyObject_IS_GC(obj); } +void +_PyObject_GC_Link(PyObject *op) +{ + PyGC_Head *g = AS_GC(op); + assert(((uintptr_t)g & (sizeof(uintptr_t)-1)) == 0); // g must be correctly aligned + + PyThreadState *tstate = _PyThreadState_GET(); + GCState *gcstate = &tstate->interp->gc; + g->_gc_next = 0; + g->_gc_prev = 0; + gcstate->generations[0].count++; /* number of allocated GC objects */ + if (gcstate->generations[0].count > gcstate->generations[0].threshold && + gcstate->enabled && + gcstate->generations[0].threshold && + !gcstate->collecting && + !_PyErr_Occurred(tstate)) + { + gcstate->collecting = 1; + gc_collect_generations(tstate); + gcstate->collecting = 0; + } +} + + + PyObject * _PyObject_GC_Malloc(size_t basicsize) { @@ -2246,7 +2271,7 @@ _PyObject_GC_Malloc(size_t basicsize) if (g == NULL) { return _PyErr_NoMemory(tstate); } - assert(((uintptr_t)g & 3) == 0); // g must be aligned 4bytes boundary + assert(((uintptr_t)g & sizeof(uintptr_t)) == 0); // g must be correctly aligned g->_gc_next = 0; g->_gc_prev = 0; @@ -2317,6 +2342,7 @@ _PyObject_GC_Resize(PyVarObject *op, Py_ssize_t nitems) void PyObject_GC_Del(void *op) { + size_t presize = _PyType_PreHeaderSize(((PyObject *)op)->ob_type); PyGC_Head *g = AS_GC(op); if (_PyObject_GC_IS_TRACKED(op)) { gc_list_remove(g); @@ -2325,7 +2351,7 @@ PyObject_GC_Del(void *op) if (gcstate->generations[0].count > 0) { gcstate->generations[0].count--; } - PyObject_Free(g); + PyObject_Free(((char *)op)-presize); } int diff --git a/Objects/object.c b/Objects/object.c index 446c974f8e614b..37a2dafc625347 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1073,19 +1073,10 @@ _PyObject_GetDictPtr(PyObject *obj) PyTypeObject *tp = Py_TYPE(obj); dictoffset = tp->tp_dictoffset; - if (dictoffset == 0) + if (dictoffset == 0) { return NULL; - if (dictoffset < 0) { - Py_ssize_t tsize = Py_SIZE(obj); - if (tsize < 0) { - tsize = -tsize; - } - size_t size = _PyObject_VAR_SIZE(tp, tsize); - - dictoffset += (long)size; - _PyObject_ASSERT(obj, dictoffset > 0); - _PyObject_ASSERT(obj, dictoffset % SIZEOF_VOID_P == 0); } + assert(dictoffset > 0 || dictoffset == -3*((Py_ssize_t)sizeof(PyObject *))); return (PyObject **) ((char *)obj + dictoffset); } @@ -1251,18 +1242,7 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name, /* Inline _PyObject_GetDictPtr */ dictoffset = tp->tp_dictoffset; if (dictoffset != 0) { - if (dictoffset < 0) { - Py_ssize_t tsize = Py_SIZE(obj); - if (tsize < 0) { - tsize = -tsize; - } - size_t size = _PyObject_VAR_SIZE(tp, tsize); - _PyObject_ASSERT(obj, size <= PY_SSIZE_T_MAX); - - dictoffset += (Py_ssize_t)size; - _PyObject_ASSERT(obj, dictoffset > 0); - _PyObject_ASSERT(obj, dictoffset % SIZEOF_VOID_P == 0); - } + assert(dictoffset > 0 || dictoffset == -3*((Py_ssize_t)sizeof(PyObject *))); dictptr = (PyObject **) ((char *)obj + dictoffset); dict = *dictptr; } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 7ae50c453ed2f8..a149f6fcbc8e99 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1160,17 +1160,16 @@ _PyType_AllocNoTrack(PyTypeObject *type, Py_ssize_t nitems) const size_t size = _PyObject_VAR_SIZE(type, nitems+1); /* note that we need to add one, for the sentinel */ - if (_PyType_IS_GC(type)) { - obj = _PyObject_GC_Malloc(size); - } - else { - obj = (PyObject *)PyObject_Malloc(size); - } - - if (obj == NULL) { + const size_t presize = _PyType_PreHeaderSize(type); + char *alloc = PyObject_Malloc(size + presize); + if (alloc == NULL) { return PyErr_NoMemory(); } - + obj = (PyObject *)(alloc + presize); + if (presize) { + ((PyObject **)alloc)[0] = NULL; + _PyObject_GC_Link(obj); + } memset(obj, '\0', size); if (type->tp_itemsize == 0) { @@ -1363,6 +1362,8 @@ subtype_dealloc(PyObject *self) int type_needs_decref = (type->tp_flags & Py_TPFLAGS_HEAPTYPE && !(base->tp_flags & Py_TPFLAGS_HEAPTYPE)); + assert((type->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0); + /* Call the base tp_dealloc() */ assert(basedealloc); basedealloc(self); @@ -2979,13 +2980,8 @@ type_new_descriptors(const type_new_ctx *ctx, PyTypeObject *type) } if (ctx->add_dict) { - if (ctx->base->tp_itemsize) { - type->tp_dictoffset = -(long)sizeof(PyObject *); - } - else { - type->tp_dictoffset = slotoffset; - } - slotoffset += sizeof(PyObject *); + type->tp_flags |= Py_TPFLAGS_MANAGED_DICT; + type->tp_dictoffset = -(long)sizeof(PyObject *)*3; } if (ctx->add_weak) { @@ -3584,6 +3580,10 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases) goto fail; } if (dictoffset) { + if (dictoffset < 0) { + PyErr_SetString(PyExc_TypeError, "Negative __dictoffset__"); + goto fail; + } type->tp_dictoffset = dictoffset; if (PyDict_DelItemString((PyObject *)type->tp_dict, "__dictoffset__") < 0) goto fail; @@ -4694,16 +4694,22 @@ compatible_for_assignment(PyTypeObject* oldto, PyTypeObject* newto, const char* if (newbase != oldbase && (newbase->tp_base != oldbase->tp_base || !same_slots_added(newbase, oldbase))) { - PyErr_Format(PyExc_TypeError, - "%s assignment: " - "'%s' object layout differs from '%s'", - attr, - newto->tp_name, - oldto->tp_name); - return 0; + goto differs; } - - return 1; + /* The above does not check for managed __dicts__ */ + if ((oldto->tp_flags & Py_TPFLAGS_MANAGED_DICT) == + ((newto->tp_flags & Py_TPFLAGS_MANAGED_DICT))) + { + return 1; + } +differs: + PyErr_Format(PyExc_TypeError, + "%s assignment: " + "'%s' object layout differs from '%s'", + attr, + newto->tp_name, + oldto->tp_name); + return 0; } static int @@ -5704,6 +5710,7 @@ inherit_special(PyTypeObject *type, PyTypeObject *base) if (type->tp_clear == NULL) type->tp_clear = base->tp_clear; } + type->tp_flags |= (base->tp_flags & Py_TPFLAGS_MANAGED_DICT); if (type->tp_basicsize == 0) type->tp_basicsize = base->tp_basicsize; From cd22dacf501ab16a3c29b8a54a7bdf53443a97d0 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 29 Nov 2021 14:37:35 +0000 Subject: [PATCH 03/19] Restore documented behavior of tp_dictoffset. --- Lib/test/test_capi.py | 10 ++++++++-- Modules/_testcapimodule.c | 6 ++++++ Objects/object.c | 31 +++++++++++++++++++++++-------- Objects/typeobject.c | 15 ++++++++------- 4 files changed, 45 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index a054fdf6565192..9165f45db64f34 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -508,8 +508,14 @@ def test_heaptype_with_dict(self): self.assertEqual({}, inst.__dict__) def test_heaptype_with_negative_dict(self): - with self.assertRaises(TypeError): - _testcapi.negative_dictoffset() + inst = _testcapi.HeapCTypeWithNegativeDict() + inst.foo = 42 + self.assertEqual(inst.foo, 42) + self.assertEqual(inst.dictobj, inst.__dict__) + self.assertEqual(inst.dictobj, {"foo": 42}) + + inst = _testcapi.HeapCTypeWithNegativeDict() + self.assertEqual({}, inst.__dict__) def test_heaptype_with_weakref(self): inst = _testcapi.HeapCTypeWithWeakref() diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index e816a6d4381cc5..38ece9af8366e1 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -7290,6 +7290,12 @@ PyInit__testcapi(void) } PyModule_AddObject(m, "HeapCTypeWithDict", HeapCTypeWithDict); + PyObject *HeapCTypeWithNegativeDict = PyType_FromSpec(&HeapCTypeWithNegativeDict_spec); + if (HeapCTypeWithNegativeDict == NULL) { + return NULL; + } + PyModule_AddObject(m, "HeapCTypeWithNegativeDict", HeapCTypeWithNegativeDict); + PyObject *HeapCTypeWithWeakref = PyType_FromSpec(&HeapCTypeWithWeakref_spec); if (HeapCTypeWithWeakref == NULL) { return NULL; diff --git a/Objects/object.c b/Objects/object.c index 37a2dafc625347..5084904468353a 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1073,9 +1073,24 @@ _PyObject_GetDictPtr(PyObject *obj) PyTypeObject *tp = Py_TYPE(obj); dictoffset = tp->tp_dictoffset; + if (tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) { + return &((PyObject **)obj)[-3]; + } if (dictoffset == 0) { return NULL; } + if (dictoffset < 0) { + Py_ssize_t tsize = Py_SIZE(obj); + if (tsize < 0) { + tsize = -tsize; + } + size_t size = _PyObject_VAR_SIZE(tp, tsize); + _PyObject_ASSERT(obj, size <= PY_SSIZE_T_MAX); + + dictoffset += (Py_ssize_t)size; + _PyObject_ASSERT(obj, dictoffset > 0); + _PyObject_ASSERT(obj, dictoffset % SIZEOF_VOID_P == 0); + } assert(dictoffset > 0 || dictoffset == -3*((Py_ssize_t)sizeof(PyObject *))); return (PyObject **) ((char *)obj + dictoffset); } @@ -1206,8 +1221,6 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name, PyObject *descr = NULL; PyObject *res = NULL; descrgetfunc f; - Py_ssize_t dictoffset; - PyObject **dictptr; if (!PyUnicode_Check(name)){ PyErr_Format(PyExc_TypeError, @@ -1239,12 +1252,14 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name, } if (dict == NULL) { - /* Inline _PyObject_GetDictPtr */ - dictoffset = tp->tp_dictoffset; - if (dictoffset != 0) { - assert(dictoffset > 0 || dictoffset == -3*((Py_ssize_t)sizeof(PyObject *))); - dictptr = (PyObject **) ((char *)obj + dictoffset); - dict = *dictptr; + if (tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) { + dict = ((PyObject **)obj)[-3]; + } + else { + PyObject **dictptr = _PyObject_GetDictPtr(obj); + if (dictptr != NULL) { + dict = *dictptr; + } } } if (dict != NULL) { diff --git a/Objects/typeobject.c b/Objects/typeobject.c index a149f6fcbc8e99..1af1e06e48e1ca 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2979,9 +2979,9 @@ type_new_descriptors(const type_new_ctx *ctx, PyTypeObject *type) } } - if (ctx->add_dict) { - type->tp_flags |= Py_TPFLAGS_MANAGED_DICT; - type->tp_dictoffset = -(long)sizeof(PyObject *)*3; + if (ctx->add_dict && ctx->base->tp_itemsize) { + type->tp_dictoffset = -(long)sizeof(PyObject *); + slotoffset += sizeof(PyObject *); } if (ctx->add_weak) { @@ -2990,6 +2990,11 @@ type_new_descriptors(const type_new_ctx *ctx, PyTypeObject *type) slotoffset += sizeof(PyObject *); } + if (ctx->add_dict && ctx->base->tp_itemsize == 0) { + type->tp_flags |= Py_TPFLAGS_MANAGED_DICT; + type->tp_dictoffset = -slotoffset - sizeof(PyObject *)*3; + } + type->tp_basicsize = slotoffset; type->tp_itemsize = ctx->base->tp_itemsize; type->tp_members = PyHeapType_GET_MEMBERS(et); @@ -3580,10 +3585,6 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases) goto fail; } if (dictoffset) { - if (dictoffset < 0) { - PyErr_SetString(PyExc_TypeError, "Negative __dictoffset__"); - goto fail; - } type->tp_dictoffset = dictoffset; if (PyDict_DelItemString((PyObject *)type->tp_dict, "__dictoffset__") < 0) goto fail; From 8a8593cbe2c6fd675df62751eafe0e304aafd110 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 29 Nov 2021 15:38:17 +0000 Subject: [PATCH 04/19] Fix up lazy dict creation logic to use managed dict pointers. --- Objects/dictobject.c | 5 +++-- Objects/typeobject.c | 13 ++++--------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 475d92d329828b..c0af3d9d9839ef 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -4961,7 +4961,7 @@ static int init_inline_values(PyObject *obj, PyTypeObject *tp) { assert(tp->tp_flags & Py_TPFLAGS_HEAPTYPE); - assert(tp->tp_dictoffset > 0); + // assert(type->tp_dictoffset > 0); -- TO DO Update this assert. assert(tp->tp_inline_values_offset > 0); PyDictKeysObject *keys = CACHED_KEYS(tp); assert(keys != NULL); @@ -5057,7 +5057,8 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values, return -1; } *((PyDictValues **)((char *)obj + tp->tp_inline_values_offset)) = NULL; - *((PyObject **) ((char *)obj + tp->tp_dictoffset)) = dict; + assert(_PyObject_DictPointer(obj) == ((PyObject **)obj)-3); + ((PyObject **)obj)[-3] = dict; return PyDict_SetItem(dict, name, value); } PyObject *old_value = values->values[ix]; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index ab8a4e299c3478..df69c908acce49 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2993,16 +2993,11 @@ type_new_descriptors(const type_new_ctx *ctx, PyTypeObject *type) type->tp_weaklistoffset = slotoffset; slotoffset += sizeof(PyObject *); } - if (type->tp_dictoffset > 0) { - type->tp_inline_values_offset = slotoffset; - slotoffset += sizeof(PyDictValues *); - } - else { - type->tp_inline_values_offset = 0; - } - + type->tp_inline_values_offset = 0; if (ctx->add_dict && ctx->base->tp_itemsize == 0) { type->tp_flags |= Py_TPFLAGS_MANAGED_DICT; + type->tp_inline_values_offset = slotoffset; + slotoffset += sizeof(PyDictValues *); type->tp_dictoffset = -slotoffset - sizeof(PyObject *)*3; } @@ -3208,7 +3203,7 @@ type_new_impl(type_new_ctx *ctx) fixup_slot_dispatchers(type); if (type->tp_inline_values_offset) { - assert(type->tp_dictoffset > 0); + // assert(type->tp_dictoffset > 0); -- TO DO Update this assert. PyHeapTypeObject *et = (PyHeapTypeObject*)type; et->ht_cached_keys = _PyDict_NewKeysForClass(); } From 34b5cead9e769d7c0eb693c9a9c7d4934cd7d7af Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 30 Nov 2021 12:44:32 +0000 Subject: [PATCH 05/19] Manage values pointer, placing them directly before managed dict pointers. --- Include/cpython/object.h | 1 - Include/internal/pycore_object.h | 12 +++++- Lib/test/test_sys.py | 2 +- Objects/dictobject.c | 39 +++++++++++--------- Objects/object.c | 33 ++++++----------- Objects/typeobject.c | 41 ++++++--------------- Python/ceval.c | 16 ++++---- Python/specialize.c | 63 +++++++++++++++----------------- Python/sysmodule.c | 5 +-- Tools/gdb/libpython.py | 8 ++-- 10 files changed, 99 insertions(+), 121 deletions(-) diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 3a8a256e3b9ae6..0c3957aff4b5d3 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -270,7 +270,6 @@ struct _typeobject { destructor tp_finalize; vectorcallfunc tp_vectorcall; - Py_ssize_t tp_inline_values_offset; }; /* The *real* layout of a type object when allocated on the heap */ diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 0b1ce53ff5e943..0c5effce94822e 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -172,7 +172,7 @@ static inline size_t _PyType_PreHeaderSize(PyTypeObject *tp) { return _PyType_IS_GC(tp) * sizeof(PyGC_Head) + - _PyType_HasFeature(tp, Py_TPFLAGS_MANAGED_DICT) * sizeof(PyObject *); + _PyType_HasFeature(tp, Py_TPFLAGS_MANAGED_DICT) * 2 * sizeof(PyObject *); } void _PyObject_GC_Link(PyObject *op); @@ -194,7 +194,15 @@ extern int _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values, PyObject *name, PyObject *value); PyObject * _PyObject_GetInstanceAttribute(PyObject *obj, PyDictValues *values, PyObject *name); -PyDictValues ** _PyObject_ValuesPointer(PyObject *); + + + +static inline PyDictValues **_PyObject_ValuesPointer(PyObject *obj) +{ + assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT); + return ((PyDictValues **)obj)-4; +} + PyObject ** _PyObject_DictPointer(PyObject *); int _PyObject_VisitInstanceAttributes(PyObject *self, visitproc visit, void *arg); void _PyObject_ClearInstanceAttributes(PyObject *self); diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index db8d0082085cb1..ed9b13d6286bd3 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1421,7 +1421,7 @@ def delx(self): del self.__x check((1,2,3), vsize('') + 3*self.P) # type # static type: PyTypeObject - fmt = 'P2nPI13Pl4Pn9Pn12PIPP' + fmt = '2PP2nPI13Pl4Pn9Pn12PIP' s = vsize(fmt) check(int, s) # class diff --git a/Objects/dictobject.c b/Objects/dictobject.c index c0af3d9d9839ef..eb33e504b46119 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -4962,7 +4962,7 @@ init_inline_values(PyObject *obj, PyTypeObject *tp) { assert(tp->tp_flags & Py_TPFLAGS_HEAPTYPE); // assert(type->tp_dictoffset > 0); -- TO DO Update this assert. - assert(tp->tp_inline_values_offset > 0); + assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT); PyDictKeysObject *keys = CACHED_KEYS(tp); assert(keys != NULL); if (keys->dk_usable > 1) { @@ -4979,7 +4979,7 @@ init_inline_values(PyObject *obj, PyTypeObject *tp) for (int i = 0; i < size; i++) { values->values[i] = NULL; } - *((PyDictValues **)((char *)obj + tp->tp_inline_values_offset)) = values; + ((PyDictValues **)obj)[-4] = values; return 0; } @@ -4990,7 +4990,7 @@ _PyObject_InitializeDict(PyObject *obj) if (tp->tp_dictoffset == 0) { return 0; } - if (tp->tp_inline_values_offset) { + if (tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) { return init_inline_values(obj, tp); } PyObject *dict; @@ -5032,7 +5032,7 @@ make_dict_from_instance_attributes(PyDictKeysObject *keys, PyDictValues *values) PyObject * _PyObject_MakeDictFromInstanceAttributes(PyObject *obj, PyDictValues *values) { - assert(Py_TYPE(obj)->tp_inline_values_offset != 0); + assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT); PyDictKeysObject *keys = CACHED_KEYS(Py_TYPE(obj)); return make_dict_from_instance_attributes(keys, values); } @@ -5042,10 +5042,10 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values, PyObject *name, PyObject *value) { assert(PyUnicode_CheckExact(name)); - PyTypeObject *tp = Py_TYPE(obj); PyDictKeysObject *keys = CACHED_KEYS(Py_TYPE(obj)); assert(keys != NULL); assert(values != NULL); + assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT); int ix = insert_into_dictkeys(keys, name); if (ix == DKIX_EMPTY) { if (value == NULL) { @@ -5056,7 +5056,7 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values, if (dict == NULL) { return -1; } - *((PyDictValues **)((char *)obj + tp->tp_inline_values_offset)) = NULL; + ((PyDictValues **)obj)[-4] = NULL; assert(_PyObject_DictPointer(obj) == ((PyObject **)obj)-3); ((PyObject **)obj)[-3] = dict; return PyDict_SetItem(dict, name, value); @@ -5103,15 +5103,17 @@ _PyObject_IsInstanceDictEmpty(PyObject *obj) if (tp->tp_dictoffset == 0) { return 1; } - PyDictValues **values_ptr = _PyObject_ValuesPointer(obj); - if (values_ptr && *values_ptr) { - PyDictKeysObject *keys = CACHED_KEYS(tp); - for (Py_ssize_t i = 0; i < keys->dk_nentries; i++) { - if ((*values_ptr)->values[i] != NULL) { - return 0; + if (tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) { + PyDictValues *values = *_PyObject_ValuesPointer(obj); + if (values) { + PyDictKeysObject *keys = CACHED_KEYS(tp); + for (Py_ssize_t i = 0; i < keys->dk_nentries; i++) { + if (values->values[i] != NULL) { + return 0; + } } + return 1; } - return 1; } PyObject **dictptr = _PyObject_DictPointer(obj); PyObject *dict = *dictptr; @@ -5126,7 +5128,7 @@ int _PyObject_VisitInstanceAttributes(PyObject *self, visitproc visit, void *arg) { PyTypeObject *tp = Py_TYPE(self); - assert(tp->tp_inline_values_offset); + assert(Py_TYPE(self)->tp_flags & Py_TPFLAGS_MANAGED_DICT); PyDictValues **values_ptr = _PyObject_ValuesPointer(self); if (*values_ptr == NULL) { return 0; @@ -5142,7 +5144,7 @@ void _PyObject_ClearInstanceAttributes(PyObject *self) { PyTypeObject *tp = Py_TYPE(self); - assert(tp->tp_inline_values_offset); + assert(Py_TYPE(self)->tp_flags & Py_TPFLAGS_MANAGED_DICT); PyDictValues **values_ptr = _PyObject_ValuesPointer(self); if (*values_ptr == NULL) { return; @@ -5157,7 +5159,7 @@ void _PyObject_FreeInstanceAttributes(PyObject *self) { PyTypeObject *tp = Py_TYPE(self); - assert(tp->tp_inline_values_offset); + assert(Py_TYPE(self)->tp_flags & Py_TPFLAGS_MANAGED_DICT); PyDictValues **values_ptr = _PyObject_ValuesPointer(self); if (*values_ptr == NULL) { return; @@ -5181,8 +5183,8 @@ PyObject_GenericGetDict(PyObject *obj, void *context) PyObject *dict = *dictptr; if (dict == NULL) { PyTypeObject *tp = Py_TYPE(obj); - PyDictValues **values_ptr = _PyObject_ValuesPointer(obj); - if (values_ptr && *values_ptr) { + PyDictValues **values_ptr = ((PyDictValues **)obj)-4; + if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) && *values_ptr) { *dictptr = dict = make_dict_from_instance_attributes(CACHED_KEYS(tp), *values_ptr); if (dict != NULL) { *values_ptr = NULL; @@ -5196,6 +5198,7 @@ PyObject_GenericGetDict(PyObject *obj, void *context) *dictptr = dict = PyDict_New(); } } + assert(dict || PyErr_Occurred()); Py_XINCREF(dict); return dict; } diff --git a/Objects/object.c b/Objects/object.c index b39bea3882d9a1..58c3b3dbfe75ff 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1106,8 +1106,11 @@ _PyObject_GetDictPtr(PyObject *obj) if (*dict_ptr != NULL) { return dict_ptr; } + if ((Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0) { + return dict_ptr; + } PyDictValues **values_ptr = _PyObject_ValuesPointer(obj); - if (values_ptr == NULL || *values_ptr == NULL) { + if (*values_ptr == NULL) { return dict_ptr; } PyObject *dict = _PyObject_MakeDictFromInstanceAttributes(obj, *values_ptr); @@ -1188,10 +1191,10 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method) } } } - PyDictValues **values_ptr = _PyObject_ValuesPointer(obj); - if (values_ptr && *values_ptr) { + PyDictValues *values; + if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) && (values = ((PyDictValues **)obj)[-4])) { assert(*_PyObject_DictPointer(obj) == NULL); - PyObject *attr = _PyObject_GetInstanceAttribute(obj, *values_ptr, name); + PyObject *attr = _PyObject_GetInstanceAttribute(obj, values, name); if (attr != NULL) { *method = attr; Py_XDECREF(descr); @@ -1243,17 +1246,6 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method) return 0; } -PyDictValues ** -_PyObject_ValuesPointer(PyObject *obj) -{ - PyTypeObject *tp = Py_TYPE(obj); - Py_ssize_t offset = tp->tp_inline_values_offset; - if (offset == 0) { - return NULL; - } - return (PyDictValues **) ((char *)obj + offset); -} - /* Generic GetAttr functions - put these in your tp_[gs]etattro slot. */ PyObject * @@ -1301,8 +1293,8 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name, } } if (dict == NULL) { - PyDictValues **values_ptr = _PyObject_ValuesPointer(obj); - if (values_ptr && *values_ptr) { + if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) && ((PyDictValues **)obj)[-4]) { + PyDictValues **values_ptr = _PyObject_ValuesPointer(obj); if (PyUnicode_CheckExact(name)) { assert(*_PyObject_DictPointer(obj) == NULL); res = _PyObject_GetInstanceAttribute(obj, *values_ptr, name); @@ -1414,9 +1406,8 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name, } if (dict == NULL) { - PyDictValues **values_ptr = _PyObject_ValuesPointer(obj); - if (values_ptr && *values_ptr) { - res = _PyObject_StoreInstanceAttribute(obj, *values_ptr, name, value); + if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) && ((PyDictValues **)obj)[-4]) { + res = _PyObject_StoreInstanceAttribute(obj, ((PyDictValues **)obj)[-4], name, value); } else { PyObject **dictptr = _PyObject_DictPointer(obj); @@ -1467,7 +1458,7 @@ PyObject_GenericSetDict(PyObject *obj, PyObject *value, void *context) PyObject **dictptr = _PyObject_GetDictPtr(obj); if (dictptr == NULL) { PyDictValues** values_ptr = _PyObject_ValuesPointer(obj); - if (values_ptr != NULL && *values_ptr != NULL) { + if ((Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT) && *values_ptr != NULL) { /* Was unable to convert to dict */ PyErr_NoMemory(); } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index df69c908acce49..740fa14eff52a6 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1154,6 +1154,7 @@ _PyType_AllocNoTrack(PyTypeObject *type, Py_ssize_t nitems) obj = (PyObject *)(alloc + presize); if (presize) { ((PyObject **)alloc)[0] = NULL; + ((PyObject **)alloc)[1] = NULL; _PyObject_GC_Link(obj); } memset(obj, '\0', size); @@ -1231,7 +1232,7 @@ subtype_traverse(PyObject *self, visitproc visit, void *arg) assert(base); } - if (type->tp_inline_values_offset) { + if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) { assert(type->tp_dictoffset); int err = _PyObject_VisitInstanceAttributes(self, visit, arg); if (err) { @@ -1300,7 +1301,7 @@ subtype_clear(PyObject *self) /* Clear the instance dict (if any), to break cycles involving only __dict__ slots (as in the case 'self.__dict__ is self'). */ - if (type->tp_inline_values_offset) { + if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) { _PyObject_ClearInstanceAttributes(self); } if (type->tp_dictoffset != base->tp_dictoffset) { @@ -1446,7 +1447,7 @@ subtype_dealloc(PyObject *self) } /* If we added a dict, DECREF it, or free inline values. */ - if (type->tp_inline_values_offset) { + if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) { _PyObject_FreeInstanceAttributes(self); } if (type->tp_dictoffset && !base->tp_dictoffset) { @@ -2244,18 +2245,10 @@ extra_ivars(PyTypeObject *type, PyTypeObject *base) return t_size != b_size || type->tp_itemsize != base->tp_itemsize; } - if (type->tp_inline_values_offset && base->tp_inline_values_offset == 0 && - type->tp_inline_values_offset + sizeof(PyDictValues *) == t_size && - type->tp_flags & Py_TPFLAGS_HEAPTYPE) - t_size -= sizeof(PyDictValues *); if (type->tp_weaklistoffset && base->tp_weaklistoffset == 0 && type->tp_weaklistoffset + sizeof(PyObject *) == t_size && type->tp_flags & Py_TPFLAGS_HEAPTYPE) t_size -= sizeof(PyObject *); - if (type->tp_dictoffset && base->tp_dictoffset == 0 && - type->tp_dictoffset + sizeof(PyObject *) == t_size && - type->tp_flags & Py_TPFLAGS_HEAPTYPE) - t_size -= sizeof(PyObject *); return t_size != b_size; } @@ -2993,11 +2986,9 @@ type_new_descriptors(const type_new_ctx *ctx, PyTypeObject *type) type->tp_weaklistoffset = slotoffset; slotoffset += sizeof(PyObject *); } - type->tp_inline_values_offset = 0; if (ctx->add_dict && ctx->base->tp_itemsize == 0) { + assert((type->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0); type->tp_flags |= Py_TPFLAGS_MANAGED_DICT; - type->tp_inline_values_offset = slotoffset; - slotoffset += sizeof(PyDictValues *); type->tp_dictoffset = -slotoffset - sizeof(PyObject *)*3; } @@ -3202,7 +3193,7 @@ type_new_impl(type_new_ctx *ctx) // Put the proper slots in place fixup_slot_dispatchers(type); - if (type->tp_inline_values_offset) { + if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) { // assert(type->tp_dictoffset > 0); -- TO DO Update this assert. PyHeapTypeObject *et = (PyHeapTypeObject*)type; et->ht_cached_keys = _PyDict_NewKeysForClass(); @@ -3590,8 +3581,7 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases) if (PyType_Ready(type) < 0) goto fail; - if (type->tp_inline_values_offset) { - assert(type->tp_dictoffset > 0); + if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) { res->ht_cached_keys = _PyDict_NewKeysForClass(); } @@ -4654,7 +4644,6 @@ compatible_with_tp_base(PyTypeObject *child) child->tp_itemsize == parent->tp_itemsize && child->tp_dictoffset == parent->tp_dictoffset && child->tp_weaklistoffset == parent->tp_weaklistoffset && - child->tp_inline_values_offset == parent->tp_inline_values_offset && ((child->tp_flags & Py_TPFLAGS_HAVE_GC) == (parent->tp_flags & Py_TPFLAGS_HAVE_GC)) && (child->tp_dealloc == subtype_dealloc || @@ -4674,8 +4663,6 @@ same_slots_added(PyTypeObject *a, PyTypeObject *b) size += sizeof(PyObject *); if (a->tp_weaklistoffset == size && b->tp_weaklistoffset == size) size += sizeof(PyObject *); - if (a->tp_inline_values_offset == size && b->tp_inline_values_offset == size) - size += sizeof(PyObject *); /* Check slots compliance */ if (!(a->tp_flags & Py_TPFLAGS_HEAPTYPE) || @@ -4828,15 +4815,13 @@ object_set_class(PyObject *self, PyObject *value, void *closure) if (compatible_for_assignment(oldto, newto, "__class__")) { /* Changing the class will change the implicit dict keys, * so we must materialize the dictionary first. */ - assert(oldto->tp_inline_values_offset == newto->tp_inline_values_offset); + assert((oldto->tp_flags & Py_TPFLAGS_MANAGED_DICT) == (newto->tp_flags & Py_TPFLAGS_MANAGED_DICT)); _PyObject_GetDictPtr(self); - PyDictValues** values_ptr = _PyObject_ValuesPointer(self); - if (values_ptr != NULL && *values_ptr != NULL) { + if (oldto->tp_flags & Py_TPFLAGS_MANAGED_DICT && *_PyObject_ValuesPointer(self)) { /* Was unable to convert to dict */ PyErr_NoMemory(); return -1; } - assert(_PyObject_ValuesPointer(self) == NULL || *_PyObject_ValuesPointer(self) == NULL); if (newto->tp_flags & Py_TPFLAGS_HEAPTYPE) { Py_INCREF(newto); } @@ -4982,15 +4967,14 @@ _PyObject_GetState(PyObject *obj, int required) assert(slotnames == Py_None || PyList_Check(slotnames)); if (required) { Py_ssize_t basicsize = PyBaseObject_Type.tp_basicsize; - if (Py_TYPE(obj)->tp_dictoffset) { + if (Py_TYPE(obj)->tp_dictoffset && + (Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0) + { basicsize += sizeof(PyObject *); } if (Py_TYPE(obj)->tp_weaklistoffset) { basicsize += sizeof(PyObject *); } - if (Py_TYPE(obj)->tp_inline_values_offset) { - basicsize += sizeof(PyDictValues *); - } if (slotnames != Py_None) { basicsize += sizeof(PyObject *) * PyList_GET_SIZE(slotnames); } @@ -5764,7 +5748,6 @@ inherit_special(PyTypeObject *type, PyTypeObject *base) COPYVAL(tp_itemsize); COPYVAL(tp_weaklistoffset); COPYVAL(tp_dictoffset); - COPYVAL(tp_inline_values_offset); #undef COPYVAL /* Setup fast subclass flags */ diff --git a/Python/ceval.c b/Python/ceval.c index 0427361a03a8bd..290c6471707ebb 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -3512,9 +3512,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr _PyAttrCache *cache1 = &caches[-1].attr; assert(cache1->tp_version != 0); DEOPT_IF(tp->tp_version_tag != cache1->tp_version, LOAD_ATTR); - assert(tp->tp_dictoffset > 0); - assert(tp->tp_inline_values_offset > 0); - PyDictValues *values = *(PyDictValues **)(((char *)owner) + tp->tp_inline_values_offset); + assert(tp->tp_dictoffset < 0); + assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT); + PyDictValues *values = ((PyDictValues **)owner)[-4]; DEOPT_IF(values == NULL, LOAD_ATTR); res = values->values[cache0->index]; DEOPT_IF(res == NULL, LOAD_ATTR); @@ -3614,9 +3614,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr _PyAttrCache *cache1 = &caches[-1].attr; assert(cache1->tp_version != 0); DEOPT_IF(tp->tp_version_tag != cache1->tp_version, STORE_ATTR); - assert(tp->tp_dictoffset > 0); - assert(tp->tp_inline_values_offset > 0); - PyDictValues *values = *(PyDictValues **)(((char *)owner) + tp->tp_inline_values_offset); + assert(tp->tp_dictoffset < 0); + assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT); + PyDictValues *values = ((PyDictValues **)owner)[-4]; DEOPT_IF(values == NULL, STORE_ATTR); STAT_INC(STORE_ATTR, hit); int index = cache0->index; @@ -4302,8 +4302,8 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr DEOPT_IF(self_cls->tp_version_tag != cache1->tp_version, LOAD_METHOD); assert(self_cls->tp_dictoffset > 0); - assert(self_cls->tp_inline_values_offset > 0); - PyDictObject *dict = *(PyDictObject **)(((char *)self) + self_cls->tp_dictoffset); + assert(self_cls->tp_flags & Py_TPFLAGS_MANAGED_DICT); + PyDictObject *dict = ((PyDictObject **)self)[-3]; DEOPT_IF(dict != NULL, LOAD_METHOD); DEOPT_IF(((PyHeapTypeObject *)self_cls)->ht_cached_keys->dk_version != cache1->dk_version_or_hint, LOAD_METHOD); STAT_INC(LOAD_METHOD, hit); diff --git a/Python/specialize.c b/Python/specialize.c index f5f12139df79b9..ce5436954e5807 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -498,7 +498,7 @@ specialize_module_load_attr( PyObject *value = NULL; PyObject *getattr; _Py_IDENTIFIER(__getattr__); - assert(owner->ob_type->tp_inline_values_offset == 0); + assert((owner->ob_type->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0); PyDictObject *dict = (PyDictObject *)m->md_dict; if (dict == NULL) { SPECIALIZATION_FAIL(opcode, SPEC_FAIL_NO_DICT); @@ -630,43 +630,40 @@ specialize_dict_access( SPECIALIZATION_FAIL(base_op, SPEC_FAIL_OUT_OF_RANGE); return 0; } + if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) { + // Virtual dictionary + PyDictKeysObject *keys = ((PyHeapTypeObject *)type)->ht_cached_keys; + assert(PyUnicode_CheckExact(name)); + Py_ssize_t index = _PyDictKeys_StringLookup(keys, name); + assert (index != DKIX_ERROR); + if (index != (uint16_t)index) { + SPECIALIZATION_FAIL(base_op, SPEC_FAIL_OUT_OF_RANGE); + return 0; + } + cache1->tp_version = type->tp_version_tag; + cache0->index = (uint16_t)index; + *instr = _Py_MAKECODEUNIT(values_op, _Py_OPARG(*instr)); + return 0; + } if (type->tp_dictoffset > 0) { PyObject **dictptr = (PyObject **) ((char *)owner + type->tp_dictoffset); PyDictObject *dict = (PyDictObject *)*dictptr; - if (type->tp_inline_values_offset && dict == NULL) { - // Virtual dictionary - PyDictKeysObject *keys = ((PyHeapTypeObject *)type)->ht_cached_keys; - assert(type->tp_inline_values_offset > 0); - assert(PyUnicode_CheckExact(name)); - Py_ssize_t index = _PyDictKeys_StringLookup(keys, name); - assert (index != DKIX_ERROR); - if (index != (uint16_t)index) { - SPECIALIZATION_FAIL(base_op, SPEC_FAIL_OUT_OF_RANGE); - return 0; - } - cache1->tp_version = type->tp_version_tag; - cache0->index = (uint16_t)index; - *instr = _Py_MAKECODEUNIT(values_op, _Py_OPARG(*instr)); + if (dict == NULL || !PyDict_CheckExact(dict)) { + SPECIALIZATION_FAIL(base_op, SPEC_FAIL_NO_DICT); return 0; } - else { - if (dict == NULL || !PyDict_CheckExact(dict)) { - SPECIALIZATION_FAIL(base_op, SPEC_FAIL_NO_DICT); - return 0; - } - // We found an instance with a __dict__. - PyObject *value = NULL; - Py_ssize_t hint = - _PyDict_GetItemHint(dict, name, -1, &value); - if (hint != (uint32_t)hint) { - SPECIALIZATION_FAIL(base_op, SPEC_FAIL_OUT_OF_RANGE); - return 0; - } - cache1->dk_version_or_hint = (uint32_t)hint; - cache1->tp_version = type->tp_version_tag; - *instr = _Py_MAKECODEUNIT(hint_op, _Py_OPARG(*instr)); - return 1; + // We found an instance with a __dict__. + PyObject *value = NULL; + Py_ssize_t hint = + _PyDict_GetItemHint(dict, name, -1, &value); + if (hint != (uint32_t)hint) { + SPECIALIZATION_FAIL(base_op, SPEC_FAIL_OUT_OF_RANGE); + return 0; } + cache1->dk_version_or_hint = (uint32_t)hint; + cache1->tp_version = type->tp_version_tag; + *instr = _Py_MAKECODEUNIT(hint_op, _Py_OPARG(*instr)); + return 1; } assert(type->tp_dictoffset == 0); /* No attribute in instance dictionary */ @@ -972,7 +969,7 @@ _Py_Specialize_LoadMethod(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, SPECIALIZATION_FAIL(LOAD_METHOD, load_method_fail_kind(kind)); goto fail; } - if (owner_cls->tp_inline_values_offset) { + if (owner_cls->tp_flags & Py_TPFLAGS_MANAGED_DICT) { PyObject **owner_dictptr = _PyObject_DictPointer(owner); assert(owner_dictptr); if (*owner_dictptr) { diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 13fae797b29c2c..af4f926f0e4080 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -1681,10 +1681,7 @@ _PySys_GetSizeOf(PyObject *o) return (size_t)-1; } - /* add gc_head size */ - if (_PyObject_IS_GC(o)) - return ((size_t)size) + sizeof(PyGC_Head); - return (size_t)size; + return (size_t)size + _PyType_PreHeaderSize(Py_TYPE(o)); } static PyObject * diff --git a/Tools/gdb/libpython.py b/Tools/gdb/libpython.py index a105e58b4af90a..8061df801e3886 100755 --- a/Tools/gdb/libpython.py +++ b/Tools/gdb/libpython.py @@ -83,6 +83,7 @@ def _sizeof_void_p(): # value computed later, see PyUnicodeObjectPtr.proxy() _is_pep393 = None +Py_TPFLAGS_MANAGED_DICT = (1 << 4) Py_TPFLAGS_HEAPTYPE = (1 << 9) Py_TPFLAGS_LONG_SUBCLASS = (1 << 24) Py_TPFLAGS_LIST_SUBCLASS = (1 << 25) @@ -523,12 +524,11 @@ def get_attr_dict(self): def get_keys_values(self): typeobj = self.type() - values_offset = int_from_int(typeobj.field('tp_inline_values_offset')) - if values_offset == 0: + has_values = int_from_int(typeobj.field('tp_flags')) & Py_TPFLAGS_MANAGED_DICT + if not has_values: return None - charptr = self._gdbval.cast(_type_char_ptr()) + values_offset PyDictValuesPtrPtr = gdb.lookup_type("PyDictValues").pointer().pointer() - valuesptr = charptr.cast(PyDictValuesPtrPtr) + valuesptr = self._gdbval.cast(PyDictValuesPtrPtr) - 4 values = valuesptr.dereference() if long(values) == 0: return None From e8c74abcb3647b59639ce84cc13acb152e865a81 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 30 Nov 2021 13:37:30 +0000 Subject: [PATCH 06/19] Refactor a bit. --- Include/internal/pycore_object.h | 6 ++++ Lib/test/test_sys.py | 4 +-- Objects/dictobject.c | 51 +++++++++++++++++++++----------- Objects/object.c | 19 ++++-------- Objects/typeobject.c | 13 ++++++-- 5 files changed, 58 insertions(+), 35 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 0c5effce94822e..bc899f022163a2 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -203,6 +203,12 @@ static inline PyDictValues **_PyObject_ValuesPointer(PyObject *obj) return ((PyDictValues **)obj)-4; } +static inline PyObject **_PyObject_ManagedDictPointer(PyObject *obj) +{ + assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT); + return ((PyObject **)obj)-3; +} + PyObject ** _PyObject_DictPointer(PyObject *); int _PyObject_VisitInstanceAttributes(PyObject *self, visitproc visit, void *arg); void _PyObject_ClearInstanceAttributes(PyObject *self); diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index ed9b13d6286bd3..13ae349046ac36 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1421,8 +1421,8 @@ def delx(self): del self.__x check((1,2,3), vsize('') + 3*self.P) # type # static type: PyTypeObject - fmt = '2PP2nPI13Pl4Pn9Pn12PIP' - s = vsize(fmt) + fmt = 'P2nPI13Pl4Pn9Pn12PIP' + s = vsize('2P' + fmt) check(int, s) # class s = vsize(fmt + # PyTypeObject diff --git a/Objects/dictobject.c b/Objects/dictobject.c index eb33e504b46119..d42e70c535548b 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -5103,6 +5103,7 @@ _PyObject_IsInstanceDictEmpty(PyObject *obj) if (tp->tp_dictoffset == 0) { return 1; } + PyObject **dictptr; if (tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) { PyDictValues *values = *_PyObject_ValuesPointer(obj); if (values) { @@ -5114,8 +5115,11 @@ _PyObject_IsInstanceDictEmpty(PyObject *obj) } return 1; } + dictptr = _PyObject_ManagedDictPointer(obj); + } + else { + dictptr = _PyObject_DictPointer(obj); } - PyObject **dictptr = _PyObject_DictPointer(obj); PyObject *dict = *dictptr; if (dict == NULL) { return 1; @@ -5174,31 +5178,44 @@ _PyObject_FreeInstanceAttributes(PyObject *self) PyObject * PyObject_GenericGetDict(PyObject *obj, void *context) { - PyObject **dictptr = _PyObject_DictPointer(obj); - if (dictptr == NULL) { - PyErr_SetString(PyExc_AttributeError, - "This object has no __dict__"); - return NULL; - } - PyObject *dict = *dictptr; - if (dict == NULL) { - PyTypeObject *tp = Py_TYPE(obj); - PyDictValues **values_ptr = ((PyDictValues **)obj)-4; - if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) && *values_ptr) { + PyObject *dict; + PyTypeObject *tp = Py_TYPE(obj); + if (_PyType_HasFeature(tp, Py_TPFLAGS_MANAGED_DICT)) { + PyDictValues **values_ptr = _PyObject_ValuesPointer(obj); + PyObject **dictptr = _PyObject_ManagedDictPointer(obj); + if (*values_ptr) { + assert(*dictptr == NULL); *dictptr = dict = make_dict_from_instance_attributes(CACHED_KEYS(tp), *values_ptr); if (dict != NULL) { *values_ptr = NULL; } } - else if (_PyType_HasFeature(tp, Py_TPFLAGS_HEAPTYPE) && CACHED_KEYS(tp)) { - dictkeys_incref(CACHED_KEYS(tp)); - *dictptr = dict = new_dict_with_shared_keys(CACHED_KEYS(tp)); + else if (*dictptr == NULL) { + *dictptr = dict = PyDict_New(); } else { - *dictptr = dict = PyDict_New(); + dict = *dictptr; + } + } + else { + PyObject **dictptr = _PyObject_DictPointer(obj); + if (dictptr == NULL) { + PyErr_SetString(PyExc_AttributeError, + "This object has no __dict__"); + return NULL; + } + dict = *dictptr; + if (dict == NULL) { + PyTypeObject *tp = Py_TYPE(obj); + if (_PyType_HasFeature(tp, Py_TPFLAGS_HEAPTYPE) && CACHED_KEYS(tp)) { + dictkeys_incref(CACHED_KEYS(tp)); + *dictptr = dict = new_dict_with_shared_keys(CACHED_KEYS(tp)); + } + else { + *dictptr = dict = PyDict_New(); + } } } - assert(dict || PyErr_Occurred()); Py_XINCREF(dict); return dict; } diff --git a/Objects/object.c b/Objects/object.c index 58c3b3dbfe75ff..bdfcf1666728b5 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1074,7 +1074,7 @@ _PyObject_DictPointer(PyObject *obj) PyTypeObject *tp = Py_TYPE(obj); if (tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) { - return ((PyObject **)obj)-3; + return _PyObject_ManagedDictPointer(obj); } dictoffset = tp->tp_dictoffset; if (dictoffset == 0) @@ -1099,27 +1099,20 @@ _PyObject_DictPointer(PyObject *obj) PyObject ** _PyObject_GetDictPtr(PyObject *obj) { - PyObject **dict_ptr = _PyObject_DictPointer(obj); - if (dict_ptr == NULL) { - return NULL; - } - if (*dict_ptr != NULL) { - return dict_ptr; - } if ((Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0) { - return dict_ptr; + return _PyObject_DictPointer(obj); } + PyObject **dict_ptr = _PyObject_ManagedDictPointer(obj); PyDictValues **values_ptr = _PyObject_ValuesPointer(obj); if (*values_ptr == NULL) { return dict_ptr; } + assert(*dict_ptr == NULL); PyObject *dict = _PyObject_MakeDictFromInstanceAttributes(obj, *values_ptr); if (dict == NULL) { PyErr_Clear(); return NULL; } - assert(*dict_ptr == NULL); - assert(*values_ptr != NULL); *values_ptr = NULL; *dict_ptr = dict; return dict_ptr; @@ -1406,8 +1399,8 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name, } if (dict == NULL) { - if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) && ((PyDictValues **)obj)[-4]) { - res = _PyObject_StoreInstanceAttribute(obj, ((PyDictValues **)obj)[-4], name, value); + if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) && *_PyObject_ValuesPointer(obj)) { + res = _PyObject_StoreInstanceAttribute(obj, *_PyObject_ValuesPointer(obj), name, value); } else { PyObject **dictptr = _PyObject_DictPointer(obj); diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 740fa14eff52a6..2fd93b61c0b2b0 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1448,9 +1448,17 @@ subtype_dealloc(PyObject *self) /* If we added a dict, DECREF it, or free inline values. */ if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) { - _PyObject_FreeInstanceAttributes(self); + PyObject **dictptr = _PyObject_ManagedDictPointer(self); + if (*dictptr != NULL) { + assert(*_PyObject_ValuesPointer(self) == NULL); + Py_DECREF(*dictptr); + *dictptr = NULL; + } + else { + _PyObject_FreeInstanceAttributes(self); + } } - if (type->tp_dictoffset && !base->tp_dictoffset) { + else if (type->tp_dictoffset && !base->tp_dictoffset) { PyObject **dictptr = _PyObject_DictPointer(self); if (dictptr != NULL) { PyObject *dict = *dictptr; @@ -3194,7 +3202,6 @@ type_new_impl(type_new_ctx *ctx) fixup_slot_dispatchers(type); if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) { - // assert(type->tp_dictoffset > 0); -- TO DO Update this assert. PyHeapTypeObject *et = (PyHeapTypeObject*)type; et->ht_cached_keys = _PyDict_NewKeysForClass(); } From 1bf13b028794b008eb716bf7d967be21b1127571 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 30 Nov 2021 15:42:08 +0000 Subject: [PATCH 07/19] Fix specialization of managed values. --- Python/specialize.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Python/specialize.c b/Python/specialize.c index ce5436954e5807..3fa6c09c58d64a 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -626,10 +626,6 @@ specialize_dict_access( assert(kind == NON_OVERRIDING || kind == NON_DESCRIPTOR || kind == ABSENT || kind == BUILTIN_CLASSMETHOD || kind == PYTHON_CLASSMETHOD); // No descriptor, or non overriding. - if (type->tp_dictoffset < 0) { - SPECIALIZATION_FAIL(base_op, SPEC_FAIL_OUT_OF_RANGE); - return 0; - } if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) { // Virtual dictionary PyDictKeysObject *keys = ((PyHeapTypeObject *)type)->ht_cached_keys; @@ -645,6 +641,10 @@ specialize_dict_access( *instr = _Py_MAKECODEUNIT(values_op, _Py_OPARG(*instr)); return 0; } + if (type->tp_dictoffset < 0) { + SPECIALIZATION_FAIL(base_op, SPEC_FAIL_OUT_OF_RANGE); + return 0; + } if (type->tp_dictoffset > 0) { PyObject **dictptr = (PyObject **) ((char *)owner + type->tp_dictoffset); PyDictObject *dict = (PyDictObject *)*dictptr; From a025dfb0150e5c459107d6951e5fcd25d8852bbd Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 30 Nov 2021 18:02:22 +0000 Subject: [PATCH 08/19] Convert hint-based load/store attr specialization target managed dict classes. --- Python/ceval.c | 9 ++++----- Python/specialize.c | 40 +++++++++++----------------------------- 2 files changed, 15 insertions(+), 34 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 290c6471707ebb..efa25aa0391bb9 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -3546,8 +3546,8 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr _PyAttrCache *cache1 = &caches[-1].attr; assert(cache1->tp_version != 0); DEOPT_IF(tp->tp_version_tag != cache1->tp_version, LOAD_ATTR); - assert(tp->tp_dictoffset > 0); - PyDictObject *dict = *(PyDictObject **)(((char *)owner) + tp->tp_dictoffset); + assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT); + PyDictObject *dict = *(PyDictObject **)_PyObject_ManagedDictPointer(owner); DEOPT_IF(dict == NULL, LOAD_ATTR); assert(PyDict_CheckExact((PyObject *)dict)); PyObject *name = GETITEM(names, cache0->original_oparg); @@ -3614,7 +3614,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr _PyAttrCache *cache1 = &caches[-1].attr; assert(cache1->tp_version != 0); DEOPT_IF(tp->tp_version_tag != cache1->tp_version, STORE_ATTR); - assert(tp->tp_dictoffset < 0); assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT); PyDictValues *values = ((PyDictValues **)owner)[-4]; DEOPT_IF(values == NULL, STORE_ATTR); @@ -3644,8 +3643,8 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr _PyAttrCache *cache1 = &caches[-1].attr; assert(cache1->tp_version != 0); DEOPT_IF(tp->tp_version_tag != cache1->tp_version, STORE_ATTR); - assert(tp->tp_dictoffset > 0); - PyDictObject *dict = *(PyDictObject **)(((char *)owner) + tp->tp_dictoffset); + assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT); + PyDictObject *dict = *(PyDictObject **)_PyObject_ManagedDictPointer(owner); DEOPT_IF(dict == NULL, STORE_ATTR); assert(PyDict_CheckExact((PyObject *)dict)); PyObject *name = GETITEM(names, cache0->original_oparg); diff --git a/Python/specialize.c b/Python/specialize.c index 3fa6c09c58d64a..ce8c63a017726b 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -447,6 +447,7 @@ initial_counter_value(void) { #define SPEC_FAIL_NON_OBJECT_SLOT 14 #define SPEC_FAIL_READ_ONLY 15 #define SPEC_FAIL_AUDITED_SLOT 16 +#define SPEC_FAIL_NOT_MANAGED_DICT 17 /* Methods */ @@ -626,7 +627,13 @@ specialize_dict_access( assert(kind == NON_OVERRIDING || kind == NON_DESCRIPTOR || kind == ABSENT || kind == BUILTIN_CLASSMETHOD || kind == PYTHON_CLASSMETHOD); // No descriptor, or non overriding. - if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) { + if ((type->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0) { + SPECIALIZATION_FAIL(base_op, SPEC_FAIL_NOT_MANAGED_DICT); + return 0; + } + PyObject **dictptr = _PyObject_ManagedDictPointer(owner); + PyDictObject *dict = (PyDictObject *)*dictptr; + if (dict == NULL) { // Virtual dictionary PyDictKeysObject *keys = ((PyHeapTypeObject *)type)->ht_cached_keys; assert(PyUnicode_CheckExact(name)); @@ -639,16 +646,9 @@ specialize_dict_access( cache1->tp_version = type->tp_version_tag; cache0->index = (uint16_t)index; *instr = _Py_MAKECODEUNIT(values_op, _Py_OPARG(*instr)); - return 0; - } - if (type->tp_dictoffset < 0) { - SPECIALIZATION_FAIL(base_op, SPEC_FAIL_OUT_OF_RANGE); - return 0; } - if (type->tp_dictoffset > 0) { - PyObject **dictptr = (PyObject **) ((char *)owner + type->tp_dictoffset); - PyDictObject *dict = (PyDictObject *)*dictptr; - if (dict == NULL || !PyDict_CheckExact(dict)) { + else { + if (!PyDict_CheckExact(dict)) { SPECIALIZATION_FAIL(base_op, SPEC_FAIL_NO_DICT); return 0; } @@ -663,26 +663,8 @@ specialize_dict_access( cache1->dk_version_or_hint = (uint32_t)hint; cache1->tp_version = type->tp_version_tag; *instr = _Py_MAKECODEUNIT(hint_op, _Py_OPARG(*instr)); - return 1; - } - assert(type->tp_dictoffset == 0); - /* No attribute in instance dictionary */ - switch(kind) { - case NON_OVERRIDING: - case BUILTIN_CLASSMETHOD: - case PYTHON_CLASSMETHOD: - SPECIALIZATION_FAIL(base_op, SPEC_FAIL_NON_OVERRIDING_DESCRIPTOR); - return 0; - case NON_DESCRIPTOR: - /* To do -- Optimize this case */ - SPECIALIZATION_FAIL(base_op, SPEC_FAIL_NOT_DESCRIPTOR); - return 0; - case ABSENT: - SPECIALIZATION_FAIL(base_op, SPEC_FAIL_EXPECTED_ERROR); - return 0; - default: - Py_UNREACHABLE(); } + return 1; } int From 5a012a8ad0d14a1172c630c3656ea61546215bb6 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 30 Nov 2021 19:10:29 +0000 Subject: [PATCH 09/19] Specialize LOAD_METHOD for managed dict objects. --- Python/ceval.c | 1 - Python/specialize.c | 9 +-------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index efa25aa0391bb9..a71e1fa9e584cf 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -4300,7 +4300,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr _PyObjectCache *cache2 = &caches[-2].obj; DEOPT_IF(self_cls->tp_version_tag != cache1->tp_version, LOAD_METHOD); - assert(self_cls->tp_dictoffset > 0); assert(self_cls->tp_flags & Py_TPFLAGS_MANAGED_DICT); PyDictObject *dict = ((PyDictObject **)self)[-3]; DEOPT_IF(dict != NULL, LOAD_METHOD); diff --git a/Python/specialize.c b/Python/specialize.c index ce8c63a017726b..7a5e60b0a68f49 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -936,12 +936,6 @@ _Py_Specialize_LoadMethod(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, } goto success; } - // Technically this is fine for bound method calls, but it's uncommon and - // slightly slower at runtime to get dict. - if (owner_cls->tp_dictoffset < 0) { - SPECIALIZATION_FAIL(LOAD_METHOD, SPEC_FAIL_OUT_OF_RANGE); - goto fail; - } PyObject *descr = NULL; DesciptorClassification kind = 0; @@ -952,8 +946,7 @@ _Py_Specialize_LoadMethod(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name, goto fail; } if (owner_cls->tp_flags & Py_TPFLAGS_MANAGED_DICT) { - PyObject **owner_dictptr = _PyObject_DictPointer(owner); - assert(owner_dictptr); + PyObject **owner_dictptr = _PyObject_ManagedDictPointer(owner); if (*owner_dictptr) { SPECIALIZATION_FAIL(LOAD_METHOD, SPEC_FAIL_IS_ATTR); goto fail; From 14d41aba186d92ac4582f569e220db67c8326232 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 1 Dec 2021 12:28:34 +0000 Subject: [PATCH 10/19] Use newer API internally. --- Objects/exceptions.c | 5 +---- Objects/object.c | 5 +++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Objects/exceptions.c b/Objects/exceptions.c index a5459da89a073d..1dea8b0d7dd5a3 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -3453,7 +3453,6 @@ _PyErr_TrySetFromCause(const char *format, ...) PyObject* msg_prefix; PyObject *exc, *val, *tb; PyTypeObject *caught_type; - PyObject **dictptr; PyObject *instance_args; Py_ssize_t num_args, caught_type_size, base_exc_size; PyObject *new_exc, *new_val, *new_tb; @@ -3499,9 +3498,7 @@ _PyErr_TrySetFromCause(const char *format, ...) } /* Ensure the instance dict is also empty */ - dictptr = _PyObject_GetDictPtr(val); - if (dictptr != NULL && *dictptr != NULL && - PyDict_GET_SIZE(*dictptr) > 0) { + if (!_PyObject_IsInstanceDictEmpty(val)) { /* While we could potentially copy a non-empty instance dictionary * to the replacement exception, for now we take the more * conservative path of leaving exceptions with attributes set diff --git a/Objects/object.c b/Objects/object.c index bdfcf1666728b5..754f96bab74e3e 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1450,8 +1450,9 @@ PyObject_GenericSetDict(PyObject *obj, PyObject *value, void *context) { PyObject **dictptr = _PyObject_GetDictPtr(obj); if (dictptr == NULL) { - PyDictValues** values_ptr = _PyObject_ValuesPointer(obj); - if ((Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT) && *values_ptr != NULL) { + if (_PyType_HasFeature(Py_TYPE(obj), Py_TPFLAGS_MANAGED_DICT) && + *_PyObject_ValuesPointer(obj) != NULL) + { /* Was unable to convert to dict */ PyErr_NoMemory(); } From 123171a083f4a9b084f98e18f08f5cdd7f5b8c1d Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 1 Dec 2021 14:06:46 +0000 Subject: [PATCH 11/19] Add NEWS. --- .../Core and Builtins/2021-12-01-14-06-36.bpo-45947.1XPPm_.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-12-01-14-06-36.bpo-45947.1XPPm_.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-12-01-14-06-36.bpo-45947.1XPPm_.rst b/Misc/NEWS.d/next/Core and Builtins/2021-12-01-14-06-36.bpo-45947.1XPPm_.rst new file mode 100644 index 00000000000000..5156bd35369e1a --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2021-12-01-14-06-36.bpo-45947.1XPPm_.rst @@ -0,0 +1,3 @@ +Place pointers to dict and values immediately before GC header. This reduces +number of dependent memory loads to access either dict or values from 3 to +1. From 48d6a5809f970a741ec2ee064a255c1822bd8955 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 1 Dec 2021 14:26:18 +0000 Subject: [PATCH 12/19] Use inline functions instead of magic constants. --- Objects/dictobject.c | 7 +++---- Objects/object.c | 8 ++++++-- Python/ceval.c | 4 ++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/Objects/dictobject.c b/Objects/dictobject.c index d42e70c535548b..7ce4b9069f77ef 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -4979,7 +4979,7 @@ init_inline_values(PyObject *obj, PyTypeObject *tp) for (int i = 0; i < size; i++) { values->values[i] = NULL; } - ((PyDictValues **)obj)[-4] = values; + *_PyObject_ValuesPointer(obj) = values; return 0; } @@ -5056,9 +5056,8 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values, if (dict == NULL) { return -1; } - ((PyDictValues **)obj)[-4] = NULL; - assert(_PyObject_DictPointer(obj) == ((PyObject **)obj)-3); - ((PyObject **)obj)[-3] = dict; + *_PyObject_ValuesPointer(obj) = NULL; + *_PyObject_ManagedDictPointer(obj) = dict; return PyDict_SetItem(dict, name, value); } PyObject *old_value = values->values[ix]; diff --git a/Objects/object.c b/Objects/object.c index 754f96bab74e3e..a1c2e16b6fa2ce 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1185,7 +1185,9 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method) } } PyDictValues *values; - if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) && (values = ((PyDictValues **)obj)[-4])) { + if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) && + (values = *_PyObject_ValuesPointer(obj))) + { assert(*_PyObject_DictPointer(obj) == NULL); PyObject *attr = _PyObject_GetInstanceAttribute(obj, values, name); if (attr != NULL) { @@ -1286,7 +1288,9 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name, } } if (dict == NULL) { - if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) && ((PyDictValues **)obj)[-4]) { + if ((tp->tp_flags & Py_TPFLAGS_MANAGED_DICT) && + *_PyObject_ValuesPointer(obj)) + { PyDictValues **values_ptr = _PyObject_ValuesPointer(obj); if (PyUnicode_CheckExact(name)) { assert(*_PyObject_DictPointer(obj) == NULL); diff --git a/Python/ceval.c b/Python/ceval.c index a9a0036a77419c..ff8e6b13e39ad3 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -3601,7 +3601,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr DEOPT_IF(tp->tp_version_tag != cache1->tp_version, LOAD_ATTR); assert(tp->tp_dictoffset < 0); assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT); - PyDictValues *values = ((PyDictValues **)owner)[-4]; + PyDictValues *values = *_PyObject_ValuesPointer(owner); DEOPT_IF(values == NULL, LOAD_ATTR); res = values->values[cache0->index]; DEOPT_IF(res == NULL, LOAD_ATTR); @@ -4385,7 +4385,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr DEOPT_IF(self_cls->tp_version_tag != cache1->tp_version, LOAD_METHOD); assert(self_cls->tp_flags & Py_TPFLAGS_MANAGED_DICT); - PyDictObject *dict = ((PyDictObject **)self)[-3]; + PyDictObject *dict = *(PyDictObject**)_PyObject_ManagedDictPointer(self); DEOPT_IF(dict != NULL, LOAD_METHOD); DEOPT_IF(((PyHeapTypeObject *)self_cls)->ht_cached_keys->dk_version != cache1->dk_version_or_hint, LOAD_METHOD); STAT_INC(LOAD_METHOD, hit); From 79e61bf5f643fe0e8fae084ce4b09378e2f74abe Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 1 Dec 2021 16:58:26 +0000 Subject: [PATCH 13/19] Remove unsafe _PyObject_GC_Malloc() function. --- Include/cpython/objimpl.h | 2 -- Lib/test/test_stable_abi_ctypes.py | 1 - Misc/stable_abi.txt | 3 --- Modules/gcmodule.c | 43 ++++++++++-------------------- PC/python3dll.c | 1 - 5 files changed, 14 insertions(+), 36 deletions(-) diff --git a/Include/cpython/objimpl.h b/Include/cpython/objimpl.h index 00bbd0cad6d405..4a905c25cc8453 100644 --- a/Include/cpython/objimpl.h +++ b/Include/cpython/objimpl.h @@ -90,8 +90,6 @@ PyAPI_FUNC(int) PyObject_IS_GC(PyObject *obj); # define _PyGC_FINALIZED(o) PyObject_GC_IsFinalized(o) #endif -PyAPI_FUNC(PyObject *) _PyObject_GC_Malloc(size_t size); - /* Test if a type supports weak references */ #define PyType_SUPPORTS_WEAKREFS(t) ((t)->tp_weaklistoffset > 0) diff --git a/Lib/test/test_stable_abi_ctypes.py b/Lib/test/test_stable_abi_ctypes.py index 1e27bcaf889a21..d0cd5c20dd5f42 100644 --- a/Lib/test/test_stable_abi_ctypes.py +++ b/Lib/test/test_stable_abi_ctypes.py @@ -817,7 +817,6 @@ def test_available_symbols(self): "_PyErr_BadInternalCall", "_PyObject_CallFunction_SizeT", "_PyObject_CallMethod_SizeT", - "_PyObject_GC_Malloc", "_PyObject_GC_New", "_PyObject_GC_NewVar", "_PyObject_GC_Resize", diff --git a/Misc/stable_abi.txt b/Misc/stable_abi.txt index 9f5a85bdec40f0..de6caa8c807467 100644 --- a/Misc/stable_abi.txt +++ b/Misc/stable_abi.txt @@ -1577,9 +1577,6 @@ function _PyObject_CallFunction_SizeT function _PyObject_CallMethod_SizeT added 3.2 abi_only -function _PyObject_GC_Malloc - added 3.2 - abi_only function _PyObject_GC_New added 3.2 abi_only diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index cf0e0341ac6be6..1808057a650e98 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -2254,46 +2254,30 @@ _PyObject_GC_Link(PyObject *op) } } - - -PyObject * -_PyObject_GC_Malloc(size_t basicsize) +static PyObject * +gc_alloc(size_t basicsize, size_t presize) { PyThreadState *tstate = _PyThreadState_GET(); - GCState *gcstate = &tstate->interp->gc; - if (basicsize > PY_SSIZE_T_MAX - sizeof(PyGC_Head)) { + if (basicsize > PY_SSIZE_T_MAX - presize) { return _PyErr_NoMemory(tstate); } - size_t size = sizeof(PyGC_Head) + basicsize; - - PyGC_Head *g = (PyGC_Head *)PyObject_Malloc(size); - if (g == NULL) { + size_t size = presize + basicsize; + char *mem = PyObject_Malloc(size); + if (mem == NULL) { return _PyErr_NoMemory(tstate); } - assert(((uintptr_t)g & sizeof(uintptr_t)) == 0); // g must be correctly aligned - - g->_gc_next = 0; - g->_gc_prev = 0; - gcstate->generations[0].count++; /* number of allocated GC objects */ - if (gcstate->generations[0].count > gcstate->generations[0].threshold && - gcstate->enabled && - gcstate->generations[0].threshold && - !gcstate->collecting && - !_PyErr_Occurred(tstate)) - { - gcstate->collecting = 1; - gc_collect_generations(tstate); - gcstate->collecting = 0; - } - PyObject *op = FROM_GC(g); + ((PyObject **)mem)[0] = NULL; + ((PyObject **)mem)[1] = NULL; + PyObject *op = (PyObject *)(mem + presize); + _PyObject_GC_Link(op); return op; } - PyObject * _PyObject_GC_New(PyTypeObject *tp) { - PyObject *op = _PyObject_GC_Malloc(_PyObject_SIZE(tp)); + size_t presize = _PyType_PreHeaderSize(tp); + PyObject *op = gc_alloc(_PyObject_SIZE(tp), presize); if (op == NULL) { return NULL; } @@ -2311,8 +2295,9 @@ _PyObject_GC_NewVar(PyTypeObject *tp, Py_ssize_t nitems) PyErr_BadInternalCall(); return NULL; } + size_t presize = _PyType_PreHeaderSize(tp); size = _PyObject_VAR_SIZE(tp, nitems); - op = (PyVarObject *) _PyObject_GC_Malloc(size); + op = (PyVarObject *)gc_alloc(size, presize); if (op == NULL) { return NULL; } diff --git a/PC/python3dll.c b/PC/python3dll.c index d2a87070de5cce..6e469357ede506 100755 --- a/PC/python3dll.c +++ b/PC/python3dll.c @@ -28,7 +28,6 @@ EXPORT_FUNC(_PyArg_VaParseTupleAndKeywords_SizeT) EXPORT_FUNC(_PyErr_BadInternalCall) EXPORT_FUNC(_PyObject_CallFunction_SizeT) EXPORT_FUNC(_PyObject_CallMethod_SizeT) -EXPORT_FUNC(_PyObject_GC_Malloc) EXPORT_FUNC(_PyObject_GC_New) EXPORT_FUNC(_PyObject_GC_NewVar) EXPORT_FUNC(_PyObject_GC_Resize) From ce0f65bb092352ff911c24b2c0a2df65bdb7ddaa Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 2 Dec 2021 17:44:14 +0000 Subject: [PATCH 14/19] Remove invalid assert. --- Tools/gdb/libpython.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Tools/gdb/libpython.py b/Tools/gdb/libpython.py index 8061df801e3886..3c74bf5ab2ec0d 100755 --- a/Tools/gdb/libpython.py +++ b/Tools/gdb/libpython.py @@ -508,7 +508,6 @@ def get_attr_dict(self): tsize = -tsize size = _PyObject_VAR_SIZE(typeobj, tsize) dictoffset += size - assert dictoffset > 0 assert dictoffset % _sizeof_void_p() == 0 dictptr = self._gdbval.cast(_type_char_ptr()) + dictoffset From 98ddaedc03eaef52b08e5662db2b98a026b0263d Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 3 Dec 2021 11:36:49 +0000 Subject: [PATCH 15/19] Add comment explaning use of Py_TPFLAGS_MANAGED_DICT. --- Include/object.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Include/object.h b/Include/object.h index 2faca60b58a2ab..e5544e8b588ed7 100644 --- a/Include/object.h +++ b/Include/object.h @@ -334,6 +334,10 @@ given type object has a specified feature. #ifndef Py_LIMITED_API +/* Placement of dict (and values) pointers are managed by the VM, not by the type. + * The VM will automatically set tp_dictoffset. Should not be used for variable sized + * classes, such as classes that extend tuple. + */ #define Py_TPFLAGS_MANAGED_DICT (1 << 4) /* Set if instances of the type object are treated as sequences for pattern matching */ From 0f376b500c7668f56293556d65f21caf2e9d3c8d Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 7 Dec 2021 11:55:23 +0000 Subject: [PATCH 16/19] Use inline function, not magic constant. --- Python/ceval.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/ceval.c b/Python/ceval.c index ff8e6b13e39ad3..b91a862c995016 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -3702,7 +3702,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr assert(cache1->tp_version != 0); DEOPT_IF(tp->tp_version_tag != cache1->tp_version, STORE_ATTR); assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT); - PyDictValues *values = ((PyDictValues **)owner)[-4]; + PyDictValues *values = *_PyObject_ValuesPointer(owner); DEOPT_IF(values == NULL, STORE_ATTR); STAT_INC(STORE_ATTR, hit); int index = cache0->index; From d7248123cd1b2218fcf11ffdff15b0700d9455ad Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 7 Dec 2021 13:18:14 +0000 Subject: [PATCH 17/19] Tidy up struct layout a bit. --- Modules/_testcapimodule.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 67c03f6b362223..56d394985eb7c5 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -5930,15 +5930,15 @@ static PyMethodDef TestMethods[] = { #if (defined(__linux__) || defined(__FreeBSD__)) && defined(__GNUC__) {"test_pep3118_obsolete_write_locks", (PyCFunction)test_pep3118_obsolete_write_locks, METH_NOARGS}, #endif - {"getbuffer_with_null_view", getbuffer_with_null_view, METH_O}, - {"PyBuffer_SizeFromFormat", test_PyBuffer_SizeFromFormat, METH_VARARGS}, - {"test_buildvalue_N", test_buildvalue_N, METH_NOARGS}, - {"negative_dictoffset", negative_dictoffset, METH_NOARGS}, + {"getbuffer_with_null_view", getbuffer_with_null_view, METH_O}, + {"PyBuffer_SizeFromFormat", test_PyBuffer_SizeFromFormat, METH_VARARGS}, + {"test_buildvalue_N", test_buildvalue_N, METH_NOARGS}, + {"negative_dictoffset", negative_dictoffset, METH_NOARGS}, {"test_buildvalue_issue38913", test_buildvalue_issue38913, METH_NOARGS}, - {"get_args", get_args, METH_VARARGS}, + {"get_args", get_args, METH_VARARGS}, {"test_get_statictype_slots", test_get_statictype_slots, METH_NOARGS}, {"test_get_type_name", test_get_type_name, METH_NOARGS}, - {"test_get_type_qualname", test_get_type_qualname, METH_NOARGS}, + {"test_get_type_qualname", test_get_type_qualname, METH_NOARGS}, {"test_type_from_ephemeral_spec", test_type_from_ephemeral_spec, METH_NOARGS}, {"get_kwargs", (PyCFunction)(void(*)(void))get_kwargs, METH_VARARGS|METH_KEYWORDS}, From 9435bae7d4aca7f2db11f8478f049a338f3ba132 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 7 Dec 2021 13:40:04 +0000 Subject: [PATCH 18/19] Tidy up gdb/libpython.py. --- Tools/gdb/libpython.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/gdb/libpython.py b/Tools/gdb/libpython.py index 3c74bf5ab2ec0d..a0a95e3fc63cba 100755 --- a/Tools/gdb/libpython.py +++ b/Tools/gdb/libpython.py @@ -83,8 +83,8 @@ def _sizeof_void_p(): # value computed later, see PyUnicodeObjectPtr.proxy() _is_pep393 = None -Py_TPFLAGS_MANAGED_DICT = (1 << 4) -Py_TPFLAGS_HEAPTYPE = (1 << 9) +Py_TPFLAGS_MANAGED_DICT = (1 << 4) +Py_TPFLAGS_HEAPTYPE = (1 << 9) Py_TPFLAGS_LONG_SUBCLASS = (1 << 24) Py_TPFLAGS_LIST_SUBCLASS = (1 << 25) Py_TPFLAGS_TUPLE_SUBCLASS = (1 << 26) From 302f46f4145437326b7ed2fb15a02f3ca3a50122 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 7 Dec 2021 13:52:58 +0000 Subject: [PATCH 19/19] Fix whitespace. --- Include/internal/pycore_object.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index bc899f022163a2..9041a4dc8a3ce5 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -195,8 +195,6 @@ extern int _PyObject_StoreInstanceAttribute(PyObject *obj, PyDictValues *values, PyObject * _PyObject_GetInstanceAttribute(PyObject *obj, PyDictValues *values, PyObject *name); - - static inline PyDictValues **_PyObject_ValuesPointer(PyObject *obj) { assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT);