[issue5670] Speed up pickling of dicts in cPickle

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Thanks!

> Committed as r72909 (trunk), r72910 (py3k).
> 
> --
> resolution: accepted -> fixed
> status: open -> closed

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Collin Winter]

Collin Winter  added the comment:

Fixed the len(d) == 1 size regression. Final performance of the patch
relative to trunk:

Using Unladen Swallow's perf.py -b pickle,pickle_dict on trunk:
pickle:
Min: 2.238 -> 1.895: 18.08% faster
Avg: 2.241 -> 1.898: 18.04% faster
Significant (t=282.066701, a=0.95)

pickle_dict:
Min: 2.163 -> 1.375: 57.36% faster
Avg: 2.168 -> 1.376: 57.50% faster
Significant (t=527.668441, a=0.95)


Performance for py3k:
pickle:
Min: 2.849 -> 2.790: 2.10% faster
Avg: 2.854 -> 2.796: 2.09% faster
Significant (t=27.624303, a=0.95)

pickle_dict:
Min: 2.121 -> 1.512: 40.27% faster
Avg: 2.128 -> 1.519: 40.13% faster
Significant (t=283.406572, a=0.95)


regrtest.py -uall test_xpickle passes all backwards-compatibility tests
for trunk, and all other tests run by regrtest.py on Linux pass.

Committed as r72909 (trunk), r72910 (py3k).

--
resolution: accepted -> fixed
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Sorry, it won't even be integrated in 2.6 actually. It's a new feature,
not a bug fix.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Kelvin Liang]

Kelvin Liang  added the comment:

Can this patch be used or ported to 2.5.x?

--
nosy: +feisan

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Amaury Forgeot d'Arc]

Amaury Forgeot d'Arc  added the comment:

Sorry, I was wrong. I think I noticed that the case size==1 was handled 
differently, and incorrectly inferred the same for size==0.
(btw, the patch for trunk was not updated)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Collin Winter]

Collin Winter  added the comment:

Amaury, I can't reproduce the issue you're seeing with empty dicts.
Here's what I'm doing:

dhcp-172-19-19-199:trunk collinwinter$ ./python.exe 
Python 2.7a0 (trunk:71100M, Apr  3 2009, 14:40:49) 
[GCC 4.0.1 (Apple Inc. build 5490)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import cPickle, pickletools
>>> data = cPickle.dumps({}, protocol=2)
>>> pickletools.dis(data)
0: \x80 PROTO  2
2: }EMPTY_DICT
3: .STOP
highest protocol among opcodes = 2
>>> data
'\x80\x02}.'
>>>

What are you doing to produce the MARK SETITEMS sequence?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Collin Winter]

Collin Winter  added the comment:

FYI, I just added a pickle_dict microbenchmark to perf.py. Using this
new microbenchmark, I see these results (perf.py -r -b pickle_dict):

pickle_dict:
Min: 2.092 -> 1.341: 56.04% faster
Avg: 2.126 -> 1.360: 56.37% faster
Significant (t=216.895643, a=0.95)

I still need to address the comment about pickling empty dicts.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Alexandre Vassalotti]

Changes by Alexandre Vassalotti :


Removed file: 
http://bugs.python.org/file13597/pickle_batch_dict_exact_py3k-4.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Alexandre Vassalotti]

Changes by Alexandre Vassalotti :


Removed file: 
http://bugs.python.org/file13596/pickle_batch_dict_exact_py3k-3.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Alexandre Vassalotti]

Alexandre Vassalotti  added the comment:

Sigh... silly me again. There is some other junk in my last patch.

--
Added file: http://bugs.python.org/file13598/pickle_batch_dict_exact_py3k-5.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Alexandre Vassalotti]

Alexandre Vassalotti  added the comment:

Silly me, I had changed the PyDict_Size call in outer loop for Py_SIZE
and this is of course totally wrong. Here's a good patch (I am pretty
sure now! ;-) I ran the whole test suite and I saw no failures.

Collin, you can go ahead and commit both patches. Nice work!

--
assignee:  -> collinwinter
keywords: +patch
resolution:  -> accepted
stage:  -> commit review
Added file: http://bugs.python.org/file13597/pickle_batch_dict_exact_py3k-4.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Alexandre Vassalotti]

Changes by Alexandre Vassalotti :


Removed file: 
http://bugs.python.org/file13594/pickle_batch_dict_exact_py3k-2.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Alexandre Vassalotti]

