From 16c4ce19033faa4ed049d03f3ed379aae5ebd880 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Fri, 11 Mar 2011 21:30:43 +0100 Subject: [PATCH] Issue #9935: Speed up pickling of instances of user-defined classes. --- Lib/test/pickletester.py | 21 ++++++++++- Misc/NEWS | 2 ++ Modules/_pickle.c | 51 +++++++++++++++++--------- Objects/typeobject.c | 78 ++++++++++++++++++++++++---------------- 4 files changed, 104 insertions(+), 48 deletions(-) diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index c2fe6331737..45e8f74a1a9 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -3,6 +3,7 @@ import unittest import pickle import pickletools import copyreg +import weakref from http.cookies import SimpleCookie from test.support import TestFailed, TESTFN, run_with_locale, no_tracing @@ -842,6 +843,25 @@ class AbstractPickleTests(unittest.TestCase): self.assertEqual(B(x), B(y), detail) self.assertEqual(x.__dict__, y.__dict__, detail) + def test_newobj_proxies(self): + # NEWOBJ should use the __class__ rather than the raw type + classes = myclasses[:] + # Cannot create weakproxies to these classes + for c in (MyInt, MyTuple): + classes.remove(c) + for proto in protocols: + for C in classes: + B = C.__base__ + x = C(C.sample) + x.foo = 42 + p = weakref.proxy(x) + s = self.dumps(p, proto) + y = self.loads(s) + self.assertEqual(type(y), type(x)) # rather than type(p) + detail = (proto, C, B, x, y, type(y)) + self.assertEqual(B(x), B(y), detail) + self.assertEqual(x.__dict__, y.__dict__, detail) + # Register a type with copyreg, with extension code extcode. Pickle # an object of that type. Check that the resulting pickle uses opcode # (EXT[124]) under proto 2, and not in proto 1. @@ -1009,7 +1029,6 @@ class AbstractPickleTests(unittest.TestCase): self.assertRaises(RuntimeError, self.dumps, x, proto) # protocol 2 don't raise a RuntimeError. d = self.dumps(x, 2) - self.assertRaises(RuntimeError, self.loads, d) def test_reduce_bad_iterator(self): # Issue4176: crash when 4th and 5th items of __reduce__() diff --git a/Misc/NEWS b/Misc/NEWS index 1fa4b0d84a7..45a724020b2 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -62,6 +62,8 @@ Core and Builtins Library ------- +- Issue #9935: Speed up pickling of instances of user-defined classes. + - Issue #5622: Fix curses.wrapper to raise correct exception if curses initialization fails. diff --git a/Modules/_pickle.c b/Modules/_pickle.c index fb6281ee3a7..32ea39b7df5 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -2842,6 +2842,28 @@ save_pers(PicklerObject *self, PyObject *obj, PyObject *func) return status; } +static PyObject * +get_class(PyObject *obj) +{ + PyObject *cls; + static PyObject *str_class; + + if (str_class == NULL) { + str_class = PyUnicode_InternFromString("__class__"); + if (str_class == NULL) + return NULL; + } + cls = PyObject_GetAttr(obj, str_class); + if (cls == NULL) { + if (PyErr_ExceptionMatches(PyExc_AttributeError)) { + PyErr_Clear(); + cls = (PyObject *) Py_TYPE(obj); + Py_INCREF(cls); + } + } + return cls; +} + /* We're saving obj, and args is the 2-thru-5 tuple returned by the * appropriate __reduce__ method for obj. */ @@ -2907,17 +2929,18 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) /* Protocol 2 special case: if callable's name is __newobj__, use NEWOBJ. */ if (use_newobj) { - static PyObject *newobj_str = NULL; - PyObject *name_str; + static PyObject *newobj_str = NULL, *name_str = NULL; + PyObject *name; if (newobj_str == NULL) { newobj_str = PyUnicode_InternFromString("__newobj__"); - if (newobj_str == NULL) + name_str = PyUnicode_InternFromString("__name__"); + if (newobj_str == NULL || name_str == NULL) return -1; } - name_str = PyObject_GetAttrString(callable, "__name__"); - if (name_str == NULL) { + name = PyObject_GetAttr(callable, name_str); + if (name == NULL) { if (PyErr_ExceptionMatches(PyExc_AttributeError)) PyErr_Clear(); else @@ -2925,9 +2948,9 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) use_newobj = 0; } else { - use_newobj = PyUnicode_Check(name_str) && - PyUnicode_Compare(name_str, newobj_str) == 0; - Py_DECREF(name_str); + use_newobj = PyUnicode_Check(name) && + PyUnicode_Compare(name, newobj_str) == 0; + Py_DECREF(name); } } if (use_newobj) { @@ -2943,20 +2966,14 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj) } cls = PyTuple_GET_ITEM(argtup, 0); - if (!PyObject_HasAttrString(cls, "__new__")) { + if (!PyType_Check(cls)) { PyErr_SetString(PicklingError, "args[0] from " - "__newobj__ args has no __new__"); + "__newobj__ args is not a type"); return -1; } if (obj != NULL) { - obj_class = PyObject_GetAttrString(obj, "__class__"); - if (obj_class == NULL) { - if (PyErr_ExceptionMatches(PyExc_AttributeError)) - PyErr_Clear(); - else - return -1; - } + obj_class = get_class(obj); p = obj_class != cls; /* true iff a problem */ Py_DECREF(obj_class); if (p) { diff --git a/Objects/typeobject.c b/Objects/typeobject.c index fa8d15d30df..efeaa9d43a4 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3073,14 +3073,19 @@ static PyObject * import_copyreg(void) { static PyObject *copyreg_str; + static PyObject *mod_copyreg = NULL; if (!copyreg_str) { copyreg_str = PyUnicode_InternFromString("copyreg"); if (copyreg_str == NULL) return NULL; } + if (!mod_copyreg) { + mod_copyreg = PyImport_Import(copyreg_str); + } - return PyImport_Import(copyreg_str); + Py_XINCREF(mod_copyreg); + return mod_copyreg; } static PyObject * @@ -3089,14 +3094,16 @@ slotnames(PyObject *cls) PyObject *clsdict; PyObject *copyreg; PyObject *slotnames; + static PyObject *str_slotnames; - if (!PyType_Check(cls)) { - Py_INCREF(Py_None); - return Py_None; + if (str_slotnames == NULL) { + str_slotnames = PyUnicode_InternFromString("__slotnames__"); + if (str_slotnames == NULL) + return NULL; } clsdict = ((PyTypeObject *)cls)->tp_dict; - slotnames = PyDict_GetItemString(clsdict, "__slotnames__"); + slotnames = PyDict_GetItem(clsdict, str_slotnames); if (slotnames != NULL && PyList_Check(slotnames)) { Py_INCREF(slotnames); return slotnames; @@ -3130,12 +3137,20 @@ reduce_2(PyObject *obj) PyObject *slots = NULL, *listitems = NULL, *dictitems = NULL; PyObject *copyreg = NULL, *newobj = NULL, *res = NULL; Py_ssize_t i, n; + static PyObject *str_getnewargs = NULL, *str_getstate = NULL, + *str_newobj = NULL; - cls = PyObject_GetAttrString(obj, "__class__"); - if (cls == NULL) - return NULL; + if (str_getnewargs == NULL) { + str_getnewargs = PyUnicode_InternFromString("__getnewargs__"); + str_getstate = PyUnicode_InternFromString("__getstate__"); + str_newobj = PyUnicode_InternFromString("__newobj__"); + if (!str_getnewargs || !str_getstate || !str_newobj) + return NULL; + } - getnewargs = PyObject_GetAttrString(obj, "__getnewargs__"); + cls = (PyObject *) Py_TYPE(obj); + + getnewargs = PyObject_GetAttr(obj, str_getnewargs); if (getnewargs != NULL) { args = PyObject_CallObject(getnewargs, NULL); Py_DECREF(getnewargs); @@ -3153,7 +3168,7 @@ reduce_2(PyObject *obj) if (args == NULL) goto end; - getstate = PyObject_GetAttrString(obj, "__getstate__"); + getstate = PyObject_GetAttr(obj, str_getstate); if (getstate != NULL) { state = PyObject_CallObject(getstate, NULL); Py_DECREF(getstate); @@ -3161,17 +3176,18 @@ reduce_2(PyObject *obj) goto end; } else { + PyObject **dict; PyErr_Clear(); - state = PyObject_GetAttrString(obj, "__dict__"); - if (state == NULL) { - PyErr_Clear(); + dict = _PyObject_GetDictPtr(obj); + if (dict && *dict) + state = *dict; + else state = Py_None; - Py_INCREF(state); - } + Py_INCREF(state); names = slotnames(cls); if (names == NULL) goto end; - if (names != Py_None) { + if (names != Py_None && PyList_GET_SIZE(names) > 0) { assert(PyList_Check(names)); slots = PyDict_New(); if (slots == NULL) @@ -3230,7 +3246,7 @@ reduce_2(PyObject *obj) copyreg = import_copyreg(); if (copyreg == NULL) goto end; - newobj = PyObject_GetAttrString(copyreg, "__newobj__"); + newobj = PyObject_GetAttr(copyreg, str_newobj); if (newobj == NULL) goto end; @@ -3238,8 +3254,8 @@ reduce_2(PyObject *obj) args2 = PyTuple_New(n+1); if (args2 == NULL) goto end; + Py_INCREF(cls); PyTuple_SET_ITEM(args2, 0, cls); - cls = NULL; for (i = 0; i < n; i++) { PyObject *v = PyTuple_GET_ITEM(args, i); Py_INCREF(v); @@ -3249,7 +3265,6 @@ reduce_2(PyObject *obj) res = PyTuple_Pack(5, newobj, args2, state, listitems, dictitems); end: - Py_XDECREF(cls); Py_XDECREF(args); Py_XDECREF(args2); Py_XDECREF(slots); @@ -3309,31 +3324,34 @@ object_reduce(PyObject *self, PyObject *args) static PyObject * object_reduce_ex(PyObject *self, PyObject *args) { + static PyObject *str_reduce = NULL, *objreduce; PyObject *reduce, *res; int proto = 0; if (!PyArg_ParseTuple(args, "|i:__reduce_ex__", &proto)) return NULL; - reduce = PyObject_GetAttrString(self, "__reduce__"); + if (str_reduce == NULL) { + str_reduce = PyUnicode_InternFromString("__reduce__"); + objreduce = PyDict_GetItemString(PyBaseObject_Type.tp_dict, + "__reduce__"); + if (str_reduce == NULL || objreduce == NULL) + return NULL; + } + + reduce = PyObject_GetAttr(self, str_reduce); if (reduce == NULL) PyErr_Clear(); else { - PyObject *cls, *clsreduce, *objreduce; + PyObject *cls, *clsreduce; int override; - cls = PyObject_GetAttrString(self, "__class__"); - if (cls == NULL) { - Py_DECREF(reduce); - return NULL; - } - clsreduce = PyObject_GetAttrString(cls, "__reduce__"); - Py_DECREF(cls); + + cls = (PyObject *) Py_TYPE(self); + clsreduce = PyObject_GetAttr(cls, str_reduce); if (clsreduce == NULL) { Py_DECREF(reduce); return NULL; } - objreduce = PyDict_GetItemString(PyBaseObject_Type.tp_dict, - "__reduce__"); override = (clsreduce != objreduce); Py_DECREF(clsreduce); if (override) {