homepage

Stefan Mihaila has been working on the implementation of PEP 3154, plus some other enhancements. His work is pretty complete and ready to be reviewed. I will do my best to finish a thorough review of his changes by the end of next week.

I get warnings when compiling with the patch:
/home/avassalotti/pickle4/Modules/_pickle.c: In function ‘save_global_binary’:
/home/avassalotti/pickle4/Modules/_pickle.c:2952: warning: pointer targets in passing argument 2 of ‘_Pickler_Write’ differ in signedness
/home/avassalotti/pickle4/Modules/_pickle.c:872: note: expected ‘const char *’ but argument is of type ‘unsigned char *’
/home/avassalotti/pickle4/Modules/_pickle.c:2956: warning: pointer targets in passing argument 2 of ‘_Pickler_Write’ differ in signedness
/home/avassalotti/pickle4/Modules/_pickle.c:872: note: expected ‘const char *’ but argument is of type ‘unsigned char *’
/home/avassalotti/pickle4/Modules/_pickle.c:2960: warning: pointer targets in passing argument 2 of ‘_Pickler_Write’ differ in signedness
/home/avassalotti/pickle4/Modules/_pickle.c:872: note: expected ‘const char *’ but argument is of type ‘unsigned char *’
/home/avassalotti/pickle4/Modules/_pickle.c:2975: warning: pointer targets in passing argument 2 of ‘_Pickler_Write’ differ in signedness
/home/avassalotti/pickle4/Modules/_pickle.c:872: note: expected ‘const char *’ but argument is of type ‘unsigned char *’
/home/avassalotti/pickle4/Modules/_pickle.c:2979: warning: pointer targets in passing argument 2 of ‘_Pickler_Write’ differ in signedness
/home/avassalotti/pickle4/Modules/_pickle.c:872: note: expected ‘const char *’ but argument is of type ‘unsigned char *’
/home/avassalotti/pickle4/Modules/_pickle.c:2988: warning: pointer targets in passing argument 2 of ‘_Pickler_Write’ differ in signedness
/home/avassalotti/pickle4/Modules/_pickle.c:872: note: expected ‘const char *’ but argument is of type ‘unsigned char *’
/home/avassalotti/pickle4/Modules/_pickle.c:3012: warning: pointer targets in passing argument 2 of ‘_Pickler_Write’ differ in signedness
/home/avassalotti/pickle4/Modules/_pickle.c:872: note: expected ‘const char *’ but argument is of type ‘unsigned char *’
/home/avassalotti/pickle4/Modules/_pickle.c: In function ‘save_set’:
/home/avassalotti/pickle4/Modules/_pickle.c:3368: warning: suggest parentheses around assignment used as truth value
/home/avassalotti/pickle4/Modules/_pickle.c: In function ‘save_frozenset’:
/home/avassalotti/pickle4/Modules/_pickle.c:3422: warning: suggest parentheses around assignment used as truth value
These should be fixed.

There are reference leaks in the _pickle.c part that will need to be fixed too. 
22:36:29 [~/pickle4]$ ./python -m test.regrtest -R :: test_pickle
[1/1] test_pickle
beginning 9 repetitions
123456789
.........
test_pickle leaked [14780, 14780, 14780, 14780] references, sum=59120
1 test failed:
 test_pickle
[319952 refs]

Maybe we could postpone the review process for a few days
until I fix some known issues

Oops, wrong patch. Uploading the right one.

Maybe you can set this issue as the superseder of issue9269, because the patches there have already been applied here.

Is this patch stable? Or are there further changes coming?

There are still some upcoming changes.

Some quick thoughts about the new implicit memoization scheme in Stefan's implementation. 
- The new scheme will need to be documented in PEP 3154 before we can accept the change.
- I don't really like the idea of changing the semantics of the PUT and GET opcodes. I would prefer new opcodes if possible.
- I would like to see benchmarks for this change.

>- I don't really like the idea of changing the semantics of the PUT and GET opcodes. I would prefer new opcodes if possible.
Well, the semantics of PUT and GET haven't really changed. It's just that the PUT opcode is not generated anymore and memoization is done "in agreement" (i.e. both the pickler and the unpickler know when to memoize so their memo tables stay in sync). So, in fact, it's the semantics of the other opcodes that has slightly changed.
>- I would like to see benchmarks for this change.
I've tried the following two snippets with timeit:
 ./python3.3 -m timeit \
 -s 'from pickle import dumps' \
 -s 'd=["a"]*100'
 'dumps(d,3)' # replace 3 with 4 for comparison
 ./python3.3 -m timeit \
 -s 'from pickle import dumps' \
 -s 'd=list(map(chr,range(0,256)))' \
 'dumps(d,3)' # replace 3 with 4 for comparison
 # you can also use loads(dumps(d,3)) here to benchmark both 
 # operations at once
The first one generates 99 BINGET opcodes. It generates 1 BINPUT opcode in pickle3 and no BINPUT opcodes in pickle4.
There appears no noticeable speed difference.
The second one generates no BINGET opcodes. It generates no BINPUT opcodes in v4, respectively 256 BINPUT opcodes in v3. It appears the v4 one is slightly faster, but I have a hard time comparing correctly, given that the measurements seem to have a very large standard deviation (v4 gets times somewhere between 32.3 and 44.2, whereas v3 gets times between 37.7 and 52.2).
I'm not sure this is the best way to benchmark, so let me know what is usually used.

Le mercredi 22 août 2012 à 15:49 +0000, Stefan Mihaila a écrit :
> I'm not sure this is the best way to benchmark, so let me know what is
> usually used.
http://hg.python.org/benchmarks/ has pickling benchmarks, you may have
to modify them to test different pickling protocols.

Are there also some known techniques on tracking down memory leaks?
I've played around with sys.gettotalrefcount to narrow down
the place where the leaks occur, but they seem to only occur in v4,
i.e. pickle.dumps(3.0+1j, 4) leaks but pickle.dumps(3.0+1j, 3) does
not.
However, there appears to be no difference in the code that gets
executed in v3 to the one executed in v4.

> Are there also some known techniques on tracking down memory leaks?
Nothing more than the usual debugging techniques. It is more of a matter of taste whether you like to add print() (or printf ;-)) calls, or set breakpoints in an actual debugger.
> i.e. pickle.dumps(3.0+1j, 4) leaks but pickle.dumps(3.0+1j, 3) does
not.
Well it looks like you've narrowed things down a bit here.
> However, there appears to be no difference in the code that gets
> executed in v3 to the one executed in v4.
Even the differences in memoization?

I have started a new implementation of PEP 3154 since Stefan hasn't been active on his. Moving the discussion to Issue #17810.

Hello. I apologize once again for not finalizing my work, but once I have started my final year of faculty and a job, I have been busy pretty much all the time. I would really like to finish this as I've really enjoyed working on it, and everything on PEP 3154 and some other stuff has already been implemented. The only remaining part was finalizing the code review and fixing some memory leaks that gave me some headaches at the time.
I would really appreciate if you could give me a few more days before deciding to start a new implementation from scratch. I'll get to fixing those memory leaks in the next couple of days and then the code review can be finalized.
Would this be acceptable to you?