Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1622)

Issue 33070: Speed up cPickle's internal output machinery

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 2 months ago by Collin Winter
Modified:
10 years, 2 months ago
Base URL:
http://svn.python.org/view/*checkout*/python/trunk/
Visibility:
Public.

Patch Set 1 #

Patch Set 2 : Remove unneeded changes #

Total comments: 11
Unified diffs Side-by-side diffs Delta from patch set Stats (+193 lines, -410 lines) Patch
M Modules/cPickle.c View 1 68 chunks +193 lines, -410 lines 11 comments Download

Messages

Total messages: 3
Antoine Pitrou
http://codereview.appspot.com/33070/diff/3/4 File Modules/cPickle.c (left): http://codereview.appspot.com/33070/diff/3/4#oldcode2863 Line 2863: return NULL; Nice simplication there :) http://codereview.appspot.com/33070/diff/3/4 File ...
10 years, 2 months ago (2009-04-04 11:46:24 UTC) #1
Alexandre Vassalotti
Apart the minor issues I saw, I think the patch is good. Just add a ...
10 years, 2 months ago (2009-04-05 08:58:15 UTC) #2
Antoine Pitrou
10 years, 2 months ago (2009-04-05 15:20:02 UTC) #3
http://codereview.appspot.com/33070/diff/3/4
File Modules/cPickle.c (right):

http://codereview.appspot.com/33070/diff/3/4#newcode348
Line 348: 
Thinking about it, building lots of small PyStrings could have a non-negligible
overhead, while the asymptotic complexity is O(N) in both cases. So I withdraw
my suggestion.

(limiting the buffer size would still be nice, though)

http://codereview.appspot.com/33070/diff/3/4#newcode452
Line 452: self->output_len);
What you could do to avoid the copy would be to create a memoryview object
pointing to the internal buffer instead.
However, it would only work in trunk and py3k (2.6 doesn't have it), and I'm not
sure it would be accepted by all file-like objects.
See
http://code.python.org/hg/branches/py3k/file/e42bcf320c14/Modules/_io/buffere...
if you want some example of doing it.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b