From bb86bf4c4eaa30b1f5192dab9f389ce0bb61114d Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 11 Dec 2018 08:28:18 +0200 Subject: [PATCH] bpo-35444: Unify and optimize the helper for getting a builtin object. (GH-11047) This speeds up pickling of some iterators. This fixes also error handling in pickling methods when fail to look up builtin "getattr". --- Include/ceval.h | 4 +++- Include/cpython/object.h | 3 --- .../2018-12-09-13-09-39.bpo-35444.9kYn4V.rst | 2 ++ Modules/_pickle.c | 8 ++------ Modules/arraymodule.c | 3 ++- Modules/itertoolsmodule.c | 3 ++- Objects/bytearrayobject.c | 5 +++-- Objects/bytesobject.c | 5 +++-- Objects/classobject.c | 7 ++----- Objects/descrobject.c | 17 ++++------------- Objects/dictobject.c | 3 ++- Objects/iterobject.c | 10 ++++++---- Objects/listobject.c | 8 +++++--- Objects/methodobject.c | 7 ++----- Objects/object.c | 17 ----------------- Objects/odictobject.c | 3 ++- Objects/rangeobject.c | 8 ++++++-- Objects/setobject.c | 3 ++- Objects/tupleobject.c | 5 +++-- Objects/unicodeobject.c | 5 +++-- Python/ceval.c | 14 ++++++++++++++ 21 files changed, 68 insertions(+), 72 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-12-09-13-09-39.bpo-35444.9kYn4V.rst diff --git a/Include/ceval.h b/Include/ceval.h index bce8a0beed8..11283c0a570 100644 --- a/Include/ceval.h +++ b/Include/ceval.h @@ -48,10 +48,12 @@ PyAPI_FUNC(PyObject *) PyEval_GetGlobals(void); PyAPI_FUNC(PyObject *) PyEval_GetLocals(void); PyAPI_FUNC(struct _frame *) PyEval_GetFrame(void); +#ifndef Py_LIMITED_API +/* Helper to look up a builtin object */ +PyAPI_FUNC(PyObject *) _PyEval_GetBuiltinId(_Py_Identifier *); /* Look at the current frame's (if any) code's co_flags, and turn on the corresponding compiler flags in cf->cf_flags. Return 1 if any flag was set, else return 0. */ -#ifndef Py_LIMITED_API PyAPI_FUNC(int) PyEval_MergeCompilerFlags(PyCompilerFlags *cf); #endif diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 77184c95151..64d196a722e 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -330,9 +330,6 @@ PyAPI_FUNC(int) _PyObject_GenericSetAttrWithDict(PyObject *, PyObject *, PyObject *, PyObject *); -/* Helper to look up a builtin object */ -PyAPI_FUNC(PyObject *) _PyObject_GetBuiltin(const char *name); - #define PyType_HasFeature(t,f) (((t)->tp_flags & (f)) != 0) static inline void _Py_Dealloc_inline(PyObject *op) diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-12-09-13-09-39.bpo-35444.9kYn4V.rst b/Misc/NEWS.d/next/Core and Builtins/2018-12-09-13-09-39.bpo-35444.9kYn4V.rst new file mode 100644 index 00000000000..39591f9c32d --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-12-09-13-09-39.bpo-35444.9kYn4V.rst @@ -0,0 +1,2 @@ +Fixed error handling in pickling methods when fail to look up builtin +"getattr". Sped up pickling iterators. diff --git a/Modules/_pickle.c b/Modules/_pickle.c index c4fe349187c..58cd09ca61a 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -209,19 +209,15 @@ _Pickle_ClearState(PickleState *st) static int _Pickle_InitState(PickleState *st) { - PyObject *builtins; PyObject *copyreg = NULL; PyObject *compat_pickle = NULL; PyObject *codecs = NULL; PyObject *functools = NULL; + _Py_IDENTIFIER(getattr); - builtins = PyEval_GetBuiltins(); - if (builtins == NULL) - goto error; - st->getattr = PyDict_GetItemString(builtins, "getattr"); + st->getattr = _PyEval_GetBuiltinId(&PyId_getattr); if (st->getattr == NULL) goto error; - Py_INCREF(st->getattr); copyreg = PyImport_ImportModule("copyreg"); if (!copyreg) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index aa7a4fb23c6..1bfc4dd95bf 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -2943,7 +2943,8 @@ static PyObject * array_arrayiterator___reduce___impl(arrayiterobject *self) /*[clinic end generated code: output=7898a52e8e66e016 input=a062ea1e9951417a]*/ { - PyObject *func = _PyObject_GetBuiltin("iter"); + _Py_IDENTIFIER(iter); + PyObject *func = _PyEval_GetBuiltinId(&PyId_iter); if (self->ao == NULL) { return Py_BuildValue("N(())", func); } diff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.c index 89c0280c9d3..581ae2c67e8 100644 --- a/Modules/itertoolsmodule.c +++ b/Modules/itertoolsmodule.c @@ -368,8 +368,9 @@ _grouper_next(_grouperobject *igo) static PyObject * _grouper_reduce(_grouperobject *lz, PyObject *Py_UNUSED(ignored)) { + _Py_IDENTIFIER(iter); if (((groupbyobject *)lz->parent)->currgrouper != lz) { - return Py_BuildValue("N(())", _PyObject_GetBuiltin("iter")); + return Py_BuildValue("N(())", _PyEval_GetBuiltinId(&PyId_iter)); } return Py_BuildValue("O(OO)", Py_TYPE(lz), lz->parent, lz->tgtkey); } diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index 14426538194..3926095a181 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -2358,11 +2358,12 @@ PyDoc_STRVAR(length_hint_doc, static PyObject * bytearrayiter_reduce(bytesiterobject *it, PyObject *Py_UNUSED(ignored)) { + _Py_IDENTIFIER(iter); if (it->it_seq != NULL) { - return Py_BuildValue("N(O)n", _PyObject_GetBuiltin("iter"), + return Py_BuildValue("N(O)n", _PyEval_GetBuiltinId(&PyId_iter), it->it_seq, it->it_index); } else { - return Py_BuildValue("N(())", _PyObject_GetBuiltin("iter")); + return Py_BuildValue("N(())", _PyEval_GetBuiltinId(&PyId_iter)); } } diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index e4a49731aba..adf0cff5f3b 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -3089,11 +3089,12 @@ PyDoc_STRVAR(length_hint_doc, static PyObject * striter_reduce(striterobject *it, PyObject *Py_UNUSED(ignored)) { + _Py_IDENTIFIER(iter); if (it->it_seq != NULL) { - return Py_BuildValue("N(O)n", _PyObject_GetBuiltin("iter"), + return Py_BuildValue("N(O)n", _PyEval_GetBuiltinId(&PyId_iter), it->it_seq, it->it_index); } else { - return Py_BuildValue("N(())", _PyObject_GetBuiltin("iter")); + return Py_BuildValue("N(())", _PyEval_GetBuiltinId(&PyId_iter)); } } diff --git a/Objects/classobject.c b/Objects/classobject.c index 1eed5d36ab0..0d1cf7a95cc 100644 --- a/Objects/classobject.c +++ b/Objects/classobject.c @@ -78,8 +78,6 @@ method_reduce(PyMethodObject *im, PyObject *Py_UNUSED(ignored)) { PyObject *self = PyMethod_GET_SELF(im); PyObject *func = PyMethod_GET_FUNCTION(im); - PyObject *builtins; - PyObject *getattr; PyObject *funcname; _Py_IDENTIFIER(getattr); @@ -87,9 +85,8 @@ method_reduce(PyMethodObject *im, PyObject *Py_UNUSED(ignored)) if (funcname == NULL) { return NULL; } - builtins = PyEval_GetBuiltins(); - getattr = _PyDict_GetItemId(builtins, &PyId_getattr); - return Py_BuildValue("O(ON)", getattr, self, funcname); + return Py_BuildValue("N(ON)", _PyEval_GetBuiltinId(&PyId_getattr), + self, funcname); } static PyMethodDef method_methods[] = { diff --git a/Objects/descrobject.c b/Objects/descrobject.c index e129d0c48f2..22546a563a5 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -452,14 +452,9 @@ descr_get_qualname(PyDescrObject *descr, void *Py_UNUSED(ignored)) static PyObject * descr_reduce(PyDescrObject *descr, PyObject *Py_UNUSED(ignored)) { - PyObject *builtins; - PyObject *getattr; _Py_IDENTIFIER(getattr); - - builtins = PyEval_GetBuiltins(); - getattr = _PyDict_GetItemId(builtins, &PyId_getattr); - return Py_BuildValue("O(OO)", getattr, PyDescr_TYPE(descr), - PyDescr_NAME(descr)); + return Py_BuildValue("N(OO)", _PyEval_GetBuiltinId(&PyId_getattr), + PyDescr_TYPE(descr), PyDescr_NAME(descr)); } static PyMethodDef descr_methods[] = { @@ -1087,13 +1082,9 @@ wrapper_repr(wrapperobject *wp) static PyObject * wrapper_reduce(wrapperobject *wp, PyObject *Py_UNUSED(ignored)) { - PyObject *builtins; - PyObject *getattr; _Py_IDENTIFIER(getattr); - - builtins = PyEval_GetBuiltins(); - getattr = _PyDict_GetItemId(builtins, &PyId_getattr); - return Py_BuildValue("O(OO)", getattr, wp->self, PyDescr_NAME(wp->descr)); + return Py_BuildValue("N(OO)", _PyEval_GetBuiltinId(&PyId_getattr), + wp->self, PyDescr_NAME(wp->descr)); } static PyMethodDef wrapper_methods[] = { diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 72cb4c5d666..a8716368237 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3810,6 +3810,7 @@ dict___reversed___impl(PyDictObject *self) static PyObject * dictiter_reduce(dictiterobject *di, PyObject *Py_UNUSED(ignored)) { + _Py_IDENTIFIER(iter); /* copy the iterator state */ dictiterobject tmp = *di; Py_XINCREF(tmp.di_dict); @@ -3819,7 +3820,7 @@ dictiter_reduce(dictiterobject *di, PyObject *Py_UNUSED(ignored)) if (list == NULL) { return NULL; } - return Py_BuildValue("N(N)", _PyObject_GetBuiltin("iter"), list); + return Py_BuildValue("N(N)", _PyEval_GetBuiltinId(&PyId_iter), list); } PyTypeObject PyDictRevIterItem_Type = { diff --git a/Objects/iterobject.c b/Objects/iterobject.c index ada1bdc7e87..5bee1e21e65 100644 --- a/Objects/iterobject.c +++ b/Objects/iterobject.c @@ -104,11 +104,12 @@ PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list( static PyObject * iter_reduce(seqiterobject *it, PyObject *Py_UNUSED(ignored)) { + _Py_IDENTIFIER(iter); if (it->it_seq != NULL) - return Py_BuildValue("N(O)n", _PyObject_GetBuiltin("iter"), + return Py_BuildValue("N(O)n", _PyEval_GetBuiltinId(&PyId_iter), it->it_seq, it->it_index); else - return Py_BuildValue("N(())", _PyObject_GetBuiltin("iter")); + return Py_BuildValue("N(())", _PyEval_GetBuiltinId(&PyId_iter)); } PyDoc_STRVAR(reduce_doc, "Return state information for pickling."); @@ -243,11 +244,12 @@ calliter_iternext(calliterobject *it) static PyObject * calliter_reduce(calliterobject *it, PyObject *Py_UNUSED(ignored)) { + _Py_IDENTIFIER(iter); if (it->it_callable != NULL && it->it_sentinel != NULL) - return Py_BuildValue("N(OO)", _PyObject_GetBuiltin("iter"), + return Py_BuildValue("N(OO)", _PyEval_GetBuiltinId(&PyId_iter), it->it_callable, it->it_sentinel); else - return Py_BuildValue("N(())", _PyObject_GetBuiltin("iter")); + return Py_BuildValue("N(())", _PyEval_GetBuiltinId(&PyId_iter)); } static PyMethodDef calliter_methods[] = { diff --git a/Objects/listobject.c b/Objects/listobject.c index 6da8391fc27..b1f2b59db0a 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -3356,23 +3356,25 @@ listreviter_setstate(listreviterobject *it, PyObject *state) static PyObject * listiter_reduce_general(void *_it, int forward) { + _Py_IDENTIFIER(iter); + _Py_IDENTIFIER(reversed); PyObject *list; /* the objects are not the same, index is of different types! */ if (forward) { listiterobject *it = (listiterobject *)_it; if (it->it_seq) - return Py_BuildValue("N(O)n", _PyObject_GetBuiltin("iter"), + return Py_BuildValue("N(O)n", _PyEval_GetBuiltinId(&PyId_iter), it->it_seq, it->it_index); } else { listreviterobject *it = (listreviterobject *)_it; if (it->it_seq) - return Py_BuildValue("N(O)n", _PyObject_GetBuiltin("reversed"), + return Py_BuildValue("N(O)n", _PyEval_GetBuiltinId(&PyId_reversed), it->it_seq, it->it_index); } /* empty iterator, create an empty list */ list = PyList_New(0); if (list == NULL) return NULL; - return Py_BuildValue("N(N)", _PyObject_GetBuiltin("iter"), list); + return Py_BuildValue("N(N)", _PyEval_GetBuiltinId(&PyId_iter), list); } diff --git a/Objects/methodobject.c b/Objects/methodobject.c index 6dec64276de..9fed3fca99b 100644 --- a/Objects/methodobject.c +++ b/Objects/methodobject.c @@ -104,16 +104,13 @@ meth_dealloc(PyCFunctionObject *m) static PyObject * meth_reduce(PyCFunctionObject *m, PyObject *Py_UNUSED(ignored)) { - PyObject *builtins; - PyObject *getattr; _Py_IDENTIFIER(getattr); if (m->m_self == NULL || PyModule_Check(m->m_self)) return PyUnicode_FromString(m->m_ml->ml_name); - builtins = PyEval_GetBuiltins(); - getattr = _PyDict_GetItemId(builtins, &PyId_getattr); - return Py_BuildValue("O(Os)", getattr, m->m_self, m->m_ml->ml_name); + return Py_BuildValue("N(Os)", _PyEval_GetBuiltinId(&PyId_getattr), + m->m_self, m->m_ml->ml_name); } static PyMethodDef meth_methods[] = { diff --git a/Objects/object.c b/Objects/object.c index c2d78aa47e6..993342eae5d 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1080,23 +1080,6 @@ PyObject_SelfIter(PyObject *obj) return obj; } -/* Convenience function to get a builtin from its name */ -PyObject * -_PyObject_GetBuiltin(const char *name) -{ - PyObject *mod_name, *mod, *attr; - - mod_name = _PyUnicode_FromId(&PyId_builtins); /* borrowed */ - if (mod_name == NULL) - return NULL; - mod = PyImport_Import(mod_name); - if (mod == NULL) - return NULL; - attr = PyObject_GetAttrString(mod, name); - Py_DECREF(mod); - return attr; -} - /* Helper used when the __next__ method is removed from a type: tp_iternext is never NULL and can be safely called without checking on every iteration. diff --git a/Objects/odictobject.c b/Objects/odictobject.c index 689062c95dd..767eb5f6061 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -1806,6 +1806,7 @@ PyDoc_STRVAR(reduce_doc, "Return state information for pickling"); static PyObject * odictiter_reduce(odictiterobject *di, PyObject *Py_UNUSED(ignored)) { + _Py_IDENTIFIER(iter); /* copy the iterator state */ odictiterobject tmp = *di; Py_XINCREF(tmp.di_odict); @@ -1818,7 +1819,7 @@ odictiter_reduce(odictiterobject *di, PyObject *Py_UNUSED(ignored)) if (list == NULL) { return NULL; } - return Py_BuildValue("N(N)", _PyObject_GetBuiltin("iter"), list); + return Py_BuildValue("N(N)", _PyEval_GetBuiltinId(&PyId_iter), list); } static PyMethodDef odictiter_methods[] = { diff --git a/Objects/rangeobject.c b/Objects/rangeobject.c index e7b47bde666..4b8e5ed4cfd 100644 --- a/Objects/rangeobject.c +++ b/Objects/rangeobject.c @@ -742,6 +742,7 @@ PyDoc_STRVAR(length_hint_doc, static PyObject * rangeiter_reduce(rangeiterobject *r, PyObject *Py_UNUSED(ignored)) { + _Py_IDENTIFIER(iter); PyObject *start=NULL, *stop=NULL, *step=NULL; PyObject *range; @@ -760,7 +761,8 @@ rangeiter_reduce(rangeiterobject *r, PyObject *Py_UNUSED(ignored)) if (range == NULL) goto err; /* return the result */ - return Py_BuildValue("N(N)i", _PyObject_GetBuiltin("iter"), range, r->index); + return Py_BuildValue("N(N)i", _PyEval_GetBuiltinId(&PyId_iter), + range, r->index); err: Py_XDECREF(start); Py_XDECREF(stop); @@ -898,6 +900,7 @@ longrangeiter_len(longrangeiterobject *r, PyObject *no_args) static PyObject * longrangeiter_reduce(longrangeiterobject *r, PyObject *Py_UNUSED(ignored)) { + _Py_IDENTIFIER(iter); PyObject *product, *stop=NULL; PyObject *range; @@ -921,7 +924,8 @@ longrangeiter_reduce(longrangeiterobject *r, PyObject *Py_UNUSED(ignored)) } /* return the result */ - return Py_BuildValue("N(N)O", _PyObject_GetBuiltin("iter"), range, r->index); + return Py_BuildValue("N(N)O", _PyEval_GetBuiltinId(&PyId_iter), + range, r->index); } static PyObject * diff --git a/Objects/setobject.c b/Objects/setobject.c index c2a1467ba61..a43ecd52853 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -842,6 +842,7 @@ static PyObject *setiter_iternext(setiterobject *si); static PyObject * setiter_reduce(setiterobject *si, PyObject *Py_UNUSED(ignored)) { + _Py_IDENTIFIER(iter); /* copy the iterator state */ setiterobject tmp = *si; Py_XINCREF(tmp.si_set); @@ -852,7 +853,7 @@ setiter_reduce(setiterobject *si, PyObject *Py_UNUSED(ignored)) if (list == NULL) { return NULL; } - return Py_BuildValue("N(N)", _PyObject_GetBuiltin("iter"), list); + return Py_BuildValue("N(N)", _PyEval_GetBuiltinId(&PyId_iter), list); } PyDoc_STRVAR(reduce_doc, "Return state information for pickling."); diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index 83c63e089c3..9cf3f3dd66e 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -1024,11 +1024,12 @@ PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list( static PyObject * tupleiter_reduce(tupleiterobject *it, PyObject *Py_UNUSED(ignored)) { + _Py_IDENTIFIER(iter); if (it->it_seq) - return Py_BuildValue("N(O)n", _PyObject_GetBuiltin("iter"), + return Py_BuildValue("N(O)n", _PyEval_GetBuiltinId(&PyId_iter), it->it_seq, it->it_index); else - return Py_BuildValue("N(())", _PyObject_GetBuiltin("iter")); + return Py_BuildValue("N(())", _PyEval_GetBuiltinId(&PyId_iter)); } static PyObject * diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index d0f0358cfc6..06338fac2b2 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -15444,14 +15444,15 @@ PyDoc_STRVAR(length_hint_doc, "Private method returning an estimate of len(list( static PyObject * unicodeiter_reduce(unicodeiterobject *it, PyObject *Py_UNUSED(ignored)) { + _Py_IDENTIFIER(iter); if (it->it_seq != NULL) { - return Py_BuildValue("N(O)n", _PyObject_GetBuiltin("iter"), + return Py_BuildValue("N(O)n", _PyEval_GetBuiltinId(&PyId_iter), it->it_seq, it->it_index); } else { PyObject *u = (PyObject *)_PyUnicode_New(0); if (u == NULL) return NULL; - return Py_BuildValue("N(N)", _PyObject_GetBuiltin("iter"), u); + return Py_BuildValue("N(N)", _PyEval_GetBuiltinId(&PyId_iter), u); } } diff --git a/Python/ceval.c b/Python/ceval.c index 7ffc68a1e1f..ebe1c50e7ba 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -4437,6 +4437,20 @@ PyEval_GetBuiltins(void) return current_frame->f_builtins; } +/* Convenience function to get a builtin from its name */ +PyObject * +_PyEval_GetBuiltinId(_Py_Identifier *name) +{ + PyObject *attr = _PyDict_GetItemIdWithError(PyEval_GetBuiltins(), name); + if (attr) { + Py_INCREF(attr); + } + else if (!PyErr_Occurred()) { + PyErr_SetObject(PyExc_AttributeError, _PyUnicode_FromId(name)); + } + return attr; +} + PyObject * PyEval_GetLocals(void) {