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

12.1% regression in blink_perf.css at 531434:531489

Project Member Reported by leszeks@chromium.org, Jan 26 2018 
See the link to graphs below.
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Jan 26 2018 

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=806216

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=8d71e0111e765a8ce58a5fffceaa1e176680162121190d9d60e97505713434da


Bot(s) for this bug's original alert(s):

linux-release
Project Member

Comment 2 by 42576172...@developer.gserviceaccount.com, Jan 26 2018 

馃搷 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/11ab5282840000
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Jan 27 2018

Cc: roc...@chromium.org jamescook@chromium.org tetsui@chromium.org toyoshim@chromium.org hayato@chromium.org dtseng@chromium.org davidben@chromium.org est...@chromium.org kinuko@chromium.org jcivelli@google.com weiliangc@chromium.org kochi@chromium.org yoshiki@chromium.org futhark@chromium.org isherman@chromium.org shimazu@chromium.org tyoshino@chromium.org reillyg@chromium.org sky@chromium.org ccameron@chromium.org chongz@chromium.org falken@chromium.org jcivelli@chromium.org mattm@chromium.org tsepez@chromium.org yhirano@chromium.org
Owner: tetsui@chromium.org
Status: Assigned (was: Untriaged)
馃搷 Found significant differences after each of 10 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/11ab5282840000

Changing SandboxedUnpacker image sanitization.
By jcivelli@google.com 路 Wed Jan 24 05:17:32 2018
chromium @ ea8f3df127f940773ccb369c1ec9c071aac106ef

Add reconnect logic to URLLoaderFactoryBundle
By chongz@chromium.org 路 Wed Jan 24 05:59:24 2018
chromium @ 7306b0bb25d8e5b87eb5043bcfb82d304e1520d2

net::NormalizeName: use CBB_flush_asn1_set_of instead of sorting encoded values manually
By mattm@chromium.org 路 Wed Jan 24 06:01:27 2018
chromium @ 69c93d3b9d6f7fe0a6618589287ca6b14b29b2f7

Remove dead code after multiple shadow removal [2/5]
By kochi@chromium.org 路 Wed Jan 24 06:01:44 2018
chromium @ 9784fa2f0eb5d8ec5055c8f79c76f30547b8b896

OOR-CORS: Move CORSURLLoader from content/renderer to content/network
By toyoshim@chromium.org 路 Wed Jan 24 06:08:41 2018
chromium @ 42926ddd2790cb10623b7a990f21736b898ac878

Provide accessibility and keyboard access to inactive bubble widgets
By dtseng@chromium.org 路 Wed Jan 24 07:12:27 2018
chromium @ c0b1b644cd36bf7e4d3a9af373c748ee17482feb

Work around black flashing in ui::Compositor
By ccameron@chromium.org 路 Wed Jan 24 07:19:34 2018
chromium @ 4f96f1f0fbf78ea9b6d6ccb0e9f6ec995497a0c3

Add ServiceWorkerServicification flag
By shimazu@chromium.org 路 Wed Jan 24 07:29:24 2018
chromium @ 98f946db176850467a07011a7e068897c6b3f184

Remove dead code after multiple shadow removal [4/5]
By kochi@chromium.org 路 Wed Jan 24 08:20:04 2018
chromium @ 391d909bf8ff508c895164adae6aa5ab7cf4c454

Revert "Collapse older notification popups."
By tetsui@chromium.org 路 Wed Jan 24 09:04:27 2018
chromium @ ffe4a6dcc09997438caf1a6b11c8ca2d1d2ca22c

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Comment 4 by davidben@chromium.org, Jan 27 2018 

Oh huh, I haven't seen this tool before! Something new?

> net::NormalizeName: use CBB_flush_asn1_set_of instead of sorting encoded values manually
> By mattm@chromium.org 路 Wed Jan 24 06:01:27 2018
> chromium @ 69c93d3b9d6f7fe0a6618589287ca6b14b29b2f7

I would not expect blink_perf.css to be exercising that code right now. If I'm reading the graph right, it looks like the change happened right afterwards at

> Remove dead code after multiple shadow removal [2/5]
> By kochi@chromium.org 路 Wed Jan 24 06:01:44 2018
> chromium @ 9784fa2f0eb5d8ec5055c8f79c76f30547b8b896

