-
Notifications
You must be signed in to change notification settings - Fork 948
Selector: avoid calls to setAttribute( "id" ) by examining selector #431
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the general idea, but this size increase is too expensive for "a modest but consistent improvement".
raw gz Sizes
66275 19804 dist/sizzle.js
20050 7453 dist/sizzle.min.js
raw gz Compared to master @ 3116795bba9a0c3d624e0718006b25aa5568d4cb
+955 +264 dist/sizzle.js
+275 +83 dist/sizzle.min.js
But I am still interested in the size and performance results from two simpler approaches (both in isolation and together):
- making selector analysis more coarse by requiring the scope fix for any selector containing a space or greater than sign, regardless of use as combinators (e.g.,
rcomplex = /[>\x20\t\r\n\f]/
) - when possible, using
:scope
instead of a synthetic id
Thank you for your consideration! I noticed that simply extracting the scopeSelector function had a size cost:
So after inlining it again, here are the results:
Performance-wise, it seems like a toss-up. I think most of the gain is from avoiding setAttribute() and not the selectors, so :scope benefits more selectors on the browsers that support it, but selector analysis benefits more browsers. |
I concur with your assessment. Let's refocus this PR to preserve the existing id-based scope fix but use coarse regex analysis to skip it for selectors that are obviously combinator-free, and then we can revisit the possibility of using |
I updated the branch and got rid of a couple more characters. (down to +54, minified). |
An optimization added in jquery#431 skips the temporary IDs for selectors not using child or descendant combinators. There are a few issues with this optimization, though: 1. For sibling combinators, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. 2. qSA in IE/Edge doesn't find elements with an empty name attribute selector (`[name=""]`) but it does find them when there's an ID prefix for the selector (`#test [name=""]`). Therefore, skipping the ID prefix more hurts than helps. 3. After jquery/jquery#4454 & jquery#453, all modern browsers other than Edge don't get temporary IDs assigned as we leverage the :scope pseudo-class. Therefore, the optimization doesn't buy us as much. 4. The regex testing the child/descendant combinators checks for `+` and whitespace as whitespace is how a descendant selector is constructed. However, users often insert spaces to selectors for readability reasons, not knowing it would cause a performance penalty.
An optimization added in jquery#431 skips the temporary IDs for selectors not using child or descendant combinators. There are a few issues with this optimization, though: 1. For sibling combinators, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. 2. qSA in IE/Edge doesn't find elements with an empty name attribute selector (`[name=""]`) but it does find them when there's an ID prefix for the selector (`#test [name=""]`). Therefore, skipping the ID prefix more hurts than helps. 3. After jquery/jquery#4454 & jquery#453, all modern browsers other than Edge don't get temporary IDs assigned as we leverage the :scope pseudo-class. Therefore, the optimization doesn't buy us as much. 4. The regex testing the child/descendant combinators checks for `+` and whitespace as whitespace is how a descendant selector is constructed. However, users often insert spaces to selectors for readability reasons, not knowing it would cause a performance penalty.
An optimization added in jquery#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery/jquery#4454 & jquery#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes
An optimization added in jquery#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery/jquery#4454 & jquery#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes
An optimization added in jquery/sizzle#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery#4454 & jquery/sizzle#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes
An optimization added in jquery/sizzle#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery#4454 & jquery/sizzle#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes Ref jquery/sizzle#431
An optimization added in jquery#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery/jquery#4454 & jquery#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes Ref jquerygh-431
An optimization added in jquery#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery/jquery#4454 & jquery#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes. Ref jquerygh-431
An optimization added in jquery/sizzle#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery#4454 & jquery/sizzle#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes. Ref jquery/sizzle#431
An optimization added in #431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery/jquery#4454 & #453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes. Closes gh-460 Ref gh-431
An optimization added in jquery/sizzle#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after #4454 & jquery/sizzle#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes. Closes gh-4509 Ref jquery/sizzle#431
An optimization added in jquery/sizzle#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery#4454 & jquery/sizzle#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes. Ref jquerygh-4509 Ref jquery/sizzle#431
An optimization added in jquery/sizzle#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery#4454 & jquery/sizzle#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes. Ref jquerygh-4509 Ref jquery/sizzle#431
An optimization added in jquery/sizzle#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery#4454 & jquery/sizzle#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes. Ref jquerygh-4509 Ref jquery/sizzle#431
An optimization added in jquery/sizzle#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery#4454 & jquery/sizzle#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes. Ref jquerygh-4509 Ref jquery/sizzle#431
An optimization added in jquery/sizzle#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery#4454 & jquery/sizzle#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes. Ref jquerygh-4509 Ref jquery/sizzle#431
An optimization added in jquery/sizzle#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery#4454 & jquery/sizzle#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes. Ref jquerygh-4509 Ref jquery/sizzle#431
An optimization added in jquery/sizzle#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery#4454 & jquery/sizzle#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes. Ref jquerygh-4509 Ref jquery/sizzle#431
An optimization added in jquery/sizzle#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery#4454 & jquery/sizzle#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes. Ref jquerygh-4509 Ref jquery/sizzle#431
An optimization added in jquery/sizzle#431 skips the temporary IDs for selectors not using child or descendant combinators. For sibling combinators, though, this pushes a selector with a leading combinator to qSA directly which crashes and falls back to a slower Sizzle route. This commit makes selectors with leading combinators not skip the selector rewriting. Note that after jquery#4454 & jquery/sizzle#453, all modern browsers other than Edge leverage the :scope pseudo-class, avoiding temporary id attributes. Ref jquerygh-4509 Ref jquery/sizzle#431
Additionally, use :scope when available.
This implements the optimization proposed in issue 430. I modified the official benchmark to use
document.documentElement
so the relevant code path would be reached, and it has shown a modest but consistent improvement in every browser I've tested. It can be found here.In practice, I've seen this change more than halve the load times of some pages in Edge, which seems to have particularly slow
setAttribute
calls.I don't know if this is the best possible order in which to try alternatives, but it is the best I could come up with. Using the selector verbatim when possible appears to be the fastest across browsers by a small margin. I haven't been able to find a clear winner between
:scope
andgetAttribute("id")
.:scope
is probably not able to be shared with CSS or with other qSA calls, but#id
should be, and my hope is that this advantage outweighs the cost of callinggetAttribute
.