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

libdeflater 0.14.0 and minor changes #495

Closed
wants to merge 8 commits into from

Conversation

parasew
Copy link

@parasew parasew commented Apr 8, 2023

Updated libdeflater from 0.11.0 to 0.13.0. Zopfli now at 0.7.2. Clap from 3.2.20 -> 3.2.23. Tested extensively and here are reproducible performance improvements and demo

Updated libdeflater from 0.11.0 to 0.13.0. Zopfli now at 0.7.2. Clap from 3.2.20 -> 3.2.23. Tested extensively and here are [reproducible performance improvements and demo](https://gist.github.com/parasew/8a4fc4bd9253b7270d908e0a23a75d7e)
@XhmikosR
Copy link
Contributor

XhmikosR commented Apr 9, 2023

Just wondering, should you also update Cargo.toml since these are important updates?

BTW I could add cargo in #492 too :)

@andrews05
Copy link
Collaborator

andrews05 commented Apr 16, 2023

Thanks for doing this! Note that the new version of libdeflate is a bit slower which will affect the performance curve of levels 2-6. Hopefully it won't be too bad though.

[edit] The lock file must also be updated, you can run cargo update to do this.

@TPS
Copy link

TPS commented May 17, 2023

@shssoichiro Is this still waiting on changes, or would you be ok to merge? 🙇🏾‍♂️

@andrews05
Copy link
Collaborator

andrews05 commented May 18, 2023

Just needs tests to re-run, they should pass now.
Although... @AlexTMjugador has been making further performance improvements to zopfli - are you planning another release soon?

[edit] Oh, it also needs rebasing as there are merge conflicts.

@AlexTMjugador
Copy link
Collaborator

AlexTMjugador commented May 18, 2023

Although... @AlexTMjugador has been making further performance improvements to zopfli - are you planning another release soon?

I'd like to do a release once I'm done studying, applying and/or improving on some of the ideas that ECT used to speed up Zopfli without negatively affecting its compression. For the next release after that, Zopfli should have a solid single-threaded performance, so I plan to enable multi-threaded compression by focusing on the necessary traits and interface rework. I'd like to use rayon for this multi-threading support, so its thread management can integrate well with applications like OxiPNG. I don't have a timeline for this though.

@shssoichiro
Copy link
Owner

Looks like this is having some merge conflicts and test failures

shssoichiro pushed a commit that referenced this pull request May 28, 2023
* Update Zopfli and several other depenencies

I've just published a new Zopfli release, v0.7.3, which includes several
new features and internal refactors. Performance should be a tad bit
better, but I didn't test it throughly, so YMMV. Perhaps more
importantly for OxiPNG, its dependency tree is smaller, and
Gzip-exclusive compression code can be excluded at compile time thanks
to new feature switches. As I mentioned on
#495 (comment),
I plan on delivering more significant Zopfli performance improvements at
some point, but for now I think it's good to give the new release more
real-world usage and testing 😄

While at it, I've upgraded other dependencies that are not
performance-critical to their latest semver-compatible versions. This
excludes `libdeflater` on purpose, as its performance characteristics
are said to be somewhat different.

* Update Zopfli to v0.7.4

v0.7.3 was superseeded shortly after v0.7.3 was released to address a
last minute change to the new API it introduced. OxiPNG is not affected
by this, but I think it's good practice to update Zopfli anyway.
@parasew parasew changed the title libdeflater 0.13.0 and minor changes libdeflater 0.14.0 and minor changes May 29, 2023
@parasew
Copy link
Author

parasew commented May 29, 2023

Updated all the crates to recent versions. The most relevant change is libdeflater 0.14.0. Everything merged and up to date with master. All tests passing.

andrews05 pushed a commit to andrews05/oxipng that referenced this pull request Jun 1, 2023
* Update Zopfli and several other depenencies

I've just published a new Zopfli release, v0.7.3, which includes several
new features and internal refactors. Performance should be a tad bit
better, but I didn't test it throughly, so YMMV. Perhaps more
importantly for OxiPNG, its dependency tree is smaller, and
Gzip-exclusive compression code can be excluded at compile time thanks
to new feature switches. As I mentioned on
shssoichiro#495 (comment),
I plan on delivering more significant Zopfli performance improvements at
some point, but for now I think it's good to give the new release more
real-world usage and testing 😄

While at it, I've upgraded other dependencies that are not
performance-critical to their latest semver-compatible versions. This
excludes `libdeflater` on purpose, as its performance characteristics
are said to be somewhat different.

* Update Zopfli to v0.7.4

v0.7.3 was superseeded shortly after v0.7.3 was released to address a
last minute change to the new API it introduced. OxiPNG is not affected
by this, but I think it's good practice to update Zopfli anyway.
@andrews05
Copy link
Collaborator

andrews05 commented Jun 23, 2023

Just to provide some benchmarks for this. 290 images totalling 52,401,011 bytes.

Level master time PR time master size PR size
-o1 (10) 0:29.91 0:29.52 32,264,654 32,200,234
-o2 (11) 0:46.72 0:51.57 31,233,176 31,145,675
-o3 (11) 1:05.93 1:12.00 31,094,663 31,012,645
-o4 (12) 1:33.75 1:49.35 30,966,883 30,903,273

Yeah, it's a bit slower, but I think it's worthwhile. (That is, I don’t think any levels need rebalancing)

@shssoichiro There's a number of PRs building up, can we get some of these moving? 🙂

@TPS
Copy link

TPS commented Jun 24, 2023

@shssoichiro There's a number of PRs building up, can we get some of these moving? 🙂

&, please, after those, could there be a full version release w/ binaries? 🙇🏾‍♂️

@AlexTMjugador
Copy link
Collaborator

AlexTMjugador commented Jun 24, 2023

indexmap v2.0.0 was just released. It would be a good thing to update this PR to also bump that dependency, which should be relatively easy to do 😄

@andrews05
Copy link
Collaborator

I realised Clap is also long out of date, with v4 being released last year. This required significant changes to update, so I posted a new PR #525 which updates everything.

@shssoichiro
Copy link
Owner

Made obsolete by #525

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.

6 participants