-
Notifications
You must be signed in to change notification settings - Fork 20.6k
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
elem.is(':visible') calls native webkitMatchesSelector #2042
Comments
We don't try to parse the selector in JS until we run it through the native method. If the native method throws an exception, we try parsing it ourselves. The only other way we could know to avoid it would be to parse the string ourselves, which would penalize people who use just standard selectors. In the talks I do, I strongly recommend against |
Great. In this and with Wikipedia's new visual editor, a tremendous amount of the app's slowness is directly attributable to using those. It's baaad. But anyway, moving on.
I see that now. In the case of obviously invalid selectors, it'd be better to not even go down the patch of bouncing off the exception. Follow the total time column above. The overhead to throw an extra regex test along with the other two here is going to be sub-millisecond on this timescale. And since we're already running two regex's against the string, the overall VM overhead will be minimal. |
I'll pull in @timmywil and @gibson042 here, maybe we could do a quick regex to look for obvious offenders? Discussing the issue in IRC, perhaps we should move things like |
This suggestion is essentially to match custom pseudos in As for removing custom pseudos from standard builds, I see |
I'm a little surprised it takes that long for webkitMatchesSelector to parse the selector and determine whether it's invalid. I wonder if that can be improved natively. |
Could we cache the fact that the selector failed |
Hmm, that's not a bad idea. @gibson042 ? |
Yeah, I like that a lot. It's not a panacea when the selectors theirselves are different, but it avoids a lot of pain for little effort. |
Issue opened here. That should address this issue fairly well with little cost, as gibson said. We'll close this issue when we merge in that Sizzle update. |
@paulirish You'll be happy to know that we now see a 1600% improvement on |
Wow. Amazing! |
nice one! that's awesome to see.
|
Wow! This is so cool! |
Yesterday I spent some time profiling the effect shown in this article:
http://engineeringblog.yelp.com/2015/01/animating-the-mobile-web.html (and here's the live m.yelp page)
During profiling I some interesting things, but two of which are specific to jQuery.
webKitMatchesSelector
is being called duringelem.is(':visible')
2 ) jQuery handler for
touchmove
seems very slow. (This one we can punt to another ticket.)A closer look at the
is
tomatchesSelector
stack:Here's the
a.rl
method we're looking at:In total, this stack from
a.rl
throughwebKitMatchesSelector
accounts for a full 15% of all time on CPU (including JS, recalc, paint, etc) during this effect, so certainly not trivial.But this still strikes me as odd in the first place.
I don't think we would ever want a native qSA or matchesSelector to handle this sort of call, would we?
The text was updated successfully, but these errors were encountered: