Incrementally flush large cPickle output to file

In http://bugs.python.org/issue5683 Antoine Pitrou added a
comment requesting that the cPickle buffer be limited in size
to avoid chewing up all of memory when pickling very large
objects.  This patch (ignore the Makefile.pre.in diff - I'm 
still new at this) implements the incremental flush operation
when pickling to a file.  There's no obvious (to me) place to
put the growing pickle when calling cPickle.dumps.

[Skip, 2010-01-13 10:19:18]

PTAL.

[Skip, 2010-01-13 11:46:06]

Here's a revised patch.  No spurious Makefile.pre.in diffs either.

[Antoine Pitrou, 2010-01-13 13:08:11]

http://codereview.appspot.com/189051/diff/7/1001
File Modules/cPickle.c (right):

http://codereview.appspot.com/189051/diff/7/1001#newcode692
Modules/cPickle.c:692: look at the entire pickled object */
You could probably do an early test on `n >= PICKLER_MAX_BUFSIZE` as well. If
the chunk to be written is big, there is no point in buffering it and flushing
it immediately after.

http://codereview.appspot.com/189051/diff/7/1001#newcode693
Modules/cPickle.c:693: self->uses_gets = 1;
I suppose this is unladen-swallow-specific? I see no trace of it in mainline
cPickle.

[Skip, 2010-01-13 13:47:16]

http://codereview.appspot.com/189051/diff/7/1001
File Modules/cPickle.c (right):

http://codereview.appspot.com/189051/diff/7/1001#newcode692
Modules/cPickle.c:692: look at the entire pickled object */
Sure, but this seems ever-so-slightly easier to get correct.

http://codereview.appspot.com/189051/diff/7/1001#newcode693
Modules/cPickle.c:693: self->uses_gets = 1;
Correct.  See _Pickler_Optimize.  It removes PUT ops if the pickle doesn't use
any GET ops.  Given that I amy possibly flushing GET ops to the file and thus
making them unavailable to the optimizer I simply pretend that the pickle does
use GET.  That just defeats the optimizer when it's called at the end.  I'm sure
someone smarter than me could solve that problem.  I'm not intimiately familiar
with the pickle virtual machine.

[Skip, 2010-01-13 13:48:59]

http://codereview.appspot.com/189051/diff/7/1001
File Modules/cPickle.c (right):

http://codereview.appspot.com/189051/diff/7/1001#newcode692
Modules/cPickle.c:692: look at the entire pickled object */
On 2010/01/13 13:47:16, Skip wrote:
> Sure, but this seems ever-so-slightly easier to get correct.

Done.

[collinwinter, 2010-01-13 17:29:42]

What is the performance impact of this change? Can you run `perf.py -r -b
pickle,pickle_list,pickle_dict` to gauge the impact of the patch?

[skip_pobox.com, 2010-01-13 23:32:35]

    Collin> What is the performance impact of this change? Can you run
    Collin> `perf.py -r -b pickle,pickle_list,pickle_dict` to gauge the
    Collin> impact of the patch?

Will do once I'm plugged back in.  No doubt it will cost something.
Hopefully not a lot.

Skip

[Skip, 2010-01-13 23:34:58]

Report on Darwin montanaro.dyndns.org 9.8.0 Darwin Kernel Version 9.8.0: Wed Jul
15 16:55:01 PDT 2009; root:xnu-1228.15.4~1/RELEASE_I386 i386 i386
Total CPU cores: 2

### pickle ###
Min: 1.617623 -> 1.614978: 1.0016x faster
Avg: 1.636064 -> 1.637322: 1.0008x slower
Not significant
Stddev: 0.01834 -> 0.01879: 1.0250x larger
Timeline: http://tinyurl.com/yb9quqf

### pickle_dict ###
Min: 1.570521 -> 1.581051: 1.0067x slower
Avg: 1.597166 -> 1.603687: 1.0041x slower
Not significant
Stddev: 0.03379 -> 0.02593: 1.3031x smaller
Timeline: http://tinyurl.com/ybxrr2z

### pickle_list ###
Min: 1.010477 -> 1.036962: 1.0262x slower
Avg: 1.022406 -> 1.078777: 1.0551x slower
Significant (t=-24.624309, a=0.95)
Stddev: 0.01708 -> 0.01525: 1.1202x smaller
Timeline: http://tinyurl.com/ycngmcx

[collinwinter, 2010-01-24 04:59:51]

LGTM. I agree that the performance degradation is acceptable, assuming all the
test_cpickle and test_xpickle tests pass.

I've added you to our committers list. Can you just check this in yourself?
Thanks for the patch!