Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

End-to-end decoding benchmarks of paletted PNG images. #453

Merged
merged 1 commit into from
Jan 20, 2024

Conversation

anforowicz
Copy link
Contributor

@anforowicz anforowicz commented Jan 12, 2024

No description provided.

@anforowicz
Copy link
Contributor Author

@fintelia, can you PTAL?

Special thanks to @Shnatsel for pointing out the mistake I made in my earlier attempt to cover expand_paletted in end-to-end benchmarks. Thanks!

@fintelia
Copy link
Contributor

Could we use a smaller test image? 8 MB is pretty large for a file checked into the git repo

@Shnatsel
Copy link
Contributor

FWIW the larger the decompressed image is, the more the palette expansion pass dominates the runtime. On 1024x1024 images it's about 20% of the time, on the map image it's about 40%, on the zune-png benchmarking image shared at the start of #393 it's about 60%.

I think it is possible to create an image e.g. with large areas of flat fill that would be small when compressed, decompress into an image with large dimensions, and have the palette pass account for a large portion of the image decoding time.

Or maybe the palette pass should just be benchmarked separately.

@Shnatsel
Copy link
Contributor

I suggest using this image instead: Stadt_Onex_2021_posterized_med

It still spends 40% of the time in palette expansion while only being 1.4MB in size (as opposed to the 8MB image proposed originally).

@anforowicz
Copy link
Contributor Author

anforowicz commented Jan 13, 2024

FWIW, I feel that it is desirable to have a somewhat realistic paletted test image that also highlights the performance of expand_paletted. FWIW using palette reduces the size of the image from 9,408,318 bytes to 8,396,692 (and so it seems reasonable that this optimization would be used in the "real world" + shows that the original "real world" image is even bigger).

OTOH, if we decide that 8MB is unreasonable, then we can try testing with existing benchmarks and a function-level benchmark that I am working on at https://github.com/anforowicz/image-png/tree/palette-benchmarks-func2. We would still want to land the current PR to ensure that expand_paletted is actuall covered by the benchmarks. (I think that other set of changes is ready for review, but I'll hold off with opening a pull request until we've figure out what to do with the current one under review. This is mostly because the performance measurements in the other one use the current PR as the baseline.)

Edit: Ooops... it seems that I missed @Shnatsel's reply above, before posting my comment... :-/

@anforowicz
Copy link
Contributor Author

I suggest using this image instead: Stadt_Onex_2021_posterized_med

It still spends 40% of the time in palette expansion while only being 1.4MB in size (as opposed to the 8MB image proposed originally).

Thanks - let me switch to this image.

@anforowicz anforowicz force-pushed the palette-benchmarks-end2end branch from b49f246 to 9703b07 Compare January 13, 2024 01:17
@anforowicz
Copy link
Contributor Author

anforowicz commented Jan 13, 2024

I suggest using this image instead: Stadt_Onex_2021_posterized_med
It still spends 40% of the time in palette expansion while only being 1.4MB in size (as opposed to the 8MB image proposed originally).

Thanks - let me switch to this image.

Actually, I wasn't able to find the image that you are referring to. But maybe this is fine - I just grabbed https://commons.wikimedia.org/wiki/File:Stadt_Onex_2021.png at a slightly smaller resolution and converted to png:IHDR.color_type: 3 (Indexed) using ImageMagick. The resulting file has 1,146,578 bytes and png::decoder::expand_paletted takes 22.84% of time.

@anforowicz
Copy link
Contributor Author

@fintelia, can you PTAL again?

@fintelia fintelia merged commit ed54082 into image-rs:master Jan 20, 2024
19 checks passed
@anforowicz anforowicz deleted the palette-benchmarks-end2end branch February 2, 2024 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants