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

Issue 729769 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

5.8%-19.8% regression in loading.desktop at 476345:476544

Project Member Reported by m...@chromium.org, Jun 5 2017

Issue description

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

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

Hi yzshen@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 : yzshen
  Commit : 2d8fb42490681813d05294e44166615c03a6aaff
  Date   : Thu Jun 01 20:29:40 2017
  Subject: This CL:

Bisect Details
  Configuration: winx64_high_dpi_perf_bisect
  Benchmark    : loading.desktop
  Metric       : cpuTimeToFirstMeaningfulPaint_avg/pcv1-warm/IndiaTimes
  Change       : 12.61% | 304.335666667 -> 342.723333333

Revision             Result                  N
chromium@476395      304.336 +- 42.2888      6       good
chromium@476404      304.898 +- 49.4077      9       good
chromium@476409      322.318 +- 92.9104      14      good
chromium@476412      322.926 +- 67.6355      9       good
chromium@476413      314.157 +- 49.6961      6       good
chromium@476414      350.265 +- 25.5731      9       bad       <--
chromium@476432      342.43 +- 27.3948       6       bad
chromium@476468      344.939 +- 50.1883      6       bad
chromium@476540      342.723 +- 28.9262      6       bad

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

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

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


| 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!
 Issue 729782  has been merged into this issue.
Hmm... This code shouldn't have any effect without --enable-network-service.

Thanks for the report. I will take a look.
Status: Started (was: Untriaged)
Reading the code, I think one possible cause is ThrottlingURLLoader has many fields that are used to cache various arguments when the request is deferred. Usually they are not needed.

I will group them into structs and holds std::unique_ptr<> to them to save the initialization cost. See whether that helps.

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

Bisect Details
  Configuration: mac_air_perf_bisect
  Benchmark    : loading.desktop
  Metric       : timeToFirstContentfulPaint_avg/pcv1-cold/FC2Blog

Revision             Result                  N
chromium@476502      345.73 +- 515.588       21      good
chromium@476601      273.316 +- 650.194      21      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=FC2Blog loading.desktop

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

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


| 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!

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

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : loading.desktop
  Metric       : timeToFirstMeaningfulPaint_avg/pcv1-warm/IndiaTimes

Revision             Result                  N
chromium@476525      874.835 +- 61.6352      21      good
chromium@476615      874.671 +- 77.3791      21      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=IndiaTimes loading.desktop

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

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


| 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!

Comment 14 by m...@chromium.org, Jun 6 2017

Cc: -m...@chromium.org

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

Suspected Commit
  Author : yzshen
  Commit : 2d8fb42490681813d05294e44166615c03a6aaff
  Date   : Thu Jun 01 20:29:40 2017
  Subject: This CL:

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : loading.desktop
  Metric       : cpuTimeToFirstMeaningfulPaint_avg/pcv1-warm/IndiaTimes
  Change       : 25.50% | 233.247833333 -> 292.723333333

Revision             Result                  N
chromium@476344      233.248 +- 18.8978      6      good
chromium@476390      230.334 +- 6.94426      6      good
chromium@476413      231.489 +- 4.05198      6      good
chromium@476414      286.189 +- 22.7022      6      bad       <--
chromium@476415      288.072 +- 28.5334      6      bad
chromium@476416      280.916 +- 9.99679      6      bad
chromium@476419      290.367 +- 24.2565      6      bad
chromium@476424      287.77 +- 29.5322       6      bad
chromium@476435      286.594 +- 22.5082      6      bad
chromium@476525      292.723 +- 36.0698      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=IndiaTimes loading.desktop

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

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


| 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!

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

Bisect Details
  Configuration: mac_air_perf_bisect
  Benchmark    : loading.desktop
  Metric       : timeToFirstContentfulPaint_avg/pcv1-cold/FC2Blog

Revision             Result                  N
chromium@476502      284.443 +- 715.456      21      good
chromium@476601      322.782 +- 518.114      21      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=FC2Blog loading.desktop

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

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


| 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!

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

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : loading.desktop
  Metric       : timeToFirstMeaningfulPaint_avg/pcv1-warm/IndiaTimes

