premailer / css_parser

Ruby CSS Parser
Other
279 stars 110 forks source link

Fixup benchmark & add `RuleSet::DeclarationValue` #117

Closed ojab closed 3 years ago

ojab commented 3 years ago

Also fix rake task, .join('test/fixtures') returns invalid absolute path. Extract from https://github.com/premailer/css_parser/pull/115.

bundle exec rake benchmark results

Before:

Executing: bundle exec rake benchmark
Warming up --------------------------------------
 import1.css loading   363.000  i/100ms
 complex.css loading    16.000  i/100ms
Calculating -------------------------------------
 import1.css loading      4.087k (± 3.6%) i/s -     20.691k in   5.069769s
 complex.css loading    160.094  (± 6.9%) i/s -    800.000  in   5.026577s

Loading `import1.css` allocated 348 objects, 25 KiB
Loading `complex.css` allocated 9111 objects, 704 KiB

After:

Executing: bundle exec rake benchmark
Warming up --------------------------------------
 import1.css loading   410.000  i/100ms
 complex.css loading    15.000  i/100ms
Calculating -------------------------------------
 import1.css loading      3.947k (± 4.7%) i/s -     20.090k in   5.101062s
 complex.css loading    156.845  (± 1.9%) i/s -    795.000  in   5.070007s

Loading `import1.css` allocated 363 objects, 26 KiB
Loading `complex.css` allocated 10010 objects, 740 KiB

Overall comparable, but it's big variability.

Anyway, I'm pretty sure that it's slightly slower, but the cause is freezing/duplicating of the values, which is arguably correctness fix.

I'll try to take a look at performance after this series is merged.

grosser commented 3 years ago

looks fine to me, lmk if you are done and I'll merge, then we can have a look at perf improvements / smaller details 👍

ojab commented 3 years ago

So yeah, I think it's ready to be merged.