This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Cleanup the pure Python pickle implementation
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: alexandre.vassalotti, jcea, pitrou, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2012-11-25 12:17 by serhiy.storchaka, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
pickle_cleanup.patch serhiy.storchaka, 2012-11-25 12:17 review
bench.py serhiy.storchaka, 2013-04-07 13:25 Microbenchmark for function lookup optimization
pickle_cleanup_globals.patch serhiy.storchaka, 2013-04-08 15:13 review
pickle_cleanup_3.patch serhiy.storchaka, 2013-04-13 13:24 review
Messages (8)
msg176345 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-11-25 12:17
This issue inspired by issue12848. The proposed patch get rid of the marshal module (the struct module used instead), removes some redundant code, and changes the code to use more modern idioms.
msg182286 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-02-17 21:34
Ping.
msg186113 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-04-05 23:21
Patch looks fine to me. Do you want to go ahead?
msg186205 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-04-07 13:25
In response to Alexandre's comment on Rietveld. Access to a local variable is faster than to a global one and the current implementation uses this for struct.pack. I just use same trick for struct.unpack. Here is a microbenchmark which demonstrate some effect of this optimization. I got 0.6491418619989417, 0.6947214259998873, and 0.5394902769985492 for optimized, non-optimized and advanced optimized functions.

Of course, we can achieve even better effect if we will cache not only struct.pack, but struct.Struct('<i').pack, struct.Struct('B').pack, etc. I were considered this as a reason for other patch, but we can do it in this issue.
msg186238 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2013-04-07 19:15
My point is I would prefer that we keep all optimizations to only the _pickle C module and keep the Python implementation as simple as possible.

Also, I doubt the slight speedup shown by your microbenchmark will actually result in any significant benefits on the overall performance of pickle.py. Furthermore, we can't use CPython to measure the benefits of such optimization because CPython will always use _pickle over the Python implementation.
msg186304 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-04-08 15:13
Here is an updated patch which get rid of trick with attribute caching. Globals used instead. Also added some minor style changes (dropped redundant semicolons and wrapped too long lines).
msg186716 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-04-13 13:24
Patch updated in response to Alexandre's comments. In additional to his suggestions some other minor things simplified. _batch_appends and _batch_setitems now use islice instead range. Some bugs found and new issues created (issue17710, issue17711).
msg186906 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-04-14 10:38
New changeset 3dff836cedef by Serhiy Storchaka in branch 'default':
Closes #16551. Cleanup pickle.py.
http://hg.python.org/cpython/rev/3dff836cedef
History
Date User Action Args
2022-04-11 14:57:38adminsetgithub: 60755
2013-04-14 10:38:06python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg186906

resolution: fixed
stage: patch review -> resolved
2013-04-13 13:24:15serhiy.storchakasetfiles: + pickle_cleanup_3.patch

messages: + msg186716
2013-04-08 15:13:16serhiy.storchakasetfiles: + pickle_cleanup_globals.patch

messages: + msg186304
2013-04-07 19:15:27alexandre.vassalottisetmessages: + msg186238
2013-04-07 13:25:41serhiy.storchakasetfiles: + bench.py

messages: + msg186205
2013-04-05 23:21:08pitrousetmessages: + msg186113
2013-02-17 21:34:45serhiy.storchakasetmessages: + msg182286
2012-12-27 20:51:38serhiy.storchakasetassignee: serhiy.storchaka
2012-11-26 17:17:25jceasetnosy: + jcea
2012-11-25 12:17:36serhiy.storchakacreate