classification
Title: Add Unladen Swallow's optimizations to Python 3's pickle.
Type: performance Stage: resolved
Components: Extension Modules Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: alexandre.vassalotti Nosy List: alexandre.vassalotti, belopolsky, collinwinter, haypo, jcea, pitrou, skip.montanaro
Priority: normal Keywords: patch

Created on 2010-07-29 06:26 by alexandre.vassalotti, last changed 2011-03-19 02:58 by jcea. This issue is now closed.

Files
File name Uploaded Description Edit
pickle_optimizations.diff alexandre.vassalotti, 2010-07-29 06:25
issue1694050_19001.diff alexandre.vassalotti, 2010-09-06 17:28
pickle_optimizations4.diff pitrou, 2010-09-06 17:35
pickle_optimizations5.diff pitrou, 2010-09-06 18:19
Messages (13)
msg111895 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2010-07-29 06:25
This is a big patch. Please review at http://codereview.appspot.com/1694050/show

This patch adds the most interesting optimizations from Unladen Swallow to Python 3's pickle.

The core of the patch already been reviewed by Antoine and me (http://codereview.appspot.com/33070/show). One of the last issue remaining the unbounded size of the internal buffer. This shouldn't be a big issue for most uses of pickle, since the size of a  pickle is often several times smaller than the object hierarchy that created it. I still hope to fix this in a following patch.

The patch also include additional cleanups to the Pdata structure. The changes felt natural to make along with the other changes from Unladen Swallow. IIRC, these changes yield an additional 1-3% speedup.
msg111904 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-07-29 09:41
Le jeudi 29 juillet 2010 à 06:26 +0000, Alexandre Vassalotti a écrit :
> New submission from Alexandre Vassalotti <alexandre@peadrop.com>:
> 
> This is a big patch. Please review at
> http://codereview.appspot.com/1694050/show
> 
> This patch adds the most interesting optimizations from Unladen
> Swallow to Python 3's pickle.
> 
> The core of the patch already been reviewed by Antoine and me
> (http://codereview.appspot.com/33070/show). One of the last issue
> remaining the unbounded size of the internal buffer. This shouldn't be
> a big issue for most uses of pickle, since the size of a  pickle is
> often several times smaller than the object hierarchy that created it.
> I still hope to fix this in a following patch.

I still think this should be fixed in this patch, especially since it
will also change benchmark numbers.
msg111958 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-29 16:12
For those not familiar with Unladen Swallow, can you describe what the "most interesting optimizations" are?   Maybe there is an Unladen Swallow document you can point to.  Would any of these optimizations apply to python implementation?
msg111960 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-07-29 16:31
I'm working on #3873 to add a read buffer (fixed size, 4096 bytes) to the unpickler. It's 6 to 8 times faster with the read buffer: but this patch is mainly to avoid the overhead introduced by the new I/O library (in Python2, unpickler was faster because it doesn't need to call Python functions to read some bytes). Is this feature included in this big patch?
msg112091 - (view) Author: Skip Montanaro (skip.montanaro) * (Python committer) Date: 2010-07-30 17:38
Look around the issues.  I'm pretty sure I worked on the
unbounded size issue at one point.

Skip
msg112956 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2010-08-05 08:13
Ah, that's right Skip. You did fixed it in Unladen Swallow's trunk. I will take a look at your solution.

http://code.google.com/p/unladen-swallow/source/diff?spec=svn1167&r=1038&format=side&path=/trunk/Modules/cPickle.c&old_path=/trunk/Modules/cPickle.c&old=988
msg115718 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-09-06 17:09
The patch doesn't apply cleanly anymore. Furthermore, I discovered some additional issues:
- load, dump, loads and dumps from the _pickle module were never used because they were shadowed by the same functions in pickle.py
- once the C functions above are enabled, they produce some bugs

I'm working on an updated patch, fixing the aforementioned bugs and adding a buffer size limit.
msg115723 - (view) Author: Alexandre Vassalotti (alexandre.vassalotti) * (Python committer) Date: 2010-09-06 17:28
Antoine, I fixed these issues in the latest patch posted on Rietveld. Also, Skip added the buffer limit in Unladen Swallow (see msg112956). We just need to merge that.
msg115724 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-09-06 17:35
Here is a patch. Benchmark numbers:

* dumps():

./python -m timeit -s "import pickle, io; d={(x, 'a'): x  for x in range(10000)}" "pickle.dumps(d)"

-> before: 100 loops, best of 3: 7.47 msec per loop
-> after: 100 loops, best of 3: 2.45 msec per loop

* loads():

./python -m timeit -s "import pickle, io; d={(x, 'a'): x  for x in range(10000)}; d=pickle.dumps(d)" "pickle.loads(d)"

-> before: 100 loops, best of 3: 12.1 msec per loop
-> after: 100 loops, best of 3: 2.62 msec per loop

* dump():

./python -m timeit -s "import pickle, io; d={(x, 'a'): x  for x in range(10000)}" "pickle.dump(d, io.BytesIO())"
-> before: 100 loops, best of 3: 13.2 msec per loop
-> after: 100 loops, best of 3: 2.54 msec per loop

* load():

./python -m timeit -s "import pickle, io; d={(x, 'a'): x  for x in range(10000)}; d=pickle.dumps(d)" "pickle.load(io.BytesIO(d))"
-> before: 100 loops, best of 3: 12.7 msec per loop
-> after: 100 loops, best of 3: 11.6 msec per loop


As you can see, load() doesn't really benefit from the buffering improvements. The three methods see quite massive speedups.
msg115725 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-09-06 17:36
Gosh. My patch is based on an outdated patch :(
msg115726 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-09-06 18:19
Ok, this patch merges my changes with Alexandre's previous patch. Performance is similar as the previous posted patch.
msg115771 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-09-07 15:12
For the record, here are the unladen swallow benchmark results against stock py3k:

### pickle ###
Min: 1.644222 -> 0.812691: 2.0232x faster
Avg: 1.652311 -> 0.814994: 2.0274x faster
Significant (t=297.660908)
Stddev: 0.00594 -> 0.00207: 2.8732x smaller

### unpickle ###
Min: 2.802217 -> 0.751013: 3.7312x faster
Avg: 2.807074 -> 0.752646: 3.7296x faster
Significant (t=980.311525)
Stddev: 0.00446 -> 0.00145: 3.0831x smaller

### pickle_dict ###
Min: 0.744259 -> 0.543992: 1.3681x faster
Avg: 0.747806 -> 0.545883: 1.3699x faster
Significant (t=114.070170)
Stddev: 0.00266 -> 0.00293: 1.1014x larger

### pickle_list ###
Min: 2.058899 -> 1.212566: 1.6980x faster
Avg: 2.066486 -> 1.216711: 1.6984x faster
Significant (t=269.964154)
Stddev: 0.00534 -> 0.00459: 1.1635x smaller

### unpickle_list ###
Min: 1.458531 -> 0.313154: 4.6576x faster
Avg: 1.464023 -> 0.319126: 4.5876x faster
Significant (t=425.745063)
Stddev: 0.00476 -> 0.00367: 1.2976x smaller
msg115957 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-09-09 18:33
The patch was finally committed in r84653. Thanks to everyone who participated in this.
History
Date User Action Args
2011-03-19 02:58:22jceasetnosy: + jcea
2010-09-09 18:33:58pitrousetstatus: open -> closed
resolution: fixed
messages: + msg115957

stage: patch review -> resolved
2010-09-07 15:12:40pitrousetmessages: + msg115771
2010-09-06 18:19:59pitrousetfiles: + pickle_optimizations5.diff

messages: + msg115726
2010-09-06 17:36:55pitrousetmessages: + msg115725
2010-09-06 17:35:14pitrousetfiles: + pickle_optimizations4.diff

messages: + msg115724
2010-09-06 17:28:20alexandre.vassalottisetfiles: + issue1694050_19001.diff

messages: + msg115723
2010-09-06 17:09:26pitrousetmessages: + msg115718
2010-08-05 08:17:01alexandre.vassalottilinkissue5683 superseder
2010-08-05 08:13:52alexandre.vassalottisetmessages: + msg112956
2010-08-05 07:28:00alexandre.vassalottilinkissue5671 superseder
2010-07-31 16:50:36georg.brandlsettitle: Add Unladden Swallow's optimizations to Python 3's pickle. -> Add Unladen Swallow's optimizations to Python 3's pickle.
2010-07-30 17:38:48skip.montanarosetnosy: + skip.montanaro
messages: + msg112091
2010-07-29 16:31:56hayposetmessages: + msg111960
2010-07-29 16:12:36belopolskysetmessages: + msg111958
2010-07-29 09:41:06pitrousetmessages: + msg111904
2010-07-29 06:26:05alexandre.vassalotticreate