Repository navigation
Speed up cPickle's pickling generally #49933
Description
| BPO | 5683 |
|---|---|
| Nosy | @pitrou, @avassalotti |
| Superseder | |
| 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 = None
closed_at = <Date 2010-08-05.08:17:01.552>
created_at = <Date 2009-04-04.00:02:17.156>
labels = ['extension-modules', 'performance']
title = "Speed up cPickle's pickling generally"
updated_at = <Date 2010-08-05.08:17:01.551>
user = 'https://bugs.python.org/collinwinter'bugs.python.org fields:
activity = <Date 2010-08-05.08:17:01.551>
actor = 'alexandre.vassalotti'
assignee = 'none'
closed = True
closed_date = <Date 2010-08-05.08:17:01.552>
closer = 'alexandre.vassalotti'
components = ['Extension Modules']
creation = <Date 2009-04-04.00:02:17.156>
creator = 'collinwinter'
dependencies = []
files = ['13603', '14466', '15808']
hgrepos = []
issue_num = 5683
keywords = ['patch', 'needs review']
message_count = 18.0
messages = ['85349', '90185', '90212', '95521', '95576', '97484', '97514', '97515', '97516', '97517', '97518', '97520', '97561', '97704', '97705', '97711', '97712', '112957']
nosy_count = 4.0
nosy_names = ['collinwinter', 'pitrou', 'alexandre.vassalotti', 'jcsalterego']
pr_nums = []
priority = 'normal'
resolution = 'duplicate'
stage = 'resolved'
status = 'closed'
superseder = '9410'
type = 'performance'
url = 'https://bugs.python.org/issue5683'
versions = ['Python 2.7', 'Python 3.2']
Activity
collinwinter commentedon Apr 4, 2009
This patch simplifies cPickle's complicated internal buffering system.
The new version uses a single buffer closer to the Pickler object,
flushing to a file object only when necessary. This avoids the overhead
of several indirection layers for what are frequently very small writes.
Benchmarked against virgin trunk r71058 using "perf.py -r -b
pickle,pickle_list,pickle_dict":
pickle:
Min: 2.225 -> 1.962: 13.37% faster
Avg: 2.254 -> 1.994: 13.03% faster
Significant (t=70.763434, a=0.95)
pickle_dict:
Min: 2.097 -> 1.418: 47.83% faster
Avg: 2.136 -> 1.446: 47.75% faster
Significant (t=214.900146, a=0.95)
pickle_list:
Min: 1.128 -> 0.777: 45.22% faster
Avg: 1.152 -> 0.807: 42.65% faster
Significant (t=169.789433, a=0.95)
A similar patch for unpickling will follow. As bpo-5670 and 5671 are
committed, I'll update this patch to support them.
This patch passes all the tests added in bpo-5665. I would recommend
reviewing that patch first. I'll port to py3k once this is reviewed for
trunk. This patch is available on Rietveld at
http://codereview.appspot.com/33070.
pitrou commentedon Jul 6, 2009
Any updates?
jcsalterego commentedon Jul 7, 2009
Applied collinwinter's patch and svn up'd to r73872; replaced additional
instances of self->write_func with _Pickler_Write.
Passed test_xpickle.py as referenced by bpo-5665.
pitrou commentedon Nov 20, 2009
Are you still willing to work on this?
avassalotti commentedon Nov 21, 2009
Last august, I worked on integrating Collin's optimization work into
py3k in a local Mercurial branch. So, I can champion these changes into
py3k, if Collin is unavailable.
And if Collin allows me, I would like to merge the other pickle
optimizations currently in Unladen Swallow.
smontanaro commentedon Jan 10, 2010
Updated the patch against the latest version of cPickle.c (r77393). All tests pass on my Mac.
pitrou commentedon Jan 10, 2010
There were a couple of comments on the Rietveld code review above.
smontanaro commentedon Jan 10, 2010
Antoine> There were a couple of comments on the Rietveld code review
Antoine> above.
Indeed there are. Given that the Unladen Swallow folks were focusing on the
2.6 branch and their goal was to improve performance I don't see any reason
to not accept what they've done, then tweak it for 2.7/3.1 assuming the
changes you and Alexandre suggested further improve performance.
Skip
pitrou commentedon Jan 10, 2010
The main thing I'm worried about is the potentially unbounded buffering,
since it could reduce performance (or even thrash the machine) instead
of improving it.
smontanaro commentedon Jan 10, 2010
Antoine> The main thing I'm worried about is the potentially unbounded
Antoine> buffering, since it could reduce performance (or even thrash
Antoine> the machine) instead of improving it.
Got a test case in mind? If so, I'll code it up and compare 2.6 and Unladen
Swallow as well as offer it up for inclusion in the U-S test suite.
S
pitrou commentedon Jan 10, 2010
Trying to pickle a structure that's larger than half the RAM should do
the trick. Something like a list of large strings.
pitrou commentedon Jan 10, 2010
Quick test on a 3GB machine:
Without patch ("top" shows the process reaches 1.2GB RAM max):
With the patch, the same command quickly swaps hopelessly and after 5 minutes of elapsed time I finally manage to kill the process.
smontanaro commentedon Jan 10, 2010
Antoine> With the patch, the same command quickly swaps hopelessly and
Antoine> after 5 minutes of elapsed time I finally manage to kill the
Antoine> process.
Verified with an Unladen Swallow test case. I'll see if I can fix it.
S
smontanaro commentedon Jan 13, 2010
You can fix it if you are dumping to a file, however if you are calling dumps() you are kind of screwed if dumping large objects. There's no place to flush the buffer.
I have a fix to Unladen Swallow's cPickle module. I'm run it by them before submitting it here.
pitrou commentedon Jan 13, 2010
Do we need an intermediate buffer at all when called from dumps()? How about allocating the buffer as a PyStringObject, so that it can be used directly for the result in that case?
(IIRC there's a handy _PyString_Resize function)
smontanaro commentedon Jan 13, 2010
Perhaps. Let's take it one step at a time though. If I change your
large pickle example to use dumps() instead of dump() in an unsullied
Python2.5 I get a MemoryError. In general, I think you have to be
careful using dumps(). Any attempt to solve that problem will likely
be independent of the Unladen Swallow patches.
smontanaro commentedon Jan 13, 2010
Oh, BTW, the proposed fix is in Rietveld: http://codereview.appspot.com/189051
avassalotti commentedon Aug 5, 2010
Too late to make this change in 2.x. And the patch in bpo-9410 includes the optimization for 3.x.