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

Issue 713085 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression

Blocked on:
issue v8:6402



Sign in to add a comment

10.2% regression in blink_perf.css at 464868:464873

Project Member Reported by alexclarke@chromium.org, Apr 19 2017

Issue description

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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgooHhvgkM


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

android-nexus6
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Apr 19 2017

Cc: leszeks@chromium.org
Owner: leszeks@chromium.org

=== Auto-CCing suspected CL author leszeks@chromium.org ===

Hi leszeks@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Leszek Swirski
  Commit : 0010be5b23f5d72744dcf5b11113568e1916d2e9
  Date   : Thu Apr 13 13:41:26 2017
  Subject: [compiler] Always use deopt count for disabling optimization

Bisect Details
  Configuration: android_nexus6_perf_bisect
  Benchmark    : blink_perf.css
  Metric       : SelectorCountScaling/SelectorCountScaling
  Change       : 11.76% | 21.2471897173 -> 18.7495559415

Revision                           Result                   N
chromium@464867                    21.2472 +- 0.333452      6      good
chromium@464870                    21.0376 +- 0.297957      6      good
chromium@464871                    21.0309 +- 0.418338      6      good
chromium@464871,v8@d9f6dd896c      21.6359 +- 0.366762      6      good
chromium@464871,v8@dc83caa6fb      21.6209 +- 0.406186      6      good
chromium@464871,v8@c4b02905d8      21.882 +- 0.48591        6      good
chromium@464871,v8@0010be5b23      19.1127 +- 0.387841      6      bad       <--
chromium@464871,v8@38c3b71c26      18.7801 +- 0.677582      6      bad
chromium@464871,v8@204989a5aa      18.72 +- 0.713421        6      bad
chromium@464871,v8@e8cfc5b7ca      18.6583 +- 0.429928      6      bad
chromium@464872                    18.6539 +- 0.572252      6      bad
chromium@464873                    18.7496 +- 0.675128      6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests blink_perf.css

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8981888508982947744

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=6462193370923008


| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Speed>Bisection.  Thank you!
There's enough improvements to the same benchmark on other bots that I think this is WontFix material. WDYT?
Well the other bots appear to be all desktop ones where this change was a perf win.  I presume we care about android perf too.
Good point -- looking at graphs of other mobile devices, it seems like they may be encountering a similar issue. I'll check if we're doing anything obviously wrong here, but I worry that it may just be a "you win some, you lose some" heuristic change.
Cc: rmcilroy@chromium.org
+rmcilroy
This benchmark is pretty micro-benchmarky and is focused on CSS performance, so I agree with Leszek that this is probably just a heuristic change which causes this particular code sequence to end up optimizing the JS at a different point that it would otherwise (which happens to help desktop but hurt mobile for this particularly microbenchmark). It's worth checking we aren't doing anything seriously wrong, but if not then I think we can wontfix this (particularly since we don't see any regressions in more real-world examples on mobile).
Status: Assigned (was: Untriaged)
Explictly assigning. A CL you landed tripped one of the speed metrics we measure in the lab. If this is the first time this has happened to one of your CLs, or if it's been a while, please read: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/addressing_performance_regressions.md

We're looking for one of the following:
1. Justification via explanation
2. Plan to revert or fix
3. Angry rage throwing of equipment at my head

Just be aware that I'm trained in trumpet playing and First Aid and am not afraid to use it.

Note: This was a bulk edit message and not very personal.
This will go away with https://chromium-review.googlesource.com/c/588894/
Blockedon: v8:6402
Status: Fixed (was: Assigned)
Looks like the fix worked.

Sign in to add a comment