Issue #22783: Pickling now uses the NEWOBJ opcode instead of the NEWOBJ_EX

opcode if possible.

Lib/test/pickletester.py

@@ -1153,16 +1153,62 @@ class AbstractPickleTests(unittest.TestCase):
                 self.assertGreaterEqual(num_additems, 2)
 
     def test_simple_newobj(self):
-        x = object.__new__(SimpleNewObj)  # avoid __init__
+        x = SimpleNewObj.__new__(SimpleNewObj, 0xface)  # avoid __init__
         x.abc = 666
         for proto in protocols:
-            s = self.dumps(x, proto)
-            self.assertEqual(opcode_in_pickle(pickle.NEWOBJ, s),
-                             2 <= proto < 4)
-            self.assertEqual(opcode_in_pickle(pickle.NEWOBJ_EX, s),
-                             proto >= 4)
-            y = self.loads(s)   # will raise TypeError if __init__ called
-            self.assert_is_copy(x, y)
+            with self.subTest(proto=proto):
+                s = self.dumps(x, proto)
+                if proto < 1:
+                    self.assertIn(b'\nL64206', s)  # LONG
+                else:
+                    self.assertIn(b'M\xce\xfa', s)  # BININT2
+                self.assertEqual(opcode_in_pickle(pickle.NEWOBJ, s),
+                                 2 <= proto)
+                self.assertFalse(opcode_in_pickle(pickle.NEWOBJ_EX, s))
+                y = self.loads(s)   # will raise TypeError if __init__ called
+                self.assert_is_copy(x, y)
+
+    def test_complex_newobj(self):
+        x = ComplexNewObj.__new__(ComplexNewObj, 0xface)  # avoid __init__
+        x.abc = 666
+        for proto in protocols:
+            with self.subTest(proto=proto):
+                s = self.dumps(x, proto)
+                if proto < 1:
+                    self.assertIn(b'\nL64206', s)  # LONG
+                elif proto < 2:
+                    self.assertIn(b'M\xce\xfa', s)  # BININT2
+                elif proto < 4:
+                    self.assertIn(b'X\x04\x00\x00\x00FACE', s)  # BINUNICODE
+                else:
+                    self.assertIn(b'\x8c\x04FACE', s)  # SHORT_BINUNICODE
+                self.assertEqual(opcode_in_pickle(pickle.NEWOBJ, s),
+                                 2 <= proto)
+                self.assertFalse(opcode_in_pickle(pickle.NEWOBJ_EX, s))
+                y = self.loads(s)   # will raise TypeError if __init__ called
+                self.assert_is_copy(x, y)
+
+    def test_complex_newobj_ex(self):
+        x = ComplexNewObjEx.__new__(ComplexNewObjEx, 0xface)  # avoid __init__
+        x.abc = 666
+        for proto in protocols:
+            with self.subTest(proto=proto):
+                if 2 <= proto < 4:
+                    self.assertRaises(ValueError, self.dumps, x, proto)
+                    continue
+                s = self.dumps(x, proto)
+                if proto < 1:
+                    self.assertIn(b'\nL64206', s)  # LONG
+                elif proto < 2:
+                    self.assertIn(b'M\xce\xfa', s)  # BININT2
+                else:
+                    assert proto >= 4
+                    self.assertIn(b'\x8c\x04FACE', s)  # SHORT_BINUNICODE
+                self.assertFalse(opcode_in_pickle(pickle.NEWOBJ, s))
+                self.assertEqual(opcode_in_pickle(pickle.NEWOBJ_EX, s),
+                                 4 <= proto)
+                y = self.loads(s)   # will raise TypeError if __init__ called
+                self.assert_is_copy(x, y)
 
     def test_newobj_list_slots(self):
         x = SlotList([1, 2, 3])
