New issue
Advanced search Search tips

Issue 739301 link

Starred by 2 users

Issue metadata

Status: Duplicate
Merged: issue 737623
Owner: ----
Closed: Jul 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

2.6%-11.9% regression in blink_perf.paint at 482501:482620

Project Member Reported by rmcilroy@chromium.org, Jul 5 2017

Issue description

See the link to graphs below.
 
Cc: lizeb@chromium.org
Owner: lizeb@chromium.org

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

Hi lizeb@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 : lizeb
  Commit : 8e8aa98f3b4b67412dc9d2a842d457f40b12caad
  Date   : Tue Jun 27 09:30:25 2017
  Subject: android: Disable the startup DNS resolutions.

Bisect Details
  Configuration: android_nexus5_perf_bisect
  Benchmark    : blink_perf.paint
  Metric       : LocalFrameView::prePaint/LocalFrameView::prePaint
  Change       : 3.09% | 346.767960317 -> 357.474662698

Revision             Result                  N
chromium@482543      346.768 +- 2.36965      6      good
chromium@482568      347.647 +- 1.06505      6      good
chromium@482580      349.941 +- 3.85358      6      good
chromium@482582      349.85 +- 1.82488       6      good
chromium@482583      354.314 +- 1.29231      6      bad       <--
chromium@482586      354.736 +- 1.42552      6      bad
chromium@482592      357.475 +- 1.87042      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.paint

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8974918088794616096


For feedback, file a bug with component Speed>Bisection

Comment 4 by lizeb@chromium.org, Jul 6 2017

This is indeed a possibility, provided that:
- The prefs are not cleared between runs,
- The same page is fetched several times in a row, and only this one, and
- DNS resolution takes a significant amount of time for the test page

If that's the case, then the fact that we no longer magically know all the origins to resolve ahead of time could affect timings. It seems a bit weird though, since most of our tests are only loading resources from 127.0.0.1, which wouldn't go through DNS, and so this change should either be a no-op (because the pre-resolution list would be empty), or actually save work (since we are not resolving useless domains). 
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.

Comment 6 by lizeb@chromium.org, Jul 28 2017

To expand a bit on what's above, I'm quite skeptical that this is actually linked to the CL above.

The CL above makes Chrome do less work at startup on Android only, by not issuing DNS requests to the list of the previously resolved host during the previous startup.
As most of our tests only get resources from 127.0.0.1, there is no DNS resolution needed, hence even if the list contained useful things (which is not the case in the majority of cases for real devices), I don't really see how not doing the work could regress paint performance.

Also, I didn't notice this at first, but the regression also appears to be on mac, and this is an Android-only commit.

Sorry, I don't really have a clue about the actual culprit here...
Owner: ----
Status: Available (was: Assigned)
This bug is a little confusing. There was a regression on Mac that returned to normal levels, and I removed it. The rest of the regressions are on Android, but looking at the graphs there is a pretty wide range, so taking into account the info about what the CL does from #6, I think we should re-bisect. I kicked off a bisect on a wider range for blink_perf.paint, and a separate bisect for frame_times. Let's wait to assign based on the results of those.
Project Member

Comment 10 by 42576172...@developer.gserviceaccount.com, Jul 28 2017


=== BISECT JOB RESULTS ===
NO Perf regression found

Bisect Details
  Configuration: android_nexus6_perf_bisect
  Benchmark    : smoothness.top_25_smooth
  Metric       : frame_times/LinkedIn

Revision             Result                   N
chromium@482547      17.2898 +- 0.597647      21      good
chromium@482593      17.2886 +- 0.837728      21      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 --story-filter=LinkedIn smoothness.top_25_smooth

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8972823434674143728


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 11 by 42576172...@developer.gserviceaccount.com, Jul 29 2017

Mergedinto: 737623
Status: Duplicate (was: Available)

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

Suspected Commit
  Author : Xianzhu Wang
  Commit : 2381016a46e881351c34f7b7c2e6692565222132
  Date   : Tue Jun 27 18:43:07 2017
  Subject: Add WritingModeUtils.h to convert logical/physical directions

Bisect Details
  Configuration: android_nexus5_perf_bisect
  Benchmark    : blink_perf.paint
  Metric       : LocalFrameView::prePaint/LocalFrameView::prePaint
  Change       : 14.74% | 324.671945767 -> 372.528920635

Revision             Result                  N
chromium@482162      324.672 +- 1.53819      6      good
chromium@482512      346.105 +- 2.74312      6      good
chromium@482600      349.332 +- 1.78695      6      good
chromium@482644      353.739 +- 3.53029      6      good
chromium@482666      345.169 +- 1.71396      6      good
chromium@482677      347.484 +- 2.9803       6      good
chromium@482682      347.265 +- 1.6311       6      good
chromium@482683      382.44 +- 1.09896       6      bad       <--
chromium@482684      382.305 +- 1.53151      6      bad
chromium@482685      381.147 +- 2.1429       6      bad
chromium@482687      375.403 +- 2.46392      6      bad
chromium@482861      372.529 +- 2.34824      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.paint

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8972823411121374288


For feedback, file a bug with component Speed>Bisection

Sign in to add a comment