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

Performance of "extends" regex #36

Closed
mkazlauskas opened this issue May 6, 2016 · 7 comments
Closed

Performance of "extends" regex #36

mkazlauskas opened this issue May 6, 2016 · 7 comments

Comments

@mkazlauskas
Copy link

mkazlauskas commented May 6, 2016

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

rtsao commented May 6, 2016

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

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

rtsao commented May 11, 2016

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: #12

@rtsao
Copy link
Owner

rtsao commented May 17, 2016

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: #39

@scott113341
Copy link
Collaborator

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

mkazlauskas commented Jul 7, 2016

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

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

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

3 participants