@@ -1891,12 +1937,20 @@ myclasses = [MyInt, MyFloat,
 class SlotList(MyList):
     __slots__ = ["foo"]
 
-class SimpleNewObj(object):
-    def __init__(self, a, b, c):
+class SimpleNewObj(int):
+    def __init__(self, *args, **kwargs):
         # raise an error, to make sure this isn't called
         raise TypeError("SimpleNewObj.__init__() didn't expect to get called")
     def __eq__(self, other):
-        return self.__dict__ == other.__dict__
+        return int(self) == int(other) and self.__dict__ == other.__dict__
+
+class ComplexNewObj(SimpleNewObj):
+    def __getnewargs__(self):
+        return ('%X' % self, 16)
+
+class ComplexNewObjEx(SimpleNewObj):
+    def __getnewargs_ex__(self):
+        return ('%X' % self,), {'base': 16}
 
 class BadGetattr:
     def __getattr__(self, key):

Lib/test/test_descr.py

@@ -4592,26 +4592,15 @@ class PicklingTests(unittest.TestCase):
 
     def _check_reduce(self, proto, obj, args=(), kwargs={}, state=None,
                       listitems=None, dictitems=None):
-        if proto >= 4:
+        if proto >= 2:
             reduce_value = obj.__reduce_ex__(proto)
-            self.assertEqual(reduce_value[:3],
-                             (copyreg.__newobj_ex__,
-                              (type(obj), args, kwargs),
-                              state))
-            if listitems is not None:
-                self.assertListEqual(list(reduce_value[3]), listitems)
-            else:
-                self.assertIsNone(reduce_value[3])
-            if dictitems is not None:
-                self.assertDictEqual(dict(reduce_value[4]), dictitems)
+            if kwargs:
+                self.assertEqual(reduce_value[0], copyreg.__newobj_ex__)
+                self.assertEqual(reduce_value[1], (type(obj), args, kwargs))
             else:
-                self.assertIsNone(reduce_value[4])
-        elif proto >= 2:
-            reduce_value = obj.__reduce_ex__(proto)
-            self.assertEqual(reduce_value[:3],
-                             (copyreg.__newobj__,
-                              (type(obj),) + args,
-                              state))
+                self.assertEqual(reduce_value[0], copyreg.__newobj__)
+                self.assertEqual(reduce_value[1], (type(obj),) + args)
+            self.assertEqual(reduce_value[2], state)
             if listitems is not None:
                 self.assertListEqual(list(reduce_value[3]), listitems)
             else:

Misc/NEWS

@@ -196,6 +196,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #22783: Pickling now uses the NEWOBJ opcode instead of the NEWOBJ_EX
+  opcode if possible.
+
 - Issue #15513: Added a __sizeof__ implementation for pickle classes.
 
 - Issue #19858: pickletools.optimize() now aware of the MEMOIZE opcode, can

Modules/_pickle.c

@@ -3506,20 +3506,19 @@ save_reduce(PicklerObject *self, PyObject *args, PyObject *obj)
             }
             PyErr_Clear();
         }
