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

Big performance loss in CSS-minify #405

Closed
wvhn opened this issue Dec 25, 2022 · 10 comments
Closed

Big performance loss in CSS-minify #405

wvhn opened this issue Dec 25, 2022 · 10 comments

Comments

@wvhn
Copy link

wvhn commented Dec 25, 2022

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
Copy link
Author

wvhn commented Dec 25, 2022

night.css.txt

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

@wvhn
Copy link
Author

wvhn commented Jan 10, 2023

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
Copy link
Author

wvhn commented Jan 10, 2023

17888a4
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
Copy link

live627 commented Mar 1, 2023

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

#407

@wvhn
Copy link
Author

wvhn commented Mar 1, 2023

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
Copy link

live627 commented Mar 1, 2023

I see that there is a pull request that changes the regex for removing comments #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
Copy link
Author

wvhn commented Mar 1, 2023

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
Copy link

live627 commented Mar 2, 2023

I linked the wrong PR; the right one is #318 (may also fix #403).

I tested your CSS against various sources

@wvhn
Copy link
Author

wvhn commented Mar 2, 2023

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
Copy link

live627 commented May 4, 2023

it is now merged

@wvhn wvhn closed this as completed Dec 3, 2023
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

No branches or pull requests

2 participants