Issue #9935: Speed up pickling of instances of user-defined classes.

4 files changed, 104 insertions, 48 deletions

diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py
index 7645b54..e21d490 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
@@ -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.
@@ -1008,7 +1028,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 9aa0708..e47c0f3 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -43,6 +43,8 @@ Core and Builtins
 Library
 -------
 
+- Issue #9935: Speed up pickling of instances of user-defined classes.
+
 - Issue #5639: Add a *server_hostname* argument to ``SSLContext.wrap_socket``
   in order to support the TLS SNI extension.  ``HTTPSConnection`` and
   ``urlopen()`` also use this argument, so that HTTPS virtual hosts are now
diff --git a/Modules/_pickle.c b/Modules/_pickle.c
index e2d6406..fca9f2c 100644
--- a/Modules/_pickle.c
+++ b/Modules/_pickle.c
@@ -2840,6 +2840,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.
  */
@@ -2905,17 +2927,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
@@ -2923,9 +2946,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) {
@@ -2941,20 +2964,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 2c1bf88..5302a73 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -3010,14 +3010,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 *
@@ -3026,14 +3031,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;
@@ -3067,12 +3074,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;
+
+    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;
+    }
 
-    cls = PyObject_GetAttrString(obj, "__class__");
-    if (cls == NULL)
-        return NULL;
+    cls = (PyObject *) Py_TYPE(obj);
 
-    getnewargs = PyObject_GetAttrString(obj, "__getnewargs__");
+    getnewargs = PyObject_GetAttr(obj, str_getnewargs);
     if (getnewargs != NULL) {
         args = PyObject_CallObject(getnewargs, NULL);
         Py_DECREF(getnewargs);
@@ -3090,7 +3105,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);
@@ -3098,17 +3113,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)
@@ -3167,7 +3183,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;
 
@@ -3175,8 +3191,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);
@@ -3186,7 +3202,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);
@@ -3246,31 +3261,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) {