Test speedups - #10042
Test speedups#10042akx wants to merge 6 commits into
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
… data A test for sequence errors doesn't need compressed data
| default_image=True, | ||
| append_images=frames, | ||
| ) | ||
| assert test_file.read_bytes().count(b"fdAT") > 2 |
There was a problem hiding this comment.
I know we have a difference of opinion on this, but I still don't see minor performance improvements in the test suite as a reason to start monkeypatching. I would rather test Pillow as it is with a real image.
There was a problem hiding this comment.
Pillow is not being monkeypatched. MAXBLOCK is a documented public API, this just sets it temporarily.
This is the same pattern as used in e.g. test_padded_idat in test_file_png.
There was a problem hiding this comment.
...when I said 'monkeypatching', I was referring to monkeypatch.setattr(ImageFile, "MAXBLOCK", 1024)
#5493 did that in order to avoid a large test image being stored permanently in the repository.
This change is about avoiding handling a 4160x870 image in memory, which doesn't seem that extreme to me.
There was a problem hiding this comment.
Right - here MAXBLOCK is being set to force writing smaller chunks to satisfy the premise of their test.
I didn't think about it also affecting reading, so if you'd like, we could undo the patch before reading them file back in? EDIT: Did that in a subsequent commit.
And, EDIT:
This change is about avoiding handling a 4160x870 image in memory, which doesn't seem that extreme to me.
No, it's not that extreme, but spending those cycles is also unnecessary for satisfying the premise of the test. Another option to speed up the test (while not doing anything about the memory cost) is to use the large image, and tell the encoder to not waste time compressing it.

On my machine, this speeds up the entire test suite by ~8% on average, when running on all cores. (hyperfine)
test_apng_save_split_fdatwas spending time compressing data, but the premise of the test is to see whether large data is split across chunks correctly. The test now uses smaller data and a smallMAXCHUNK, and tests the premise.test_parser()is split up with parametrization (or in the explicitly failing case of PDFs, into a separate test...), so the different formats run in parallel (Run tests in parallel with pytest-xdist #9945 and all that).convert_to_comparableusedget_flattened_datato convert a palette image to a list of numbers, just so it could be again interpreted asLbytes. Can just as well go with the bytes themselves (and Improve memory usage and performance fortobytes()#9938 will speed that up too).assert_image_similarnow usesImageChops.difference()to compute the difference map between 8bpp images in one multiband sweep. (This got faster in Speed up ImageChops operations #9738.) (ImageChopsfunctions don't do non-8bpp at all; I might take a look at that.) Since many of the tests end up doingassert_image_similar, that's a nice win. (Improve memory usage and performance fortobytes()#9938 will speed upassert_image_equal.) This is probably the source of most of the speedup here. :)