[Python-checkins] cpython: Issue #22783: Pickling now uses the NEWOBJ opcode instead of the NEWOBJ_EX
serhiy.storchaka
python-checkins at python.org
Tue Dec 16 19:02:59 CET 2014
https://hg.python.org/cpython/rev/2ffaac4c8e53
changeset: 93912:2ffaac4c8e53
user: Serhiy Storchaka <storchaka at gmail.com>
date: Tue Dec 16 19:43:46 2014 +0200
summary:
Issue #22783: Pickling now uses the NEWOBJ opcode instead of the NEWOBJ_EX
opcode if possible.
files:
Lib/test/pickletester.py | 76 ++++++++++-
Lib/test/test_descr.py | 25 +--
Misc/NEWS | 3 +
Modules/_pickle.c | 25 +--
Objects/typeobject.c | 166 ++++++++++----------------
5 files changed, 152 insertions(+), 143 deletions(-)
diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py
--- a/Lib/test/pickletester.py
+++ b/Lib/test/pickletester.py
@@ -1153,16 +1153,62 @@
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 @@
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):
diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py
--- a/Lib/test/test_descr.py
+++ b/Lib/test/test_descr.py
@@ -4592,26 +4592,15 @@
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)
+ if kwargs:
+ self.assertEqual(reduce_value[0], copyreg.__newobj_ex__)
+ self.assertEqual(reduce_value[1], (type(obj), args, kwargs))
else:
- self.assertIsNone(reduce_value[3])
- if dictitems is not None:
- self.assertDictEqual(dict(reduce_value[4]), dictitems)
- 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:
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -196,6 +196,9 @@
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
diff --git a/Modules/_pickle.c b/Modules/_pickle.c
--- a/Modules/_pickle.c
+++ b/Modules/_pickle.c
@@ -3506,20 +3506,19 @@
}
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 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;
+ }
}
- else {
- _Py_IDENTIFIER(__newobj__);
- use_newobj = PyUnicode_Check(name) &&
- PyUnicode_Compare(
- name, _PyUnicode_FromId(&PyId___newobj__)) == 0;
- Py_DECREF(name);
- }
+ Py_XDECREF(name);
}
if (use_newobj_ex) {
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -3886,48 +3886,87 @@
}
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)
+ if (args == NULL) {
+ Py_XDECREF(kwargs);
return NULL;
- }
- if (kwargs == NULL) {
- kwargs = PyDict_New();
- if (kwargs == NULL)
- 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);
+ if (newargs == NULL) {
+ Py_DECREF(newobj);
+ return NULL;
+ }
+ }
+ 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;
}
- newargs = PyTuple_Pack(3, Py_TYPE(obj), args, kwargs);
- Py_DECREF(args);
- Py_DECREF(kwargs);
- if (newargs == NULL) {
- Py_DECREF(newobj);
- return NULL;
- }
+
state = _PyObject_GetState(obj);
if (state == NULL) {
Py_DECREF(newobj);
@@ -3950,79 +3989,6 @@
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 @@
{
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)
--
Repository URL: https://hg.python.org/cpython
More information about the Python-checkins
mailing list