matthiasmullie / minify

CSS & JavaScript minifier, in PHP. Removes whitespace, strips comments, combines files (incl. @import statements and small assets in CSS files), and optimizes/shortens a few common programming patterns.
https://matthiasmullie.github.io/minify/
MIT License
1.96k stars 310 forks source link

Big performance loss in CSS-minify #405

Closed wvhn closed 9 months ago

wvhn commented 1 year ago

Hi,

I have been using an older version of the minifier from Mid 2016 - most likely v1.3.36 (couldn't find a version no. in the sources). Since the js-minifier produced faulty code in a specific situation I had to update to a newer version - expecting better compatibility w/ newer php versions, too.

My product is a web-based smarthome visualization framework which often runs on Rapberry Pi. Testing in this environment first broke my pages completely. PHP timed out during CSS-minify so I had to increase max_execution_time to 45 seconds. Investigating the effects more closely I found that script execution time increases massively from 6.2 to 36.5 seconds working on 11 files with approx 250kB in total size. PHP version on the Raspi is v7.3. On a modern PC w/ PHP v8.0 the performance loss was still around 30%.

On the contrary, performance of js-minify has slightly improved with the new version.

I can send the CSS-files for testing, if needed.

Best regards Wolfram

wvhn commented 1 year ago

night.css.txt

This file seems to make the difference. It takes approx. 30 seconds to minify on a Raspberry Pi 2.

wvhn commented 1 year ago

While v1.3.60 is still good v1.3.61 brings the massive loss of performance. I'll continue testing by checking out the individual commits between v1.3.60 and 1.3.61.

wvhn commented 1 year ago

https://github.com/matthiasmullie/minify/commit/17888a49462bfc135fe180f018d18b3424b001c6 with this commit, performance of CSS minify was still OK. All later versions are much slower. There seem to be some duplicate commits after this one which I don't understand.

live627 commented 1 year ago

@wvhn my pull request might help; can you try it?

https://github.com/matthiasmullie/minify/pull/407

wvhn commented 1 year ago

Thanks, John. Unfortunately the code doesn't improve performance for the data I am minimizing. There are not so much color codes in my CSS files.

Your PR brought me to the idea to check the "slow" file I had provided above. There is a massive use of comments in this file and if I deactivate comment stripping I see that this function consumes 85% of the total minification time for the whole set of CSS files in the actual version (49 sec of 56 sec total). In v1.3.60 it takes only 10% (0.8 sec of 9 sec total).

live627 commented 1 year ago

I see that there is a pull request that changes the regex for removing comments https://github.com/matthiasmullie/minify/pull/330. I haven't tried it, so I don't know if it is more performant.

Is your data teh CSS file that you uploaded earlier?

wvhn commented 1 year ago

Yes. It's part of a set of files but I figured out that this one is consuming most of the minimization time and shows the most significant differences in performance between the two minify versions (approx. 6 vs. 36 seconds on a Raspberry Pi 2).

live627 commented 1 year ago

I linked the wrong PR; the right one is https://github.com/matthiasmullie/minify/pull/318 (may also fix https://github.com/matthiasmullie/minify/issues/403).

I tested your CSS against various sources

wvhn commented 1 year ago

Great job, John! Thanks a lot for providing a working merge of this fix for testing. I can confirm your results:

318 solves the CSS minify performance issue. Minify times are now back to the former values - even slightly better. It improves performance on JS minify as well and solves #403.

Inspection of the CSS output file shows a reasonable structure. JS minify delivers line breaks after each closed if-statement but I see this also with earlier versions of minify.

I'll leave the issue open until the fix is officially merged to master.

live627 commented 1 year ago

it is now merged