New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 748790 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug



Sign in to add a comment

::slotted() selector use causes polynomial increase in style recalc

Project Member Reported by bickne...@google.com, Jul 26 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36

Steps to reproduce the problem:
Add many elements with shadow roots containing `::slotted(*)` selectors.

What is the expected behavior?

What went wrong?
Style recalc time increases polynomially with the number of elements added.

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 62.0.3166.0  Channel: canary
OS Version: OS X 10.12.5
Flash Version:
 
slotted-selector.html
2.1 KB View Download
Screen Shot 2017-07-25 at 17.20.11.png
257 KB View Download
Labels: Needs-Triage-M62
Cc: pnangunoori@chromium.org
Labels: -Needs-Triage-M62 M-61 OS-Linux OS-Windows
Status: Untriaged (was: Unconfirmed)
Tested on Canary #62.0.3167.0 on Mac 10.12.5 and able to reproduce the issue. Issue is also observed on Windows 7 Canary #62.0.3166.0 and Ubuntu 14.04 latest Dev #61.0.3159.5

This is a non-regression issue and able to reproduce from M-61 #61.0.3149.0. Marking it as untriaged so that issue gets addressed.

Note: For the builds earlier to 61.0.3149.0, provided html file displays a blank screen.

Thanks.

Project Member

Comment 3 by sheriffbot@chromium.org, Jul 26 2017

Labels: Hotlist-Google
> Note: For the builds earlier to 61.0.3149.0, provided html file displays a blank screen.

Here's a modified version of the repro page without module scripts; it exhibits the same issue in stable (59.0.3071.115).
slotted-selector.html
2.2 KB View Download
Labels: Performance
Cc: hayato@chromium.org kochi@chromium.org
Components: -Blink>CSS
Labels: Update-Monthly
Status: Available (was: Untriaged)

Comment 7 by hayato@chromium.org, Jul 28 2017

kochi@, do you have any insights?

Comment 8 by kochi@chromium.org, Jul 28 2017

I'll investigate.
Labels: -Performance Performance-Responsiveness

Comment 10 by kochi@chromium.org, Jul 31 2017

Status: Started (was: Available)

Comment 11 by kochi@chromium.org, Jul 31 2017

This was because the style sheet for each shadow dom are categorized
as "containing tree-boundary rules" and matched against all other tree
scopes - which resulted in O(n^2) behavior.

The fix CL is in progress.

Comment 12 by kochi@chromium.org, Jul 31 2017

Labels: -Pri-2 Pri-1
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab70dc71b6156e8e3b79310463f668d96b842796

commit ab70dc71b6156e8e3b79310463f668d96b842796
Author: Takayoshi Kochi <kochi@chromium.org>
Date: Mon Aug 07 08:02:59 2017

Decouple ::slotted rules from TreeBoundaryCrossing rules

::slotted() pseudo element rules were treated as tree-boundary
crossing rules (same as legacy /deep/ and ::shadow).
This caused O(N^2) issue where N is the node of shadow trees with
::slotted() rules, but without real tree-boundary crossing rules,
as adding the scope to StyleEngine's tree boundary crossing scopes
is slow.

The perf issue of rule matching was already fixed by Rune's old CL:
https://codereview.chromium.org/1853713002/

This CL is a performance fix and should not expose any behavioral
changes except for performance.

With the streamlined test case from the original report (N=100,1000)
http://jsbin.com/nodiwuxuge/edit?html,console,output
I got the following results on my local workstation:

Without this fix: 15.7ms (N=100), 1495ms (N=1000)
With this fix: 14.9ms (N=100), 93.4ms (N=1000)

Bug:  748790 
Change-Id: I4b3c4b29fcdd4bf152ed3e6409044df748eedc93
Reviewed-on: https://chromium-review.googlesource.com/595295
Reviewed-by: Rune Lillesveen <rune@opera.com>
Commit-Queue: Takayoshi Kochi <kochi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492282}
[add] https://crrev.com/ab70dc71b6156e8e3b79310463f668d96b842796/third_party/WebKit/LayoutTests/shadow-dom/v0/slotted-pseudo-element-in-v0v1-mixed.html
[modify] https://crrev.com/ab70dc71b6156e8e3b79310463f668d96b842796/third_party/WebKit/Source/core/css/ActiveStyleSheetsTest.cpp
[modify] https://crrev.com/ab70dc71b6156e8e3b79310463f668d96b842796/third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.cpp
[modify] https://crrev.com/ab70dc71b6156e8e3b79310463f668d96b842796/third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.h
[modify] https://crrev.com/ab70dc71b6156e8e3b79310463f668d96b842796/third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp

Status: Fixed (was: Started)
Labels: allpublic

Sign in to add a comment