rosell-dk / webp-convert

Convert jpeg/png to webp with PHP (if at all possible)
MIT License
580 stars 102 forks source link

Conversion with cwebp takes much longer with version 2.3.0+ #285

Closed mrbig00 closed 3 years ago

mrbig00 commented 3 years ago

Simply downgrading the package to 2.2.2 resolves the issue. Tested on multiple php versions and multiple servers

rosell-dk commented 3 years ago

Hm. Which converter is doing the conversion?

rosell-dk commented 3 years ago

@mrbig00: Looking at the 2.3.0 milestone, I can see that most changes are related to the cwebp converter. So I'm guessing that your active converter is cwebp?

It might be this feature that is costly. It tries executing "which -a cwebp" and "whereis -b cwebp" in order to locate path for cwebp. Can you try to disable the feature and see if that helps? You disable the feature by setting the try-discovering-cwebp option to false

rosell-dk commented 3 years ago

You can try to disable all the methods of discovering callable cwebps in the system by setting the following options to false:

You probably need at least one of them to be on, though! Look at the conversion log before turning the methods off in order to find out which of them you need to be on

mrbig00 commented 3 years ago

I'm using cwebp with cwebp-try-supplied-binary-for-os: true and the others false

rosell-dk commented 3 years ago

I have added logging of time consumption in the sub processes in Cwebp. By examining the output, we can see which sub process that takes the time.

Here is an example of how you can see the log produced on conversion: https://github.com/rosell-dk/webp-convert/blob/master/docs/v2.0/converting/introduction-for-converting.md#insights-to-the-process

rosell-dk commented 3 years ago

@mrbig00: Can you test it with the new time logging and paste the output here? Thanks

rosell-dk commented 3 years ago

The total time is the time used doing the actual conversion plus the overhead.

The time for the actual conversion varies depending on options and the image, and we cannot do anything about that. For very small images, it is as low as 10 ms on my machine. For 1MB images, it is around 200 ms.

The overhead consists of detecting available binaries, detecting their versions, ordering them in order to pick the highest version available and creating options. It is pretty constant around 70 ms on my machine when only cwebp-try-supplied-binary-for-os is enabled and around 85 ms when all methods for detecting cwebp binaries are enabled.

There are 4 supplied binaries for linux. If I comment out 3 of them, the overhead is decreased to 25 ms. Now, why does it take so long time detect a supplied binary? It turns out that it is the hash check that is expensive. It takes between 10-20 ms to create the checksum for each binary file.

Now, why not just have 1 supplied binary? The reason is that they have different dependencies. Some builds works on some systems, while others works on others. See #196 and #278.

But actually, some improvement could be made. There is no need to detect the versions of the supplied binaries, as we know what they are supposed to be beforehand. So the hash check could be postponed till right before doing the conversion. In case a cwebp binary is found using one of the other methods, and it is not an older version than the supplied binaries, no hash check will need to be run at all.

Until such optimizations are made, you could try disabling the "cwebp-try-supplied-binary-for-os" option and see if it still works. If it does, you should experience a significant performance gain.

rosell-dk commented 3 years ago

Hash-check is now postponed as described. Usually, only one hash-check will be needed now. However, if the newest binary doesn't work, an extra hash-check will be needed for the next-newest binary. It is probably pretty common that cwebp-120-linux-x86-64 doesn't work, due to its dependencies (see #278). So on many systems, two hash-checks will be needed

rosell-dk commented 3 years ago

I could add an option to ignore a supplied binary that is known not to work

rosell-dk commented 3 years ago

Added option to skip supplied binaries that are known not to work on current system. It is documented here

rosell-dk commented 3 years ago

With the first change, conversion with 2.6.0+ should be faster than 2.2.2. With the second change, you can (with a little effort) improve further, in case the newest of the precompiled binaries aren't working on your system

rosell-dk commented 3 years ago

Oh, btw: By setting an environment variable called "WEBPCONVERT_CWEBP_PATH", the discovering binaries step will be completely skipped, and there will be no hash-checking. Doing so however makes your system a little bit less secure - exactly because it bypasses the hash-checking. If some security whole allows an attacker to upload a binary, replacing the one set like this, an attacker would then have a way to have that binary executed. Also beware that by doing this, you will need to update your code in order to take advantage of future cwebp releases.

Apart from setting it as an environment variable, you can also define it like this: define("WEBPCONVERT_CWEBP_PATH", "/path/to/working/cwebp/for/example/one/in/src/Convert/Converters/Binaries/dir");

rosell-dk commented 3 years ago

@mrbig00: 2.6.0 has been released