Revision             Result                  N
chromium@476525      877.047 +- 56.8845      21      good
chromium@476615      878.858 +- 69.2526      21      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=IndiaTimes loading.desktop

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

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


| 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!
Project Member

Comment 19 by bugdroid1@chromium.org, Jun 7 2017

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

commit e2e435ef7d38319633f7a01d051f2760af8806c7
Author: yzshen <yzshen@chromium.org>
Date: Wed Jun 07 17:05:13 2017

Make content::ThrottlingURLLoader take a task runner and more efficient.

- The task runner is used to dispatch URLLoaderClient messages.
- Previously it had got many members to cache params when operations were
  deferred. This CL groups those params into structs and only creates instances
  when necessary.

BUG= 729769 ,  715673 

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

[modify] https://crrev.com/e2e435ef7d38319633f7a01d051f2760af8806c7/content/child/resource_dispatcher.cc
[modify] https://crrev.com/e2e435ef7d38319633f7a01d051f2760af8806c7/content/child/url_loader_client_impl.cc
[modify] https://crrev.com/e2e435ef7d38319633f7a01d051f2760af8806c7/content/child/url_loader_client_impl.h
[modify] https://crrev.com/e2e435ef7d38319633f7a01d051f2760af8806c7/content/common/throttling_url_loader.cc
[modify] https://crrev.com/e2e435ef7d38319633f7a01d051f2760af8806c7/content/common/throttling_url_loader.h

About bisect of #20 and #21:

I tried CL https://codereview.chromium.org/2926693002 locally and it did appear to fix the regression. However, on bots the perf numbers didn't restore to previous level.

I noticed that on some bots (chromium-rel-win7-dual) the numbers improved to where they were with my fix, but got worse on the next run. That made me think that maybe there was some other regression crept in right after my fix.

Therefore I did bisects of #20 and #21 to be sure.

=== BISECT JOB RESULTS ===
Perf regression found but unable to narrow commit range

Build failures prevented the bisect from narrowing the range further.


Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : loading.desktop
  Metric       : timeToFirstMeaningfulPaint_avg/pcv1-warm/IndiaTimes
  Change       : 6.14% | 831.203833333 -> 882.227333333

Suspected Commit Range
  2 commits in range
  https://chromium.googlesource.com/chromium/src/+log/7c8bd685a9baeff5169fd4cc63422a69d56b1046..856592d9bae5db8f6550e85e11f71405255e2993


Revision             Result                  N
chromium@477751      831.204 +- 6.64567      6        good
chromium@477759      831.536 +- 21.7228      14       good
chromium@477760      ---                     ---      build failure
chromium@477761      835.623 +- 21.5961      14       bad
chromium@477763      836.633 +- 16.7987      9        bad
chromium@477766      866.895 +- 64.8574      9        bad
chromium@477781      871.328 +- 74.6519      9        bad
chromium@477810      875.864 +- 37.5517      6        bad
chromium@477869      882.227 +- 41.8297      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=IndiaTimes loading.desktop

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

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


| 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!
Cc: roc...@chromium.org
Owner: roc...@chromium.org

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

Hi rockot@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 : Ken Rockot
  Commit : 1caed9e9b41dfa2485d899edde6aacc64c907c9f
  Date   : Wed Jun 07 21:31:19 2017
  Subject: Mojo C++ Bindings: Eliminate CreateInterfacePtrAndBind

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : loading.desktop
  Metric       : cpuTimeToFirstMeaningfulPaint_avg/pcv1-warm/IndiaTimes
  Change       : 12.03% | 216.957888889 -> 243.051666667

Revision             Result                  N
chromium@477751      216.958 +- 22.7489      9      good
chromium@477759      215.653 +- 20.1498      9      good
chromium@477763      215.224 +- 19.192       9      good
chromium@477765      213.197 +- 16.1296      9      good
chromium@477766      250.124 +- 44.4726      9      bad       <--
chromium@477781      245.14 +- 49.4012       9      bad
chromium@477810      262.823 +- 20.6992      6      bad
chromium@477869      243.052 +- 39.8065      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=IndiaTimes loading.desktop

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

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


| 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!

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

