Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2139843002:
Allow [id=X] selectors to use the querySelector ID fast path. (Closed)
|
Created:
4 years, 5 months ago by esprehn Modified:
4 years, 5 months ago CC:
blink-reviews, blink-reviews-dom_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, rwlbuis, sof Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAllow [id=X] selectors to use the querySelector ID fast path.
This removes 300ms of input latency when clicking the input boxes in the
Mint transaction table on a Reks chromebook and 800ms when clicking
anything on Zillow on a Reks.
On a 2013 MacPro this saves 30-100ms when clicking on those websites.
querySelector(All) supports a fast path which transforms calls with
selectors like "#foo" and ".foo #bar" into a getElementById call
followed by a matches() call against the element and the selector.
This patch allows using that getElementById path when writing selectors
like "[id=foo]" or ".foo [id=foo]". Selectors of this type are used by
the YUI framework and pages like Zillow and Mint often execute a thousand
calls to one of these selectors per click event.
BUG=627242
Committed: https://crrev.com/696789dfa7c62bef99966bbe2e0a41109e636a84
Cr-Commit-Position: refs/heads/master@{#404933}
Patch Set 1 #Patch Set 2 : fix assert. #
Total comments: 2
Messages
Total messages: 30 (19 generated)
The CQ bit was checked by esprehn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by esprehn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Allow [id=X] selectors to use the querySelector ID fast path. This removes 300ms of input latency when clicking the input boxes in the mint transaction table on a Reks chromebook and 800ms when clicking anything on Zillow on a Reks. On a 2013 MacPro this saves 30-100ms when clicking on those websites. BUG=627242 ========== to ========== Allow [id=X] selectors to use the querySelector ID fast path. This removes 300ms of input latency when clicking the input boxes in the mint transaction table on a Reks chromebook and 800ms when clicking anything on Zillow on a Reks. On a 2013 MacPro this saves 30-100ms when clicking on those websites. querySelector(All) supports a fast path which transforms calls with selectors like "#foo" and ".foo #bar" into a getElementById call followed by a matches() call against the element and the selector. This patch allows using that getElementById path when writing selectors like "[id=foo]" or ".foo [id=foo]". Selectors of this type are used by the YUI framework and pages like Zillow and Mint often execute a thousand calls to one of these selectors per click event. BUG=627242 ==========
Description was changed from ========== Allow [id=X] selectors to use the querySelector ID fast path. This removes 300ms of input latency when clicking the input boxes in the mint transaction table on a Reks chromebook and 800ms when clicking anything on Zillow on a Reks. On a 2013 MacPro this saves 30-100ms when clicking on those websites. querySelector(All) supports a fast path which transforms calls with selectors like "#foo" and ".foo #bar" into a getElementById call followed by a matches() call against the element and the selector. This patch allows using that getElementById path when writing selectors like "[id=foo]" or ".foo [id=foo]". Selectors of this type are used by the YUI framework and pages like Zillow and Mint often execute a thousand calls to one of these selectors per click event. BUG=627242 ========== to ========== Allow [id=X] selectors to use the querySelector ID fast path. This removes 300ms of input latency when clicking the input boxes in the Mint.com transaction table on a Reks chromebook and 800ms when clicking anything on Zillow on a Reks. On a 2013 MacPro this saves 30-100ms when clicking on those websites. querySelector(All) supports a fast path which transforms calls with selectors like "#foo" and ".foo #bar" into a getElementById call followed by a matches() call against the element and the selector. This patch allows using that getElementById path when writing selectors like "[id=foo]" or ".foo [id=foo]". Selectors of this type are used by the YUI framework and pages like Zillow and Mint often execute a thousand calls to one of these selectors per click event. BUG=627242 ==========
Description was changed from ========== Allow [id=X] selectors to use the querySelector ID fast path. This removes 300ms of input latency when clicking the input boxes in the Mint.com transaction table on a Reks chromebook and 800ms when clicking anything on Zillow on a Reks. On a 2013 MacPro this saves 30-100ms when clicking on those websites. querySelector(All) supports a fast path which transforms calls with selectors like "#foo" and ".foo #bar" into a getElementById call followed by a matches() call against the element and the selector. This patch allows using that getElementById path when writing selectors like "[id=foo]" or ".foo [id=foo]". Selectors of this type are used by the YUI framework and pages like Zillow and Mint often execute a thousand calls to one of these selectors per click event. BUG=627242 ========== to ========== Allow [id=X] selectors to use the querySelector ID fast path. This removes 300ms of input latency when clicking the input boxes in the Mint transaction table on a Reks chromebook and 800ms when clicking anything on Zillow on a Reks. On a 2013 MacPro this saves 30-100ms when clicking on those websites. querySelector(All) supports a fast path which transforms calls with selectors like "#foo" and ".foo #bar" into a getElementById call followed by a matches() call against the element and the selector. This patch allows using that getElementById path when writing selectors like "[id=foo]" or ".foo [id=foo]". Selectors of this type are used by the YUI framework and pages like Zillow and Mint often execute a thousand calls to one of these selectors per click event. BUG=627242 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
esprehn@chromium.org changed reviewers: + rune@chromium.org, sashab@chromium.org
This is awesome!!! LGTM https://codereview.chromium.org/2139843002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/SelectorQuery.cpp (right): https://codereview.chromium.org/2139843002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/SelectorQuery.cpp:460: return selector; Would it be worth adding a StyleResolverStats for when this fast path is triggered?
https://codereview.chromium.org/2139843002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/SelectorQuery.cpp (right): https://codereview.chromium.org/2139843002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/SelectorQuery.cpp:460: return selector; On 2016/07/12 at 01:37:03, sashab wrote: > Would it be worth adding a StyleResolverStats for when this fast path is triggered? It would be worth adding some metrics, but StyleResolverStats are only reported from inside updateStyle so this wouldn't ever show up there since it's inside querySelector(All). Recording a trace for every querySelector would probably be pretty expensive, maybe a disabled by default one. We should consider that for a future patch. :)
esprehn@chromium.org changed reviewers: + meade@chromium.org, rune@opera.com - rune@chromium.org
On 2016/07/12 at 01:39:16, esprehn wrote: > https://codereview.chromium.org/2139843002/diff/20001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/dom/SelectorQuery.cpp (right): > > https://codereview.chromium.org/2139843002/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/dom/SelectorQuery.cpp:460: return selector; > On 2016/07/12 at 01:37:03, sashab wrote: > > Would it be worth adding a StyleResolverStats for when this fast path is triggered? > > It would be worth adding some metrics, but StyleResolverStats are only reported from inside updateStyle so this wouldn't ever show up there since it's inside querySelector(All). Recording a trace for every querySelector would probably be pretty expensive, maybe a disabled by default one. We should consider that for a future patch. :) Yup, sgtm. Also it would enable us to add a test to make sure this path is triggered, again future patch :)
The CQ bit was checked by esprehn@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by ojan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by esprehn@chromium.org
The CQ bit was checked by esprehn@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== Allow [id=X] selectors to use the querySelector ID fast path. This removes 300ms of input latency when clicking the input boxes in the Mint transaction table on a Reks chromebook and 800ms when clicking anything on Zillow on a Reks. On a 2013 MacPro this saves 30-100ms when clicking on those websites. querySelector(All) supports a fast path which transforms calls with selectors like "#foo" and ".foo #bar" into a getElementById call followed by a matches() call against the element and the selector. This patch allows using that getElementById path when writing selectors like "[id=foo]" or ".foo [id=foo]". Selectors of this type are used by the YUI framework and pages like Zillow and Mint often execute a thousand calls to one of these selectors per click event. BUG=627242 ========== to ========== Allow [id=X] selectors to use the querySelector ID fast path. This removes 300ms of input latency when clicking the input boxes in the Mint transaction table on a Reks chromebook and 800ms when clicking anything on Zillow on a Reks. On a 2013 MacPro this saves 30-100ms when clicking on those websites. querySelector(All) supports a fast path which transforms calls with selectors like "#foo" and ".foo #bar" into a getElementById call followed by a matches() call against the element and the selector. This patch allows using that getElementById path when writing selectors like "[id=foo]" or ".foo [id=foo]". Selectors of this type are used by the YUI framework and pages like Zillow and Mint often execute a thousand calls to one of these selectors per click event. BUG=627242 Committed: https://crrev.com/696789dfa7c62bef99966bbe2e0a41109e636a84 Cr-Commit-Position: refs/heads/master@{#404933} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/696789dfa7c62bef99966bbe2e0a41109e636a84 Cr-Commit-Position: refs/heads/master@{#404933} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
