Frommi / miniz_oxide

Rust replacement for miniz
MIT License
168 stars 48 forks source link

Don't heap-allocate callbacks #73

Closed RReverser closed 4 years ago

RReverser commented 4 years ago

Currently compress_to_output always heap-allocates a copy of the function callback, which might end up being relatively expensive, especially if it's called repetitively on a stream.

This is not strictly necessary, because the callback data has to be alive only for the lifetime of the compress_inner call, so we can use a dyn reference instead.

oyvindln commented 4 years ago

Seems fine.

Shnatsel commented 4 years ago

Out of curiosity, what's the impact of this change on benchmarks?

oyvindln commented 4 years ago

You would have to test, the compress_to_output function isn't used in any of the higher-level API functions in miniz_oxide (or the flate2 frontend), so it would mainly be if it was used manually. The compress function which takes a reference to a buffer is used instead (there probably some improvements that could be made there to avoid using more buffers than needed though.)