Skip to content

use Decompress::reset() instead of recreating in DeflateDecoder#514

Merged
Byron merged 1 commit intorust-lang:mainfrom
amirshukayev:main
Dec 1, 2025
Merged

use Decompress::reset() instead of recreating in DeflateDecoder#514
Byron merged 1 commit intorust-lang:mainfrom
amirshukayev:main

Conversation

@amirshukayev
Copy link
Copy Markdown
Contributor

From https://www.zlib.net/manual.html

This function is equivalent to inflateEnd followed by inflateInit, but does not free and reallocate the internal decompression state. The The stream will keep attributes that may have been set by inflateInit2. total_in, total_out, adler, and msg are initialized.

This led to a ~14% performance improvement on my workloads with MultiGZDecoder.

Copy link
Copy Markdown
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great catch, thanks a lot!

From the Git history, I also couldn't see if this was needed or intentional at some point, so doing a reset seems like a much better choice here.

Let's have @jongiddy for a second pair of eyes on this one line change for good measure :).

@Byron Byron requested a review from jongiddy November 30, 2025 14:23
Copy link
Copy Markdown
Contributor

@jongiddy jongiddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it would be nice to know why this was changed many years ago to avoid the reset. I cannot see a problem with accepting it now.

@Byron
Copy link
Copy Markdown
Member

Byron commented Dec 1, 2025

Thank you! Then I'd say we merge it, as I'd expect to have seen 'something codified' coming with Decompress::new() that prevents future selfs from changing it back.

In the worst case we have to roll it back, which seems particularly easy in this case, in the best there is a significant performance improvement for those who have been using the MultiGZDecoder.

@Byron Byron merged commit 4015264 into rust-lang:main Dec 1, 2025
15 checks passed
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