Suspected Commit
  Author : Ken Rockot
  Commit : 1caed9e9b41dfa2485d899edde6aacc64c907c9f
  Date   : Wed Jun 07 21:31:19 2017
  Subject: Mojo C++ Bindings: Eliminate CreateInterfacePtrAndBind

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : loading.desktop
  Metric       : timeToFirstMeaningfulPaint_avg/pcv1-warm/IndiaTimes
  Change       : 8.61% | 862.242357143 -> 936.465222222

Revision             Result                  N
chromium@477751      862.242 +- 46.7196      14      good
chromium@477759      857.517 +- 23.1195      6       good
chromium@477763      861.673 +- 22.4565      6       good
chromium@477765      851.275 +- 12.2871      6       good
chromium@477766      910.457 +- 22.839       6       bad       <--
chromium@477781      910.135 +- 74.7677      9       bad
chromium@477810      892.552 +- 67.2728      9       bad
chromium@477869      936.465 +- 271.365      9       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=IndiaTimes loading.desktop

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

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


| 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!

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

Suspected Commit
  Author : Ken Rockot
  Commit : 1caed9e9b41dfa2485d899edde6aacc64c907c9f
  Date   : Wed Jun 07 21:31:19 2017
  Subject: Mojo C++ Bindings: Eliminate CreateInterfacePtrAndBind

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : loading.desktop
  Metric       : cpuTimeToFirstMeaningfulPaint_avg/pcv1-warm/IndiaTimes
  Change       : 15.33% | 216.012714286 -> 248.450111111

Revision             Result                  N
chromium@477751      216.013 +- 25.6755      14      good
chromium@477759      214.251 +- 12.718       6       good
chromium@477763      214.947 +- 15.464       6       good
chromium@477765      214.853 +- 15.1634      6       good
chromium@477766      258.669 +- 15.9077      6       bad       <--
chromium@477781      236.966 +- 66.6454      14      bad
chromium@477810      255.918 +- 66.8269      9       bad
chromium@477869      248.45 +- 43.1648       9       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=IndiaTimes loading.desktop

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

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


| 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!
Owner: ----
Status: Untriaged (was: Started)
My change was a mechanical inlining of some code that was probably already inlined by the compiler. I don't believe there's any way this could cause such a regression.
Owner: yzshen@chromium.org
Yeah. It seems unlikely.

The system automatically re-assigned the bug when it got the result; assigned it back to myself.

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

Suspected Commit
  Author : Ken Rockot
  Commit : 1caed9e9b41dfa2485d899edde6aacc64c907c9f
  Date   : Wed Jun 07 21:31:19 2017
  Subject: Mojo C++ Bindings: Eliminate CreateInterfacePtrAndBind

Bisect Details
  Configuration: mac_10_11_perf_bisect
  Benchmark    : loading.desktop
  Metric       : cpuTimeToFirstMeaningfulPaint_avg/pcv1-warm/IndiaTimes
  Change       : 11.00% | 246.212666667 -> 273.2945

Revision             Result                  N
chromium@477710      246.213 +- 7.42199      6      good
chromium@477748      246.466 +- 6.77184      6      good
chromium@477758      249.601 +- 12.4014      6      good
chromium@477763      247.444 +- 4.87909      6      good
chromium@477765      246.017 +- 12.2668      6      good
chromium@477766      269.776 +- 11.5145      6      bad       <--
chromium@477767      272.486 +- 7.25244      6      bad
chromium@477786      273.294 +- 6.20295      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=IndiaTimes loading.desktop

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

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


| 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!
Project Member

Comment 33 by bugdroid1@chromium.org, Jun 9 2017

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

commit 511543cca3d300f35c0f1556cec42a95f634d1b9
Author: yzshen <yzshen@chromium.org>
Date: Fri Jun 09 22:50:53 2017

Fix ThrottlingURLLoader to set task runner on Binding<URLLoaderClient>.

The recent change of massively removing CreateInterfaceAndBind() incorrectly set
the task runner on the URLLoaderClientPtr. This regressed loading performance.

BUG= 729769 

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

[modify] https://crrev.com/511543cca3d300f35c0f1556cec42a95f634d1b9/content/common/throttling_url_loader.cc

Status: Fixed (was: Untriaged)

Sign in to add a comment