From 8ca6cd042b1e1fe91e2751c5abf3204631c749c0 Mon Sep 17 00:00:00 2001 From: Koudai Aono Date: Sun, 23 Aug 2026 15:06:36 +0900 Subject: [PATCH] gh-156250: Speed up inherited mapping assignment slot calls --- Lib/test/test_descr.py | 79 +++++++++++++++++++ ...-08-23-13-19-43.gh-issue-156250.xJMn6W.rst | 2 + Objects/typeobject.c | 60 +++++++++++--- 3 files changed, 130 insertions(+), 11 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-08-23-13-19-43.gh-issue-156250.xJMn6W.rst diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index 7ac22455541fe2..816741512b81f9 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -5145,6 +5145,85 @@ class X(dict): self.assertEqual(x["y"], 42) self.assertEqual(x, -x) + def test_mixing_mapping_assignment_slot_wrappers(self): + class SetOnly(dict): + def __setitem__(self, key, value): + super().__setitem__(key, value + 1) + + class DelOnly(dict): + def __delitem__(self, key): + super().__delitem__(key) + + obj = SetOnly() + obj["x"] = 1 + self.assertEqual(obj, {"x": 2}) + del obj["x"] + self.assertEqual(obj, {}) + with self.assertRaises(KeyError) as cm: + del obj["missing"] + self.assertEqual(cm.exception.args, ("missing",)) + + obj = DelOnly() + obj["x"] = 1 + self.assertEqual(obj, {"x": 1}) + del obj["x"] + self.assertEqual(obj, {}) + + class SetOnlyList(list): + def __setitem__(self, key, value): + super().__setitem__(key, value) + + obj = SetOnlyList([1, 2, 3]) + del obj[1:] + self.assertEqual(obj, [1]) + + def test_mapping_assignment_slot_wrapper_fallback(self): + class ReturnSetWrapper: + def __get__(self, obj, owner=None): + return dict.__setitem__ + + class ReturnDeleteWrapper: + def __get__(self, obj, owner=None): + return dict.__delitem__ + + class X(dict): + __setitem__ = ReturnSetWrapper() + + def __delitem__(self, key): + super().__delitem__(key) + + with self.assertRaisesRegex(TypeError, "requires a 'dict' object"): + X()["key"] = "value" + + class Y(dict): + def __setitem__(self, key, value): + super().__setitem__(key, value) + + __delitem__ = ReturnDeleteWrapper() + + with self.assertRaisesRegex(TypeError, "requires a 'dict' object"): + del Y()["key"] + + from collections import OrderedDict + + class WrongOwner(dict): + __setitem__ = OrderedDict.__setitem__ + + def __delitem__(self, key): + super().__delitem__(key) + + with self.assertRaises(TypeError): + WrongOwner()["key"] = "value" + + class WrongOperation(dict): + __setitem__ = dict.__delitem__ + + def __delitem__(self, key): + super().__delitem__(key) + + with self.assertRaises(TypeError): + WrongOperation()["key"] = "value" + def test_wrong_class_slot_wrapper(self): # Check bpo-37619: a wrapper descriptor taken from the wrong class # should raise an exception instead of silently being ignored diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-08-23-13-19-43.gh-issue-156250.xJMn6W.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-23-13-19-43.gh-issue-156250.xJMn6W.rst new file mode 100644 index 00000000000000..11ab607d79fe91 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-23-13-19-43.gh-issue-156250.xJMn6W.rst @@ -0,0 +1,2 @@ +Speed up inherited C implementations of item assignment and deletion on +Python subclasses of types that use the mapping assignment slot. diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 572e302df8d80d..1d566998652b57 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3,6 +3,7 @@ #include "Python.h" #include "pycore_abstract.h" // _PySequence_IterSearch() #include "pycore_call.h" // _PyObject_VectorcallTstate() +#include "pycore_ceval.h" // _Py_EnterRecursiveCallTstate() #include "pycore_code.h" // CO_FAST_FREE #include "pycore_descrobject.h" // _PyMember_GetOffset() #include "pycore_dict.h" // _PyDict_KeysSize() @@ -10647,21 +10648,58 @@ SLOT1(slot_mp_subscript, __getitem__, PyObject *) static int slot_mp_ass_subscript(PyObject *self, PyObject *key, PyObject *value) { - PyObject *stack[3]; - PyObject *res; - - stack[0] = self; - stack[1] = key; - if (value == NULL) { - res = vectorcall_method(&_Py_ID(__delitem__), stack, 2); + PyThreadState *tstate = _PyThreadState_GET(); + PyObject *name = value == NULL ? &_Py_ID(__delitem__) : &_Py_ID(__setitem__); + _PyCStackRef cref; + _PyThreadState_PushCStackRef(tstate, &cref); + int unbound = lookup_method(self, name, &cref.ref); + if (unbound < 0) { + _PyThreadState_PopCStackRef(tstate, &cref); + return -1; } - else { - stack[2] = value; - res = vectorcall_method(&_Py_ID(__setitem__), stack, 3); + + PyObject *func = PyStackRef_AsPyObjectBorrow(cref.ref); + if (unbound && Py_IS_TYPE(func, &PyWrapperDescr_Type)) { + PyWrapperDescrObject *descr = (PyWrapperDescrObject *)func; + wrapperfunc expected_wrapper = + value == NULL ? wrap_delitem : wrap_objobjargproc; + /* __setitem__ and __delitem__ share mp_ass_subscript. An override of + either installs this dispatcher for both methods. Avoid the generic + call path when lookup finds the inherited C slot wrapper. */ + if (descr->d_base->name_strobj == name && + descr->d_base->wrapper == expected_wrapper && + PyObject_TypeCheck(self, PyDescr_TYPE(descr))) + { + /* Keep recursion handling in sync with _PyObject_MakeTpCall(). */ + if (_Py_EnterRecursiveCallTstate( + tstate, " while calling a Python object")) + { + _PyThreadState_PopCStackRef(tstate, &cref); + return -1; + } + objobjargproc slot = (objobjargproc)descr->d_wrapped; + int result = slot(self, key, value); + _Py_LeaveRecursiveCallTstate(tstate); + if (result != -1 && _PyErr_Occurred(tstate)) { + /* Match the result check performed by the generic call path. */ + (void)_Py_CheckFunctionResult( + tstate, func, Py_NewRef(Py_None), NULL); + _PyThreadState_PopCStackRef(tstate, &cref); + return -1; + } + int error = result == -1 && _PyErr_Occurred(tstate); + _PyThreadState_PopCStackRef(tstate, &cref); + return error ? -1 : 0; + } } - if (res == NULL) + PyObject *stack[3] = {self, key, value}; + PyObject *res = vectorcall_unbound( + tstate, unbound, func, stack, value == NULL ? 2 : 3); + _PyThreadState_PopCStackRef(tstate, &cref); + if (res == NULL) { return -1; + } Py_DECREF(res); return 0; }