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

Bypass BufReader and state machine when decoding #451

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

fintelia
Copy link
Contributor

This sketches out how we could switch away from our current BufRead approach of taking slices of input to drive a state machine. It instead takes a Read impl and pull bytes from it as needed.

Leaving this draft state because there's APNG decoding isn't fully working at the moment and because I'm not fully sure we want to go down this path of duplicating the decoding logic between the low-level streaming API and the higher-level decoding API.

Performance is mixed with both significant improvements and regressions. It seems to vary based on the constants driving the minimum and maximum IDAT buffer sizes passed to the zlib decompressor.

decode/kodim17.png
                        time:   [-2.7008% -2.6371% -2.5847%] (p = 0.00 < 0.05)

decode/kodim07.png
                        time:   [+0.6333% +1.3371% +2.5938%] (p = 0.00 < 0.05)

decode/kodim02.png
                        time:   [+0.8983% +1.0315% +1.1734%] (p = 0.00 < 0.05)

decode/kodim23.png
                        time:   [+0.1002% +0.1620% +0.2178%] (p = 0.00 < 0.05)

decode/Lohengrin_-_Illustrated_Sporting_and_Dramatic_News.png
                        time:   [-1.0207% -0.6634% -0.3293%] (p = 0.00 < 0.05)

decode/Transparency.png time:
                        time:   [+77.678% +78.337% +78.794%] (p = 0.00 < 0.05)

generated-noncompressed-4k-idat/8x8.png
                        time:   [-44.694% -44.589% -44.510%] (p = 0.00 < 0.05)

generated-noncompressed-4k-idat/128x128.png
                        time:   [+28.799% +29.139% +29.482%] (p = 0.00 < 0.05)
...

generated-noncompressed-2g-idat/12288x12288.png
                        time:   [-21.915% -20.856% -19.853%] (p = 0.00 < 0.05)

@anforowicz
Copy link
Contributor

Is it fair to say that the buffer inside BufReader is replaced with the idat_stream: Vec<u8>?

@fintelia
Copy link
Contributor Author

fintelia commented Jan 22, 2024

Yeah, I think that's a reasonable summary. And then #458 extends this further to eliminate the idat_stream Vec

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.

None yet

2 participants