Pass image data directly to ZlibStream, bypassing ChunkState::raw_bytes
#424
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This helps with unifying how image data from IDAT and fdAT is handled. This unification is a prerequisite for the follow-up commit where we will avoid temporarily copying compressed image data into `current_chunk.raw_bytes`.
…bytes`. Before these changes we would store all image data in `ChunkState::raw_bytes` before attempting to decompress them via `ZlibStream`. This is undesirable for performance, because 1) it copies data unnecessarily into and out of `raw_bytes` and 2) by the time we have stored all the image data, the start of it may be already gone from the L1 cache. After these changes, the image data is passed directly into `ZlibStream`. This commit is part of a bigger set of changes that avoids copying image pixels across different buffers. This commit alone gives notable although relatively small performance improvements, but the real motivation here is to make progress toward merging the bigger set of changes and realizing their performance gains. For more information see the "Revisiting copy avoidance" part of the discussion at image-rs#416 (comment) Performance impact of this commit (2 measurement attempts): * `decode/8x8-noncompressed.png` - runtime improved by 9.18% - 9.99% * `128x128-noncompressed.png` - runtime improved by 7.89% - 8.51% * `decode/Transparency.png` - runtime improved by 1.92% - 2.69% No impact on the other test inputs: * `decode/kodim02.png` - change within noise threshold (same result when rerun) * `decode/kodim07.png` - no change detected (p = 0.06 or 0.17 > 0.05) * `decode/kodim17.png` - runtime improved by 1.75% (change within noise threshold when rerun) * `decode/kodim23.png` - runtime regressed by 2.15% (no change detected when rerun - p = 0.86 > 0.05) * `Lohengrin_-_Illustrated_Sporting_and_Dramatic_News.png` - change within noise threshold (no change detected when rerun - p = 0.14 > 0.05)
fintelia
reviewed
Nov 12, 2023
The move will help with follow-up work, where we want to use generated PNGs in unit tests of `src/decoder/stream.rs`.
fintelia
reviewed
Nov 14, 2023
anforowicz
force-pushed
the
idat-straight-to-zlibstream
branch
from
dfc989e
to
2bca55c
Compare
fintelia
reviewed
Nov 14, 2023
This has accidentally regressed in a recent commit and was caught during PR review.
anforowicz
force-pushed
the
idat-straight-to-zlibstream
branch
from
2bca55c
to
e75843f
Compare
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PTAL?
This is a part of the "Revisiting copy avoidance" part of the discussion at #416 (comment).