Comment 5 by isherman@chromium.org, Jan 29 2018

Cc: -isherman@chromium.org

Comment 6 by kochi@chromium.org, Jan 29 2018 

Hmm, basically my change was "dead code removal", but it touches some
CSS code, it might have affected anyway.  FYI, the changes were series of
5 patches (as the title suggests), and 5/3-5/5 were already reverted due
to Mac bot flakiness ( issue 805383 ).

Let me revert the 2/5 tentatively so we can see any performance recovery.

Comment 7 by tetsui@chromium.org, Jan 29 2018

Owner: kochi@chromium.org
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 29 2018 

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

commit f88e32ff524468c37012659dc81a020575c0b5db
Author: Takayoshi Kochi <kochi@chromium.org>
Date: Mon Jan 29 08:02:14 2018

Revert "Remove dead code after multiple shadow removal [2/5]"

This reverts commit 9784fa2f0eb5d8ec5055c8f79c76f30547b8b896.

Reason for revert:  crbug.com/806216 , potential cause for CSS perf
regression (and 3/5-5/5 were already reverted due to  crbug.com/805383 
and have not relanded).

Original change's description:
> Remove dead code after multiple shadow removal [2/5]
>
> Remove
>  - ElementShadow::YoungestShadowRoot()
>  - ElementShadow::OldestShadowRoot()
>
> TBR=mlamouri@chromium.org for media_controls
>
> Bug:  795221 
> Change-Id: I391b737acf04b45978ab1474725314442c979a06
> Reviewed-on: https://chromium-review.googlesource.com/828344
> Commit-Queue: Takayoshi Kochi <kochi@chromium.org>
> Reviewed-by: Hayato Ito <hayato@chromium.org>
> Reviewed-by: Rune Lillesveen <futhark@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#531452}

TBR=kochi@chromium.org,hayato@chromium.org,futhark@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  795221 ,  806216 
Change-Id: I123f9da565b8b43cbb808d613c35171321b76c49
Reviewed-on: https://chromium-review.googlesource.com/890842
Commit-Queue: Takayoshi Kochi <kochi@chromium.org>
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532347}
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/css/SelectorQuery.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/css/StyleEngine.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/dom/ChildFrameDisconnector.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/dom/ChildFrameDisconnector.h
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/dom/Element.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/dom/ElementShadow.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/dom/ElementShadow.h
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/dom/ElementShadowV0.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/dom/ElementShadowV0.h
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/dom/FlatTreeTraversal.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/dom/Node.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/dom/ng/flat_tree_traversal_ng.cc
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/editing/serializers/StyledMarkupSerializer.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/exported/WebFrameSerializer.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/frame/MHTMLTest.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/modules/media_controls/elements/MediaControlOverlayPlayButtonElement.cpp
[modify] https://crrev.com/f88e32ff524468c37012659dc81a020575c0b5db/third_party/WebKit/Source/modules/media_controls/elements/MediaControlSliderElement.cpp

Comment 10 by kochi@chromium.org, Jan 29 2018

Status: Started (was: Assigned)
FYI, I locally tested a release build of chrome and
blink_perf.css/ClassInvalidation, and found somewhat significant difference:
avg 5931.455146692632 runs/s without the change while
avg 5079.539638642621 runs/s WITH the change (reverted now).

So hopefully the graph should recover (at least for the part of my change).

Comment 11 by kochi@chromium.org, Feb 5 2018

Owner: leszeks@chromium.org
Status: Assigned (was: Started)
I reverted my change, but how can I make sure it fixed this issue,
or any other remaining CLs to revert?

Assigning back to the original reporter.

Comment 12 by kochi@chromium.org, Feb 6 2018

Status: Fixed (was: Assigned)
Looks like the perf graph moved back in the range of the revert CL,
so closing this.

According to the information at the graph:

Alert information:
Median of segment before: 7502.254127969896
Median of segment after: 8519.891476138504
Relative change: 13.6%
Test: ChromiumPerf/linux-release/blink_perf.css/ClassInvalidation
Value: 8,496.64 (卤 11.0882)
Point ID: 532397
Time added: 2018-01-29T13:02:03.000Z
Chromium Commit Position range: 532339 - 532397 
Chromium Git Hash range: 9c980a8 - 5b4239f 
V8 Commit Position: b4e468e
►

Sign in to add a comment