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.

Sign up for GitHub

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

Jump to bottom

Never uses more than 6 threads #169

Closed
Cygon opened this issue Apr 15, 2019 · 8 comments
Closed

Never uses more than 6 threads #169

Cygon opened this issue Apr 15, 2019 · 8 comments
Labels
I-Minor Issues that would be nice to have fixed, but are the lowest priority T-Docs Issues for adding or improving documentation

Comments

@Cygon
Copy link

Cygon commented Apr 15, 2019

Oxipng only ever uses 18% of my CPU power, or about 6 threads.

My command line is:

        /opt/oxipng/target/release/oxipng \
                --opt 6 \
                --zw 32k \
                --zopfli \
                --threads 24 \
                *.png

I've tried with and without the --threads 24 option, it doesn't seem to change anything. Are there only 6 parameter combinations at those settings, is it using a limited thread pool or misdetecting my cpu count (default is supposedly 1.5x cpu cores, so if it somehow couldn't detect and fell back to a fixed value of 4...)

This is on Gentoo Linux, Kernel 4.19.27, system with two Xeon E5-2680 CPUs.
@shssoichiro shssoichiro added I-Medium Issues that are breaking core functionality, but have a known workaround T-Bug Some piece of the software is not working as intended labels Apr 16, 2019
@Calinou
Copy link
Contributor

Calinou commented Jun 18, 2019

To make better use of multiple CPU threads, you can run oxipng in parallel on multiple images using the GNU parallel tool (or its Rust reimplementation):

parallel oxipng -o6 --strip --zopfli ::: **/*.png

@shssoichiro
Copy link
Owner

Normally -o 6 would have 180 combinations which would easily max out the thread pool. But zopfli is less configurable than zlib, which means the number of trials we can do when using zopfli is fewer--6, specifically.

The help page specifies that --zopfli "overrides zlib-specific options", but it may be worth updating that text to clarify what that means.

@shssoichiro shssoichiro added I-Minor Issues that would be nice to have fixed, but are the lowest priority T-Docs Issues for adding or improving documentation and removed I-Medium Issues that are breaking core functionality, but have a known workaround T-Bug Some piece of the software is not working as intended labels Jun 25, 2019
@TPS
Copy link

TPS commented Jul 25, 2020

Perhaps (optionally?) using a Zopfli fork (e.g., https://github.com/MrKrzYch00/zopfli) would be helpful?

@TPS TPS mentioned this issue Aug 11, 2020
Closed
@TPS
Copy link

TPS commented Mar 16, 2021

Leanify just got proper parallel processing support. Is there anything there that'd be useful here, particularly the use of TaskFlow?

@TPS
Copy link

TPS commented Feb 13, 2022

Would glommio be useful?

@TPS TPS mentioned this issue May 10, 2023
Open
7 tasks
@TPS TPS mentioned this issue Jun 22, 2023
Closed
@TPS
Copy link

TPS commented Jun 23, 2023 •
edited
Loading

#517 (comment) → #461

[S]ince #169 was opened we've added new filter types, so it will use up to 10 threads now at -o 6.

@andrews05 andrews05 mentioned this issue Jul 9, 2023
Merged
shssoichiro pushed a commit that referenced this issue Jul 11, 2023
@andrews05
Process files in parallel (#531)
4ae64c5 
This PR makes the oxipng binary process multiple files in parallel,
finally fulfilling #275. There seemed to be some debate about whether
oxipng _should_ do this or not but there's a couple of reasons I think
it makes sense:
1. The concern seemed mostly around the complexity of such a feature.
Not to worry, it was trivial* 🙂
2. Since then, oxipng has dropped from a max of something like 180
simultaneous compression trials down to 10, which is very much a good
thing but it does mean it's not utilising any more cores than that.

Some benchmarks on around 100 files on a machine with 8 cores:
Level | Master time | PR time
-|-|-
2 | 28.303 | 19.005
3 | 36.507 | 23.089
5 | 1:10.86 | 1:16.01

*Some additional changes were required in order to make sure sensible
output is printed to the terminal, since things won't be in order
anymore. Here's some example output from before:
```
Processing: tests/files/fully_optimized.png
    file size = 67 bytes (0 bytes = 0.00% decrease)
File already optimized
Processing: tests/files/corrupted_header.png
Invalid PNG header detected
Processing: tests/files/verbose_mode.png
    file size = 102480 bytes (12228 bytes = 10.66% decrease)
Output: tests/files/verbose_mode.png
```
And after:
```
Processing: tests/files/verbose_mode.png
Processing: tests/files/fully_optimized.png
Processing: tests/files/corrupted_header.png
tests/files/corrupted_header.png: Invalid PNG header detected
tests/files/fully_optimized.png: Could not optimize further, no change written
102480 bytes (10.66% smaller): tests/files/verbose_mode.png
```

Closes #275, #84, #169, #196 and #419.

[edit] This is the last thing I wanted to land before the next release 🥳
@ace-dent
Copy link

ace-dent commented Oct 8, 2023

@Cygon @TPS - Is this still an issue? Should the docs be updated with a suggested maximum threads?

@andrews05
Copy link
Collaborator

andrews05 commented Oct 8, 2023 •
edited
Loading

I think we can close this. Parallel processing is also coming in the next release, so it will take full advantage of all available threads when running on many files.
(We should do a good cleanup of issues once v9 releases)

@ace-dent ace-dent mentioned this issue Nov 7, 2023
Open
Pr0methean pushed a commit to Pr0methean/oxipng that referenced this issue Dec 1, 2023
@andrews05 @Pr0methean
Process files in parallel (shssoichiro#531)
003f700 
This PR makes the oxipng binary process multiple files in parallel,
finally fulfilling shssoichiro#275. There seemed to be some debate about whether
oxipng _should_ do this or not but there's a couple of reasons I think
it makes sense:
1. The concern seemed mostly around the complexity of such a feature.
Not to worry, it was trivial* 🙂
2. Since then, oxipng has dropped from a max of something like 180
simultaneous compression trials down to 10, which is very much a good
thing but it does mean it's not utilising any more cores than that.

Some benchmarks on around 100 files on a machine with 8 cores:
Level | Master time | PR time
-|-|-
2 | 28.303 | 19.005
3 | 36.507 | 23.089
5 | 1:10.86 | 1:16.01

*Some additional changes were required in order to make sure sensible
output is printed to the terminal, since things won't be in order
anymore. Here's some example output from before:
```
Processing: tests/files/fully_optimized.png
    file size = 67 bytes (0 bytes = 0.00% decrease)
File already optimized
Processing: tests/files/corrupted_header.png
Invalid PNG header detected
Processing: tests/files/verbose_mode.png
    file size = 102480 bytes (12228 bytes = 10.66% decrease)
Output: tests/files/verbose_mode.png
```
And after:
```
Processing: tests/files/verbose_mode.png
Processing: tests/files/fully_optimized.png
Processing: tests/files/corrupted_header.png
tests/files/corrupted_header.png: Invalid PNG header detected
tests/files/fully_optimized.png: Could not optimize further, no change written
102480 bytes (10.66% smaller): tests/files/verbose_mode.png
```

Closes shssoichiro#275, shssoichiro#84, shssoichiro#169, shssoichiro#196 and shssoichiro#419.

[edit] This is the last thing I wanted to land before the next release 🥳
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
I-Minor Issues that would be nice to have fixed, but are the lowest priority T-Docs Issues for adding or improving documentation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@Calinou @TPS @ace-dent @Cygon @andrews05 @shssoichiro