Use (new) buffer protocol in Pickle.decode on Python 2
#150
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
On Python 3, `pickle.loads` is able to take anything that conforms to the (new) buffer protocol. So there has been no issue giving it an `ndarray` to work with. Unfortunately, on Python 2, `pickle.loads` requires a `bytes` object specifically and is not able to take any type implementing the (new) buffer protocol. So we have been going ahead and coercing everything to `bytes` on Python 2. However it turns out that `cStringIO`'s `StringIO` on Python 2 does support the buffer protocol. Thus a `StringIO` object can be created on Python 2 without copying the data. While this still cannot be used with `pickle.loads`, it can be used with `pickle.load`, which special cases reading from `StringIO` leveraging the read function, which amounts to sharing a pointer between `StringIO` and `pickle.loads`. Thus achieving a no copying unpickler for Python 2. ref: http://www.hydrogen18.com/blog/unpickling-buffers.html ref: https://github.com/python/cpython/blob/2.7/Modules/cStringIO.c#L716 ref: https://github.com/python/cpython/blob/2.7/Modules/cStringIO.c#L681 ref: https://github.com/python/cpython/blob/2.7/Modules/cPickle.c#L614 ref: https://github.com/python/cpython/blob/2.7/Modules/cStringIO.c#L160
jakirkham
changed the title
Pickle.decodePickle.decode on Python 2
8 tasks
alimanfoo
approved these changes
Dec 3, 2018
Member
alimanfoo
left a comment
alimanfoo
left a comment
There was a problem hiding this comment.
More good stuff! Thanks, LGTM.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On Python 3,
pickle.loadsis able to take anything that conforms to the (new) buffer protocol. So there has been no issue giving it anndarrayto work with. Unfortunately, on Python 2,pickle.loadsrequires abytesobject specifically and is not able to take any type implementing the (new) buffer protocol. So we have been going ahead and coercing everything tobyteson Python 2.However it turns out that
cStringIO'sStringIOon Python 2 does support the buffer protocol. Thus aStringIOobject can be created on Python 2 without copying the data. While this still cannot be used withpickle.loads, it can be used withpickle.load, which special cases reading fromStringIOleveraging the read function, which amounts to sharing a pointer betweenStringIOandpickle.loads. Thus achieving a no copying unpickler for Python 2.ref: http://www.hydrogen18.com/blog/unpickling-buffers.html
ref: https://github.com/python/cpython/blob/2.7/Modules/cStringIO.c#L716
ref: https://github.com/python/cpython/blob/2.7/Modules/cStringIO.c#L681
ref: https://github.com/python/cpython/blob/2.7/Modules/cPickle.c#L614
ref: https://github.com/python/cpython/blob/2.7/Modules/cStringIO.c#L160
TODO:
tox -e py37passes locallytox -e py27passes locallytox -e docspasses locally