[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

[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 _

[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 f

[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

[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 ___ __

[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: p

[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). -- _

[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 __

[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.

[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 > impleme

[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

[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 ru

[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(" > does change how

[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 ___

[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("http://www.python.org/dev/peps/pep-3

[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

[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. --

[issue19780] Pickle 4 frame headers optimization

Changes by Arfrever Frehtes Taifersar Arahesis : -- nosy: +Arfrever ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscri

[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.