New issue
Advanced search Search tips

Issue 739161 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

31.1%-58.9% regression in loading.desktop at 483666:483828

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

Issue description

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

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

Hi yhirano@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 : Yutaka Hirano
  Commit : 3a0905c1392858d49ee03a2081500351d4568320
  Date   : Fri Jun 30 16:09:00 2017
  Subject: Remove PingLoaderImpl

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : loading.desktop
  Metric       : timeToFirstContentfulPaint_avg/pcv1-warm/money.cnn
  Change       : 48.68% | 259.2105 -> 385.3905

Revision             Result                  N
chromium@483665      259.211 +- 20.8702      6      good
chromium@483693      263.66 +- 4.37838       6      good
chromium@483707      261.869 +- 8.85965      6      good
chromium@483714      281.824 +- 134.263      9      good
chromium@483718      262.607 +- 5.88797      6      good
chromium@483719      265.557 +- 10.627       6      good
chromium@483720      389.009 +- 78.6868      6      bad       <--
chromium@483721      385.39 +- 79.6965       6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=money.cnn loading.desktop

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

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


For feedback, file a bug with component Speed>Bisection
All reports are for money.cnn.
I confirmed that PingLoader::LoadImage is used by the story.
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 13 2017

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

commit 7c45454f3655609e883505dc3269b2a6140302a6
Author: yhirano <yhirano@chromium.org>
Date: Thu Jul 13 04:19:56 2017

Set "ping" requests priority as lowest

Requests from PingLoader had the lowest priority, but I changed it
unintentionally. This CL restores the original behavior.

BUG= 739161 

Review-Url: https://codereview.chromium.org/2982523002
Cr-Commit-Position: refs/heads/master@{#486267}

[modify] https://crrev.com/7c45454f3655609e883505dc3269b2a6140302a6/third_party/WebKit/Source/core/loader/PingLoaderTest.cpp
[modify] https://crrev.com/7c45454f3655609e883505dc3269b2a6140302a6/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp

Status: Assigned (was: Untriaged)
Status: Fixed (was: Assigned)
Fixed by #6

Sign in to add a comment