-        else if (self->proto >= 4) {
-            _Py_IDENTIFIER(__newobj_ex__);
-            use_newobj_ex = PyUnicode_Check(name) &&
-                PyUnicode_Compare(
-                    name, _PyUnicode_FromId(&PyId___newobj_ex__)) == 0;
-            Py_DECREF(name);
-        }
-        else {
-            _Py_IDENTIFIER(__newobj__);
-            use_newobj = PyUnicode_Check(name) &&
-                PyUnicode_Compare(
-                    name, _PyUnicode_FromId(&PyId___newobj__)) == 0;
-            Py_DECREF(name);
+        else if (PyUnicode_Check(name)) {
+            if (self->proto >= 4) {
+                _Py_IDENTIFIER(__newobj_ex__);
+                use_newobj_ex = PyUnicode_Compare(
+                        name, _PyUnicode_FromId(&PyId___newobj_ex__)) == 0;
+            }
+            if (!use_newobj_ex) {
+                _Py_IDENTIFIER(__newobj__);
+                use_newobj = PyUnicode_Compare(
+                        name, _PyUnicode_FromId(&PyId___newobj__)) == 0;
+            }
         }
+        Py_XDECREF(name);
     }
 
     if (use_newobj_ex) {

Objects/typeobject.c

@@ -3886,48 +3886,87 @@ _PyObject_GetItemsIter(PyObject *obj, PyObject **listitems,
 }
 
 static PyObject *
-reduce_4(PyObject *obj)
+reduce_newobj(PyObject *obj, int proto)
 {
     PyObject *args = NULL, *kwargs = NULL;
     PyObject *copyreg;
     PyObject *newobj, *newargs, *state, *listitems, *dictitems;
     PyObject *result;
-    _Py_IDENTIFIER(__newobj_ex__);
 
-    if (_PyObject_GetNewArguments(obj, &args, &kwargs) < 0) {
+    if (_PyObject_GetNewArguments(obj, &args, &kwargs) < 0)
         return NULL;
-    }
+
     if (args == NULL) {
         args = PyTuple_New(0);
-        if (args == NULL)
-            return NULL;
-    }
-    if (kwargs == NULL) {
-        kwargs = PyDict_New();
-        if (kwargs == NULL)
+        if (args == NULL) {
+            Py_XDECREF(kwargs);
             return NULL;
+        }
     }
-
     copyreg = import_copyreg();
     if (copyreg == NULL) {
         Py_DECREF(args);
-        Py_DECREF(kwargs);
+        Py_XDECREF(kwargs);
         return NULL;
     }
-    newobj = _PyObject_GetAttrId(copyreg, &PyId___newobj_ex__);
-    Py_DECREF(copyreg);
-    if (newobj == NULL) {
+    if (kwargs == NULL || PyDict_Size(kwargs) == 0) {
+        _Py_IDENTIFIER(__newobj__);
+        PyObject *cls;
+        Py_ssize_t i, n;
+
+        Py_XDECREF(kwargs);
+        newobj = _PyObject_GetAttrId(copyreg, &PyId___newobj__);
+        Py_DECREF(copyreg);
+        if (newobj == NULL) {
+            Py_DECREF(args);
+            return NULL;
+        }
+        n = PyTuple_GET_SIZE(args);
+        newargs = PyTuple_New(n+1);
+        if (newargs == NULL) {
+            Py_DECREF(args);
+            Py_DECREF(newobj);
+            return NULL;
+        }
+        cls = (PyObject *) Py_TYPE(obj);
+        Py_INCREF(cls);
+        PyTuple_SET_ITEM(newargs, 0, cls);
+        for (i = 0; i < n; i++) {
+            PyObject *v = PyTuple_GET_ITEM(args, i);
+            Py_INCREF(v);
+            PyTuple_SET_ITEM(newargs, i+1, v);
+        }
+        Py_DECREF(args);
+    }
+    else if (proto >= 4) {
+        _Py_IDENTIFIER(__newobj_ex__);
+
+        newobj = _PyObject_GetAttrId(copyreg, &PyId___newobj_ex__);
+        Py_DECREF(copyreg);
+        if (newobj == NULL) {
+            Py_DECREF(args);
+            Py_DECREF(kwargs);
+            return NULL;
+        }
+        newargs = PyTuple_Pack(3, Py_TYPE(obj), args, kwargs);
         Py_DECREF(args);
         Py_DECREF(kwargs);
-        return NULL;
+        if (newargs == NULL) {
+            Py_DECREF(newobj);
+            return NULL;
+        }
     }
-    newargs = PyTuple_Pack(3, Py_TYPE(obj), args, kwargs);
-    Py_DECREF(args);
-    Py_DECREF(kwargs);
-    if (newargs == NULL) {
-        Py_DECREF(newobj);
+    else {
+        PyErr_SetString(PyExc_ValueError,
+                        "must use protocol 4 or greater to copy this "
+                        "object; since __getnewargs_ex__ returned "
+                        "keyword arguments.");
+        Py_DECREF(args);
+        Py_DECREF(kwargs);
+        Py_DECREF(copyreg);
         return NULL;
     }
+
     state = _PyObject_GetState(obj);
     if (state == NULL) {
         Py_DECREF(newobj);
@@ -3950,79 +3989,6 @@ reduce_4(PyObject *obj)
     return result;
 }
 
-static PyObject *
-reduce_2(PyObject *obj)
-{
-    PyObject *cls;
-    PyObject *args = NULL, *args2 = NULL, *kwargs = NULL;
-    PyObject *state = NULL, *listitems = NULL, *dictitems = NULL;
-    PyObject *copyreg = NULL, *newobj = NULL, *res = NULL;
-    Py_ssize_t i, n;
-    _Py_IDENTIFIER(__newobj__);
-
-    if (_PyObject_GetNewArguments(obj, &args, &kwargs) < 0) {
-        return NULL;
-    }
-    if (args == NULL) {
-        assert(kwargs == NULL);
-        args = PyTuple_New(0);
-        if (args == NULL) {
-            return NULL;
-        }
-    }
-    else if (kwargs != NULL) {
-        if (PyDict_Size(kwargs) > 0) {
-            PyErr_SetString(PyExc_ValueError,
-                            "must use protocol 4 or greater to copy this "
-                            "object; since __getnewargs_ex__ returned "
-                            "keyword arguments.");
-            Py_DECREF(args);
-            Py_DECREF(kwargs);
-            return NULL;
-        }
-        Py_CLEAR(kwargs);
-    }
-
-    state = _PyObject_GetState(obj);
-    if (state == NULL)
-        goto end;
-
-    if (_PyObject_GetItemsIter(obj, &listitems, &dictitems) < 0)
-        goto end;
-
-    copyreg = import_copyreg();
-    if (copyreg == NULL)
-        goto end;
-    newobj = _PyObject_GetAttrId(copyreg, &PyId___newobj__);
-    if (newobj == NULL)
-        goto end;
-
-    n = PyTuple_GET_SIZE(args);
-    args2 = PyTuple_New(n+1);
-    if (args2 == NULL)
-        goto end;
-    cls = (PyObject *) Py_TYPE(obj);
-    Py_INCREF(cls);
-    PyTuple_SET_ITEM(args2, 0, cls);
-    for (i = 0; i < n; i++) {
-        PyObject *v = PyTuple_GET_ITEM(args, i);
-        Py_INCREF(v);
-        PyTuple_SET_ITEM(args2, i+1, v);
-    }
-
-    res = PyTuple_Pack(5, newobj, args2, state, listitems, dictitems);
-
-  end:
-    Py_XDECREF(args);
-    Py_XDECREF(args2);
-    Py_XDECREF(state);
-    Py_XDECREF(listitems);
-    Py_XDECREF(dictitems);
-    Py_XDECREF(copyreg);
-    Py_XDECREF(newobj);
-    return res;
-}
-
 /*
  * There were two problems when object.__reduce__ and object.__reduce_ex__
  * were implemented in the same function:
@@ -4043,10 +4009,8 @@ _common_reduce(PyObject *self, int proto)
 {
     PyObject *copyreg, *res;
 
-    if (proto >= 4)
-        return reduce_4(self);
-    else if (proto >= 2)
-        return reduce_2(self);
+    if (proto >= 2)
+        return reduce_newobj(self, proto);
 
     copyreg = import_copyreg();
     if (!copyreg)