rtsao / csjs

:sparkles: Modular, scoped CSS with ES6
MIT License
576 stars 32 forks source link

Performance of "extends" regex #36

Closed mkazlauskas closed 8 years ago

mkazlauskas commented 8 years ago

Current way of parsing extends is way too slow. It takes up most of my (small) app's load time. Here is a screenshot from Chrome's Timeline. Looks like it takes 95%+ of the whole script eval.

Wrapping regex parse in

if (css.indexOf('extends') > 0) {
    while (found = regex.exec(css)) {
      matches.unshift(found);
    }
}

reduced page load time by about 1.3 seconds. I tried removing every single extends from my scss, it doesn't really matter, the pattern is just slow by itself.

rtsao commented 8 years ago

Wow, that's pretty extreme. Out of curiosity, how large is the CSJS template string you are using?

I haven't really done any benchmarking yet so I'm sure I there's lots of room for optimization. The current implementation is very simple. I can probably make it a lot faster, I just hadn't run into a problem myself yet.

mkazlauskas commented 8 years ago

I have about 15 calls to csjs with average of about 20 lines per each.

I'm trying to improve regex. This seems to work much, much faster: \.([^\s]+)(\s+)(extends\s+)((?:\.[^{]+))

It parses some whitespace differently but so far it seems to be working in the same way. Didn't have time to get into actual code or test if it works with every extends in my app. Let me know if you can identify any cases it doesn't cover.

rtsao commented 8 years ago

I think there's some easy (and possibly really big) performance wins to be had by doing a bit of simple lexical analysis (i.e. splitting the CSS into tokens). That should allow for simpler regex.

But first I'm going to set up some benchmarking tooling: https://github.com/rtsao/csjs/issues/12

rtsao commented 8 years ago

Thank you so much for you contribution of your faster regex! I couldn't find any cases where it wasn't working and all tests were passing, so I've merged it in and released it as v1.0.3

I've created an issue for general performance optimization here: https://github.com/rtsao/csjs/issues/39

scott113341 commented 8 years ago

@mkazlauskas, I'm working on some benchmarking tools right now, would you mind posting some of the rules you were working with when you had the severe performance issues? Thanks!

mkazlauskas commented 8 years ago

@scott113341 Sorry I can't provide anything specific since I replaced csjs with css modules. I wish I stayed with csjs though, much more flexible.

It should be fairly easy to reproduce - I don't think I used anything special as my css skills are pretty amateur, just a good number of extends rules. It should really depend on number of lines as opposed to specific rules.

scott113341 commented 8 years ago

Ok, no worries. Thanks @mkazlauskas, we'll make sure to test extends-heavy stylesheets!