From 9e4f2f3a6b8ee995c365e86d976937c141d867f8 Mon Sep 17 00:00:00 2001 From: Inada Naoki Date: Fri, 12 Apr 2019 16:11:28 +0900 Subject: [PATCH] bpo-20180: Use argument clinic for dict.pop() and dict.popitem() (GH-12792) --- .../2019-04-12-15-49-15.bpo-20180.KUqVk7.rst | 2 + Objects/clinic/dictobject.c.h | 59 +++++++++++++- Objects/dictobject.c | 77 ++++++++++--------- 3 files changed, 102 insertions(+), 36 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-04-12-15-49-15.bpo-20180.KUqVk7.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-04-12-15-49-15.bpo-20180.KUqVk7.rst b/Misc/NEWS.d/next/Core and Builtins/2019-04-12-15-49-15.bpo-20180.KUqVk7.rst new file mode 100644 index 00000000000..8c9067081aa --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2019-04-12-15-49-15.bpo-20180.KUqVk7.rst @@ -0,0 +1,2 @@ +``dict.pop()`` is now up to 33% faster thanks to Argument Clinic. Patch by +Inada Naoki. diff --git a/Objects/clinic/dictobject.c.h b/Objects/clinic/dictobject.c.h index 713781ce880..b87244d8734 100644 --- a/Objects/clinic/dictobject.c.h +++ b/Objects/clinic/dictobject.c.h @@ -116,6 +116,63 @@ exit: return return_value; } +PyDoc_STRVAR(dict_pop__doc__, +"pop($self, key, default=None, /)\n" +"--\n" +"\n" +"Remove specified key and return the corresponding value.\n" +"\n" +"If key is not found, default is returned if given, otherwise KeyError is raised"); + +#define DICT_POP_METHODDEF \ + {"pop", (PyCFunction)(void(*)(void))dict_pop, METH_FASTCALL, dict_pop__doc__}, + +static PyObject * +dict_pop_impl(PyDictObject *self, PyObject *key, PyObject *default_value); + +static PyObject * +dict_pop(PyDictObject *self, PyObject *const *args, Py_ssize_t nargs) +{ + PyObject *return_value = NULL; + PyObject *key; + PyObject *default_value = NULL; + + if (!_PyArg_CheckPositional("pop", nargs, 1, 2)) { + goto exit; + } + key = args[0]; + if (nargs < 2) { + goto skip_optional; + } + default_value = args[1]; +skip_optional: + return_value = dict_pop_impl(self, key, default_value); + +exit: + return return_value; +} + +PyDoc_STRVAR(dict_popitem__doc__, +"popitem($self, /)\n" +"--\n" +"\n" +"Remove and return a (key, value) pair as a 2-tuple.\n" +"\n" +"Pairs are returned in LIFO (last-in, first-out) order.\n" +"Raises KeyError if the dict is empty."); + +#define DICT_POPITEM_METHODDEF \ + {"popitem", (PyCFunction)dict_popitem, METH_NOARGS, dict_popitem__doc__}, + +static PyObject * +dict_popitem_impl(PyDictObject *self); + +static PyObject * +dict_popitem(PyDictObject *self, PyObject *Py_UNUSED(ignored)) +{ + return dict_popitem_impl(self); +} + PyDoc_STRVAR(dict___reversed____doc__, "__reversed__($self, /)\n" "--\n" @@ -133,4 +190,4 @@ dict___reversed__(PyDictObject *self, PyObject *Py_UNUSED(ignored)) { return dict___reversed___impl(self); } -/*[clinic end generated code: output=12c21ce3552d9617 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=0fd5cafc61a51d3c input=a9049054013a1b77]*/ diff --git a/Objects/dictobject.c b/Objects/dictobject.c index c1187c2cb8e..9ff009f6aa4 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2985,19 +2985,37 @@ dict_clear(PyDictObject *mp, PyObject *Py_UNUSED(ignored)) Py_RETURN_NONE; } +/*[clinic input] +dict.pop + + key: object + default: object = NULL + / + +Remove specified key and return the corresponding value. + +If key is not found, default is returned if given, otherwise KeyError is raised +[clinic start generated code]*/ + static PyObject * -dict_pop(PyDictObject *mp, PyObject *args) +dict_pop_impl(PyDictObject *self, PyObject *key, PyObject *default_value) +/*[clinic end generated code: output=3abb47b89f24c21c input=016f6a000e4e633b]*/ { - PyObject *key, *deflt = NULL; - - if(!PyArg_UnpackTuple(args, "pop", 1, 2, &key, &deflt)) - return NULL; - - return _PyDict_Pop((PyObject*)mp, key, deflt); + return _PyDict_Pop((PyObject*)self, key, default_value); } +/*[clinic input] +dict.popitem + +Remove and return a (key, value) pair as a 2-tuple. + +Pairs are returned in LIFO (last-in, first-out) order. +Raises KeyError if the dict is empty. +[clinic start generated code]*/ + static PyObject * -dict_popitem(PyDictObject *mp, PyObject *Py_UNUSED(ignored)) +dict_popitem_impl(PyDictObject *self) +/*[clinic end generated code: output=e65fcb04420d230d input=1c38a49f21f64941]*/ { Py_ssize_t i, j; PyDictKeyEntry *ep0, *ep; @@ -3015,44 +3033,43 @@ dict_popitem(PyDictObject *mp, PyObject *Py_UNUSED(ignored)) res = PyTuple_New(2); if (res == NULL) return NULL; - if (mp->ma_used == 0) { + if (self->ma_used == 0) { Py_DECREF(res); - PyErr_SetString(PyExc_KeyError, - "popitem(): dictionary is empty"); + PyErr_SetString(PyExc_KeyError, "popitem(): dictionary is empty"); return NULL; } /* Convert split table to combined table */ - if (mp->ma_keys->dk_lookup == lookdict_split) { - if (dictresize(mp, DK_SIZE(mp->ma_keys))) { + if (self->ma_keys->dk_lookup == lookdict_split) { + if (dictresize(self, DK_SIZE(self->ma_keys))) { Py_DECREF(res); return NULL; } } - ENSURE_ALLOWS_DELETIONS(mp); + ENSURE_ALLOWS_DELETIONS(self); /* Pop last item */ - ep0 = DK_ENTRIES(mp->ma_keys); - i = mp->ma_keys->dk_nentries - 1; + ep0 = DK_ENTRIES(self->ma_keys); + i = self->ma_keys->dk_nentries - 1; while (i >= 0 && ep0[i].me_value == NULL) { i--; } assert(i >= 0); ep = &ep0[i]; - j = lookdict_index(mp->ma_keys, ep->me_hash, i); + j = lookdict_index(self->ma_keys, ep->me_hash, i); assert(j >= 0); - assert(dictkeys_get_index(mp->ma_keys, j) == i); - dictkeys_set_index(mp->ma_keys, j, DKIX_DUMMY); + assert(dictkeys_get_index(self->ma_keys, j) == i); + dictkeys_set_index(self->ma_keys, j, DKIX_DUMMY); PyTuple_SET_ITEM(res, 0, ep->me_key); PyTuple_SET_ITEM(res, 1, ep->me_value); ep->me_key = NULL; ep->me_value = NULL; /* We can't dk_usable++ since there is DKIX_DUMMY in indices */ - mp->ma_keys->dk_nentries = i; - mp->ma_used--; - mp->ma_version_tag = DICT_NEXT_VERSION(); - assert(_PyDict_CheckConsistency(mp)); + self->ma_keys->dk_nentries = i; + self->ma_used--; + self->ma_version_tag = DICT_NEXT_VERSION(); + assert(_PyDict_CheckConsistency(self)); return res; } @@ -3135,14 +3152,6 @@ PyDoc_STRVAR(getitem__doc__, "x.__getitem__(y) <==> x[y]"); PyDoc_STRVAR(sizeof__doc__, "D.__sizeof__() -> size of D in memory, in bytes"); -PyDoc_STRVAR(pop__doc__, -"D.pop(k[,d]) -> v, remove specified key and return the corresponding value.\n\ -If key is not found, d is returned if given, otherwise KeyError is raised"); - -PyDoc_STRVAR(popitem__doc__, -"D.popitem() -> (k, v), remove and return some (key, value) pair as a\n\ -2-tuple; but raise KeyError if D is empty."); - PyDoc_STRVAR(update__doc__, "D.update([E, ]**F) -> None. Update D from dict/iterable E and F.\n\ If E is present and has a .keys() method, then does: for k in E: D[k] = E[k]\n\ @@ -3175,10 +3184,8 @@ static PyMethodDef mapp_methods[] = { sizeof__doc__}, DICT_GET_METHODDEF DICT_SETDEFAULT_METHODDEF - {"pop", (PyCFunction)dict_pop, METH_VARARGS, - pop__doc__}, - {"popitem", (PyCFunction)(void(*)(void))dict_popitem, METH_NOARGS, - popitem__doc__}, + DICT_POP_METHODDEF + DICT_POPITEM_METHODDEF {"keys", dictkeys_new, METH_NOARGS, keys__doc__}, {"items", dictitems_new, METH_NOARGS,