From bb86bf4c4eaa30b1f5192dab9f389ce0bb61114d Mon Sep 17 00:00:00 2001
From: Serhiy Storchaka <storchaka@gmail.com>
Date: Tue, 11 Dec 2018 08:28:18 +0200
Subject: 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 bce8a0b..11283c0 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 77184c9..64d196a 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 0000000..39591f9
--- /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 c4fe349..58cd09c 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 aa7a4fb..1bfc4dd 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 89c0280..581ae2c 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 1442653..3926095 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 e4a4973..adf0cff 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 1eed5d3..0d1cf7a 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 e129d0c..22546a5 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 72cb4c5..a871636 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 ada1bdc..5bee1e2 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 6da8391..b1f2b59 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 6dec642..9fed3fc 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 c2d78aa..993342e 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 689062c..767eb5f 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 e7b47bd..4b8e5ed 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 c2a1467..a43ecd5 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 83c63e0..9cf3f3d 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 d0f0358..06338fa 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 7ffc68a..ebe1c50 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)
{
--
cgit v0.12