Repository navigation
Implement PEP 3154 (pickle protocol 4) #62010
Description
| BPO | 17810 |
|---|---|
| Nosy | @tim-one, @loewis, @rhettinger, @ncoghlan, @pitrou, @larryhastings, @avassalotti, @asvetlov, @serhiy-storchaka |
| Dependencies | |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/avassalotti'
closed_at = <Date 2013-11-24.04:39:41.547>
created_at = <Date 2013-04-21.06:48:58.149>
labels = ['type-feature', 'library', 'release-blocker']
title = 'Implement PEP 3154 (pickle protocol 4)'
updated_at = <Date 2013-11-25.21:56:35.101>
user = 'https://github.com/avassalotti'bugs.python.org fields:
activity = <Date 2013-11-25.21:56:35.101>
actor = 'serhiy.storchaka'
assignee = 'alexandre.vassalotti'
closed = True
closed_date = <Date 2013-11-24.04:39:41.547>
closer = 'alexandre.vassalotti'
components = ['Library (Lib)']
creation = <Date 2013-04-21.06:48:58.149>
creator = 'alexandre.vassalotti'
dependencies = ['15397', '17893']
files = ['29966', '30068', '30072', '30094', '30118', '30213', '32640', '32709', '32840']
hgrepos = ['184']
issue_num = 17810
keywords = ['patch']
message_count = 63.0
messages = ['187496', '187500', '187510', '187512', '187516', '187830', '187833', '187834', '187874', '187876', '187877', '187878', '187891', '187896', '187918', '188087', '188089', '188090', '188096', '188098', '188102', '188109', '188227', '188280', '188281', '188282', '188312', '188315', '188320', '188327', '188330', '188331', '188338', '188351', '188363', '188889', '188971', '188989', '189017', '190554', '190583', '195499', '195522', '195583', '202939', '203339', '203420', '203435', '204066', '204067', '204088', '204089', '204093', '204097', '204175', '204176', '204389', '204390', '204391', '204397', '204399', '204401', '204426']
nosy_count = 13.0
nosy_names = ['tim.peters', 'loewis', 'rhettinger', 'ncoghlan', 'pitrou', 'larry', 'alexandre.vassalotti', 'Arfrever', 'asvetlov', 'neologix', 'python-dev', 'serhiy.storchaka', 'mstefanro']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue17810'
versions = ['Python 3.4']
Activity
avassalotti commentedon Apr 21, 2013
I have restarted the work on PEP-3154. Stefan Mihaila had begun an implementation as part of the Google Summer of Code 2012. Unfortunately, he hit multiple roadblocks which prevented him to finish his work by the end of the summer. He previously shown interest in completing his implementation. However he got constrained by time and never resumed his work.
So I am taking over the implementation of the PEP. I have decided to go forward with a brand new code, using Stefan's work only as a guide. At the moment, I have completed about half of the PEP---missing only support for calling __new__ with keyword arguments and the use of new qualified name for referring objects.
Design-wise, there is still a few things that we should discuss. For example, I think Stefan's idea, which is not specified in the PEP, to eliminate PUT opcodes is interesting. His proposal was to emit an implicit PUT opcode after each object pickled and make the Pickler and Unpickler classes agree on the scheme. A drawback of this implicit scheme is we cannot be selective about which object we save in the memo during unpickling. That means, for example, we won't be able to make pickletools.optimize work with protocol 4 to reduce the memory footprint of the unpickling process. This scheme also alters the meaning of all previously defined opcodes because of the implicit PUTs, which is sort of okay because we are changing protocol. Alternatively, we could use an explicit scheme by defining new "fat" opcodes, for the built-in types we care about, which includes memoization. This scheme would a bit more flexible however it would also be slightly more involved implementation-wise. In any case, I will run benchmarks to see if either schemes are worthwhile.
pitrou commentedon Apr 21, 2013
Thank you for reviving this :)
A couple of questions:
Are the savings worth it?
I've tried pickletools.optimize() on two objects:
a typical data dict (http.client.responses). The pickle length decreases from 1155 to 1063 (8% shrink); unpickling is faster by 4%.
a Logger object (logging.getLogger("foobar"). The pickle length decreases from 427 to 389 (9% shrink); unpickling is faster by 2%.
serhiy-storchaka commentedon Apr 21, 2013
Link to the previous attempt: bpo-15642.
serhiy-storchaka commentedon Apr 21, 2013
Memoization consumes memory during pickling. For now every memoized object requires memory for:
dict's entity;
an id() integer object;
a 2-element tuple;
a pickle's index (an integer object).
It's about 80 bytes on 32-bit platform (and twice as this on 64-bit). For data which contains a lot of floats it can be cumbersome.
pitrou commentedon Apr 21, 2013
As far as I understand, Alexandre doesn't propose to suppress
memoization, only to make it implicit. Therefore the memory overhead
would be the same (but the pickle would have less opcodes).
Apparently, floats don't get memoized:
rhettinger commentedon Apr 26, 2013
I would like to see Proto4 include an option for compression (zlib,bz2) or somesuch and become self-decompressing upon unpickling. The primary use cases for pickling involve writing to disk or transmitting across a wire -- both use cases benefit from compression (with reduced read/write times).
neologix commentedon Apr 26, 2013
I don't see what this would bring over explicit compression:
pitrou commentedon Apr 26, 2013
I agree with Charles-François.
A feature that may be actually nice to have in the pickle protocol would
be some framing, to help with streaming unpickling (right now unpickling
a stream can read almost one byte at a time, IIRC).
However, that would also make the protocol and the pickler significantly
more complex.
pitrou commentedon Apr 26, 2013
A proof of concept hack to enable framing on pickle showed a massive performance increase on streaming unpickling (up to 5x faster with a C file object such as io.BytesIO, up to 150x faster with a pure Python file object such as _pyio.BytesIO). There is a slight slowdown on non-streaming operation, but that could probably be optimized.
pitrou commentedon Apr 26, 2013
(note: I've updated PEP-3154 with framing and GLOBAL_STACK)
serhiy-storchaka commentedon Apr 26, 2013
What if just use io.BufferedReader?
(at start of _Unpickler.__init__)
pitrou commentedon Apr 26, 2013
Two problems:
semantically, it is wrong; the BufferedReader will read bytes beyond
the pickle end, so the underlying stream will be desynchronized
performance-wise, it doesn't solve the issue either: read() method
calls are costly, even on an optimized C object
39 remaining items
python-dev commentedon Nov 23, 2013
New changeset d719975f4d25 by Christian Heimes in branch 'default':
Issue bpo-17810: Add NULL check to save_frozenset
http://hg.python.org/cpython/rev/d719975f4d25
python-dev commentedon Nov 23, 2013
New changeset c54becd69805 by Christian Heimes in branch 'default':
Issue bpo-17810: return -1 on error
http://hg.python.org/cpython/rev/c54becd69805
python-dev commentedon Nov 23, 2013
New changeset a02adfb3260a by Christian Heimes in branch 'default':
Issue bpo-17810: Add two missing error checks to save_global
http://hg.python.org/cpython/rev/a02adfb3260a
python-dev commentedon Nov 23, 2013
New changeset 3e16c8c34e69 by Christian Heimes in branch 'default':
Issue bpo-17810: Fixed NULL check in _PyObject_GetItemsIter()
http://hg.python.org/cpython/rev/3e16c8c34e69
avassalotti commentedon Nov 24, 2013
I've finalized the framing implementation in de9bda43d552.
There will be more improvements to come until 3.4 final. However, feature-wise we are done. Thank you everyone for the help!
tim-one commentedon Nov 24, 2013
[Alexandre Vassalotti]
Woo hoo! Thank YOU for the hard work - I know how much fun this is ;-)
serhiy-storchaka commentedon Nov 25, 2013
Here is a patch which restores optimization for frame headers. Unfortunately it breaks test_optional_frames.
larryhastings commentedon Nov 25, 2013
Isn't it a little late to be changing the pickle protocol, now that we've hit feature-freeze? If you want to check something like this in you're going to have to make a good case for it.
serhiy-storchaka commentedon Nov 25, 2013
This doesn't change the pickle protocol. This is just an implementation detail.
avassalotti commentedon Nov 25, 2013
Optimizing the output of the pickler class should be fine during the feature freeze as long the semantics of the current opcodes stay unchanged.
pitrou commentedon Nov 25, 2013
Well, Larry may expand, but I think we don't commit performance optimizations during the feature freeze either.
("feature" is taken in the same sense as in "no new features in the bugfix branches")
larryhastings commentedon Nov 25, 2013
I'll make you a deal. As long as the protocol remains 100% backwards and forwards compatible (3.4.0b1 can read anything written by trunk, and trunk can read anything written by 3.4.0b1), you can make optimizations until beta 2. After that you have to stop... or get permission again.
serhiy-storchaka commentedon Nov 25, 2013
I have opened separate bpo-19780 for this.