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

Issue 829408 link

Starred by 3 users

10.7%-14.1% regression in blink_perf.css at 547665:547687

Project Member Reported by fmea...@chromium.org, Apr 5 2018

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=829408

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


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

android-nexus5X
android-nexus6
android-nexus7v2
android-webview-nexus6
😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/14d4a748c40000
Cc: hirosh...@chromium.org hayato@chromium.org raphael....@intel.com kochi@chromium.org yukishiino@chromium.org kinuko@chromium.org skia-chr...@skia-buildbots.google.com.iam.gserviceaccount.com nhiroki@chromium.org xidac...@chromium.org haraken@chromium.org smcgruer@chromium.org futhark@chromium.org leszeks@chromium.org kouhei@chromium.org
Owner: smcgruer@chromium.org
Status: Assigned (was: Untriaged)
📍 Found significant differences after each of 5 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/14f83764c40000

Cache code for inline scripts by leszeks@chromium.org
https://chromium.googlesource.com/chromium/src/+/fc4e797b1522d0dd5163de7c291524e0519d160c

Move ExecutionContext and SecurityContext to core/execution_context by kochi@chromium.org
https://chromium.googlesource.com/chromium/src/+/ac85ca2a9cb8c76a37f9d7a6c611c24114f1f05d

Roll src/third_party/skia/ c0528e241..fd88fe4d5 (1 commit) by skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com
https://chromium.googlesource.com/chromium/src/+/7dbfeb630c33a014da36c2e4c8b3c397f102b078

Remove custom bindings for CSSStyleDeclaration. by raphael.kubo.da.costa@intel.com
https://chromium.googlesource.com/chromium/src/+/5b84e88a070e40a1a74576a6cf41109d29a8035a

Web Animations: give ComputedEffectTiming::endTime a lower bound of 0 by smcgruer@chromium.org
https://chromium.googlesource.com/chromium/src/+/55aa8afeb7c99f5aabbe1e44fffcf70f9babd094

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Owner: raphael....@intel.com
I'm pretty sure this is caused by my CL; there were similar reports when a we tried to get rid of custom bindings for CSSStyleDeclaration back in 2015.
Components: Blink>CSS Blink>Bindings
Status: Started (was: Assigned)
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 11 2018

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

commit a21878dbbdb4f3ebe979d63b690f5a499bbd02fd
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Wed Apr 11 22:34:10 2018

CSSStyleDeclaration: Remove [RaisesException] from setter

Previously, the custom bindings code for CSSStyleDeclaration only
create an ExceptionState locally to pass to SetPropertyInternal().

Go back to this behavior and drop [RaisesException] from the setter due
to performance issues: [RaisesException] causes an ExceptionState to be
generated automatically in namedPropertySetter(), and an expensive
CString needs to be created to set the ExceptionState's |property_name|
argument.

By creating an ExceptionState ourselves in AnonymousNamedSetter(), we
can avoid creating a CString altogether and use a custom value for
|property_name|. This brings some performance tests back to the values
they used to report when we had custom bindings for CSSStyleDeclaration,
namely:
- blink_perf.css' CSSPropertySetterGetter and CSSPropertyUpdateValue
- blink_perf.layout's SimpleTextPathLineLayout

Bug: 764633,  829408 
Change-Id: I99db6021d4483c5743daa452f9cb532eacda79a7
Reviewed-on: https://chromium-review.googlesource.com/1002881
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549955}
[modify] https://crrev.com/a21878dbbdb4f3ebe979d63b690f5a499bbd02fd/third_party/blink/renderer/core/css/css_style_declaration.cc
[modify] https://crrev.com/a21878dbbdb4f3ebe979d63b690f5a499bbd02fd/third_party/blink/renderer/core/css/css_style_declaration.h
[modify] https://crrev.com/a21878dbbdb4f3ebe979d63b690f5a499bbd02fd/third_party/blink/renderer/core/css/css_style_declaration.idl

Status: Fixed (was: Started)
Cc: kojii@chromium.org jbudorick@chromium.org sullivan@chromium.org eyaich@google.com
 Issue 832080  has been merged into this issue.
Cc: primiano@chromium.org
 Issue 829835  has been merged into this issue.

Sign in to add a comment