[issue19780] Pickle 4 frame headers optimization
Alexandre Vassalotti added the comment: Yeah, let's close this. It is much simpler to just double the frame size target if the extra reads ever become a performance issue. -- status: pending - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Antoine Pitrou added the comment: While the speedup may be nice, I still don't think this optimization complies with the protocol definition in the PEP, so I would like to reject this patch. -- resolution: - rejected stage: patch review - committed/rejected status: open - pending ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Serhiy Storchaka added the comment:
22:36:13 [ ~/PythonDev/cpython ]$ ./python.exe -m timeit import
pickle
with open('test.pickle4', 'rb', buffering=0) as f: pickle.load(f) 100
loops, best of 3: 9.28 msec per loop
Did you use the same test file or different files (generated by patched
and unpatched pickler)? Try to run this command several times and take a
minimum.
I wrote a benchmark to simulate slow reads. But again, there is no
performance difference with the patch.
Test data are too small, they all less than frame size.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19780
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Alexandre Vassalotti added the comment: Test data are too small, they all less than frame size. Ah, good point! It seems your patch helps when the reads are *very* slow and buffering is disabled. ### unpickle_file ### Min: 1.125320 - 0.663367: 1.70x faster Avg: 1.237206 - 0.701303: 1.76x faster Significant (t=30.77) Stddev: 0.12031 - 0.02634: 4.5682x smaller That certainly is a nice improvement though that seems to be fairly narrow use case. With more normal read times, the improvement diminishes greatly: ### unpickle_file ### Min: 0.494841 - 0.466837: 1.06x faster Avg: 0.540923 - 0.511165: 1.06x faster Significant (t=4.10) Stddev: 0.03740 - 0.03510: 1.0657x smaller -- Added file: http://bugs.python.org/file32900/unpickle_file_bench_2.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Antoine Pitrou added the comment: If you want a slow file object, you could benchmark with a GzipFile or similar. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Serhiy Storchaka added the comment: Frames are not overlapped. And no one opcode straddles frame boundaries. When an implementation supports optional frames, it should support optimized frames as well. All tests are passed with this optimization except test_optional_frames which hacks pickled data to remove some frame opcodes. This test relied on implementation details. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Antoine Pitrou added the comment: Bad wording perhaps, but: +if not final: +n += 9 # next frame header write = self.file_write write(FRAME) write(pack(Q, n)) does change how the frame length is calculated and emitted in the pickle stream. This is not compliant with how the PEP defines it (the frame size doesn't include the header of the next frame): http://www.python.org/dev/peps/pep-3154/#framing All tests are passed with this optimization Well, perhaps there are not enough tests :-) But the protocol is standardized so that other people can implement it. The reference implementation can't do something different than the PEP does. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
STINNER Victor added the comment: If it's an optimizatio, can I see some benchmarks numbers? :-) I would be interested of benchmarks pickle 3 vs pickle 4. -- nosy: +haypo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Serhiy Storchaka added the comment: Bad wording perhaps, but: +if not final: +n += 9 # next frame header write = self.file_write write(FRAME) write(pack(Q, n)) does change how the frame length is calculated and emitted in the pickle stream. Of course (as any optimizer). It produces more optimal pickled data which can be parsed by existing unpicklers. This is not compliant with how the PEP defines it (the frame size doesn't include the header of the next frame): http://www.python.org/dev/peps/pep-3154/#framing How the pickler decides to partition the pickle stream into frames is an implementation detail. All tests are passed with this optimization Well, perhaps there are not enough tests :-) But the protocol is standardized so that other people can implement it. The reference implementation can't do something different than the PEP does. Could you write tests which exposes behavior difference without sticking implementation details? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Serhiy Storchaka added the comment:
If it's an optimizatio, can I see some benchmarks numbers? :-)
First create two files. Run unpatched Python:
./python -c import pickle, lzma; data = [bytes([i])*2**16 for i in
range(256)]; with open('test.pickle4', 'wb'): pickle.dump(data, f, 4)
Then run the same with patched Python for 'test.pickle4opt'.
Now benchmark loading.
$ ./python -m timeit import pickle; with open('test.pickle4', 'rb',
buffering=0) as f: pickle.load(f)
10 loops, best of 3: 52.9 msec per loop
$ ./python -m timeit import pickle; with open('test.pickle4opt', 'rb',
buffering=0) as f: pickle.load(f)
10 loops, best of 3: 48.9 msec per loop
The difference is about 5%. On faster computers with slower files (sockets?) it
should be larger.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19780
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
STINNER Victor added the comment: On faster computers with slower files (sockets?) it should be larger. If your patch avoids unbuffered reads, you can test using these commands before your benchmark: sync; echo 3 /proc/sys/vm/drop_caches And use one large load() instead of running it in a loop. Or call these commands in the loop. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Antoine Pitrou added the comment: This is not compliant with how the PEP defines it (the frame size doesn't include the header of the next frame): http://www.python.org/dev/peps/pep-3154/#framing How the pickler decides to partition the pickle stream into frames is an implementation detail. Of course, but it still has to respect the frame structure shown in the ASCII art drawing. Could you write tests which exposes behavior difference without sticking implementation details? That's a good idea. At least I'll open an issue for it. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Serhiy Storchaka added the comment: If your patch avoids unbuffered reads, you can test using these commands before your benchmark: sync; echo 3 /proc/sys/vm/drop_caches Thank you. But starting Python needs a lot of I/O. This causes very unstable results for this test data. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
STINNER Victor added the comment: This causes very unstable results for this test data. So try os.system(sync; echo 3 /proc/sys/vm/drop_caches) in your timeit benchmark. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Serhiy Storchaka added the comment: Of course, but it still has to respect the frame structure shown in the ASCII art drawing. I think the ASCII art drawing is just inaccurate. It forbids optional framing (tested in test_optional_frames). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Alexandre Vassalotti added the comment:
Serhiy, I am not able to reproduce your results. I don't get any significant
performance improvements with your patch.
22:34:03 [ ~/PythonDev/cpython-baseline ]$ ./python.exe -m timeit import
pickle with open('test.pickle4', 'rb', buffering=0) as f: pickle.load(f)
100 loops, best of 3: 9.26 msec per loop
22:36:13 [ ~/PythonDev/cpython ]$ ./python.exe -m timeit import pickle with
open('test.pickle4', 'rb', buffering=0) as f: pickle.load(f)
100 loops, best of 3: 9.28 msec per loop
I wrote a benchmark to simulate slow reads. But again, there is no performance
difference with the patch.
22:24:56 [ ~/PythonDev/benchmarks ]$ ./perf.py -v -b unpickle_file
../cpython-baseline/python.exe ../cpython/python.exe
### unpickle_file ###
Min: 1.103715 - 1.103666: 1.00x faster
Avg: 1.158279 - 1.146820: 1.01x faster
Not significant
Stddev: 0.04127 - 0.03273: 1.2608x smaller
--
Added file: http://bugs.python.org/file32864/unpickle_file_bench.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19780
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
New submission from Serhiy Storchaka: Here is a patch which restores (removed at last moment before 3.4beta1 release) frame headers optimization for the pickle protocol 4. Frame header now saved as a part of previous frame. This decreases the number of unbuffered reads per frame from 3 to 1. -- components: Library (Lib) files: pickle_frame_headers.patch keywords: patch messages: 204424 nosy: alexandre.vassalotti, larry, pitrou, serhiy.storchaka priority: normal severity: normal stage: patch review status: open title: Pickle 4 frame headers optimization type: performance versions: Python 3.4 Added file: http://bugs.python.org/file32843/pickle_frame_headers.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com: -- nosy: +Arfrever ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19780] Pickle 4 frame headers optimization
Antoine Pitrou added the comment: Hmm... I thought your patch was only an optimization, but if you're overlapping frames (by adding 9 to the frame size), it becomes a protocol change. I personally am against changing the PEP at this point, especially for what looks like a rather minor win. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue19780 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