Changes by Alexandre Vassalotti :


Removed file: http://bugs.python.org/file13593/pickle_batch_dict_exact_py3k.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Alexandre Vassalotti]

Alexandre Vassalotti  added the comment:

Oops again, I just remarked that the comment for batch_dict_exact refers
to batch_dict as being above, but I copied batch_dict_exact before
batch_dict. Here's a good patch (hopefully) that puts batch_dict_exact
at the right place.

--
keywords:  -patch
versions: +Python 3.1
Added file: http://bugs.python.org/file13596/pickle_batch_dict_exact_py3k-3.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Alexandre Vassalotti]

Alexandre Vassalotti  added the comment:

Oops, I forgot to add the comment on top of batch_dict_exact in the
patch. Here is a better patch.

--
Added file: http://bugs.python.org/file13594/pickle_batch_dict_exact_py3k-2.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Alexandre Vassalotti]

Alexandre Vassalotti  added the comment:

I ported the patch to py3k. In addition, I added a special-case when the
dict contains only one item; you probably want this special-case in the
trunk version as well.

--
nosy: +alexandre.vassalotti
Added file: http://bugs.python.org/file13593/pickle_batch_dict_exact_py3k.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Antoine Pitrou]

Antoine Pitrou  added the comment:

> Antoine: pickletester.py:test_newobj_generic() appears to test dict
> subclasses, though in a roundabout-ish way. I don't know of any tests
> for dict subclasses in the C level sense (ie, PyDict_Check() vs
> PyDict_CheckExact()). I can add more explicit tests for Python-level
> dict subclasses, if you want.

Well, Python-level dict subclasses are also C-level subclasses (in the
PyDict_Check() sense), or am I mistaken?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Amaury Forgeot d'Arc]

Amaury Forgeot d'Arc  added the comment:

The patch produces different output for an empty dict: a sequence "MARK 
SETITEMS" is written, which is useless and wastes 2 bytes.

--
nosy: +amaury.forgeotdarc

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Collin Winter]

Collin Winter  added the comment:

Antoine: pickletester.py:test_newobj_generic() appears to test dict
subclasses, though in a roundabout-ish way. I don't know of any tests
for dict subclasses in the C level sense (ie, PyDict_Check() vs
PyDict_CheckExact()). I can add more explicit tests for Python-level
dict subclasses, if you want.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Antoine Pitrou]

Antoine Pitrou  added the comment:

> Certainly; see http://bugs.python.org/issue5671 for the list version.
> It doesn't make as big an impact on the benchmark, though.

How about splitting the benchmark in parts:
- (un)pickling lists
- (un)pickling dicts
- (un)pickling sets
(etc.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Collin Winter]

Collin Winter  added the comment:

On Thu, Apr 2, 2009 at 12:20 PM, Antoine Pitrou  wrote:
>
> Antoine Pitrou  added the comment:
>
> By the way, could the same approach be applied to lists and sets as well?

Certainly; see http://bugs.python.org/issue5671 for the list version.
It doesn't make as big an impact on the benchmark, though.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Antoine Pitrou]

Antoine Pitrou  added the comment:

By the way, could the same approach be applied to lists and sets as well?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Without taking a very detailed look, the patch looks good.
Are there already tests for pickling of dict subclasses? Otherwise, they
should be added.

--
nosy: +pitrou

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue5670] Speed up pickling of dicts in cPickle

[Collin Winter]

New submission from Collin Winter :

The attached patch adds another version of cPickle.c's batch_dict(),
batch_dict_exact(), which is specialized for "type(x) is dict". This
provides a nice performance boost when pickling objects that use
dictionaries:

Pickle:
Min: 2.216 -> 1.858: 19.24% faster
Avg: 2.238 -> 1.889: 18.50% faster
Significant (t=106.874099, a=0.95)

Benchmark is at
http://code.google.com/p/unladen-swallow/source/browse/tests/performance/macro_pickle.py
(driver is  ../perf.py; perf.py was run with "--rigorous -b pickle").

This patch passes all the tests added in issue 5665. I would recommend
reviewing that patch first. I'll port to py3k once this is reviewed for
trunk.

--
components: Extension Modules
files: cpickle_dict.patch
keywords: needs review, patch
messages: 85239
nosy: collinwinter
severity: normal
status: open
title: Speed up pickling of dicts in cPickle
type: performance
versions: Python 2.7
Added file: http://bugs.python.org/file13584/cpickle_dict.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com