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

Issue 704778 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Regression



Sign in to add a comment

17.2% regression in blink_perf.paint at 457730:457783

Project Member Reported by jasontiller@chromium.org, Mar 24 2017

Issue description

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

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


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

android-nexus5
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Mar 24 2017

Cc: nhiroki@chromium.org
Owner: nhiroki@chromium.org

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

Hi nhiroki@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 : nhiroki
  Commit : 1d85fa7d0be347292a5805ec87ca3d2d10a8c57f
  Date   : Fri Mar 17 12:24:56 2017
  Subject: Bindings: Remove DOMWrapperWorld::fromWorldId() for cleanup

Bisect Details
  Configuration: android_nexus5_perf_bisect
  Benchmark    : blink_perf.paint
  Metric       : complex-content-slow-scroll/complex-content-slow-scroll
  Change       : 16.02% | 38.1773333333 -> 44.2944166667

Revision             Result                   N
chromium@457729      38.1773 +- 1.49483       6      good
chromium@457736      37.4615 +- 1.7369        6      good
chromium@457738      37.6928 +- 1.76342       6      good
chromium@457739      43.353 +- 1.95136        6      bad       <--
chromium@457740      43.347 +- 1.62071        6      bad
chromium@457743      43.0798 +- 0.775813      6      bad
chromium@457756      44.2807 +- 3.27388       6      bad
chromium@457783      44.2944 +- 2.77377       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

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

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


| 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: wangxianzhu@chromium.org
Components: Blink>Bindings Blink>Paint
Labels: -Pri-2 Pri-1
Status: Assigned (was: Untriaged)
I'll take a look...

CC to wangxianzhu@ who is the author of the test:
https://chromium.googlesource.com/chromium/src/+/fd7fc207884807c1b5365c191cf6d5cb55e33f4c
Note that my culprit CL is just for refactoring, so I'll revert it if it's difficult to fix.
Cc: haraken@chromium.org yukishiino@chromium.org
Hm..., probably changing the hash traits for WTF::HashMap and/or adding the main world into the world map changed the performance? I'll try to revert that part.
I'm not sure but let's try to revert it.

Status: Started (was: Assigned)
CL: https://codereview.chromium.org/2777763003/
Project Member

Comment 10 by 42576172...@developer.gserviceaccount.com, Mar 27 2017


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

Suspected Commit
  Author : nhiroki
  Commit : 1d85fa7d0be347292a5805ec87ca3d2d10a8c57f
  Date   : Fri Mar 17 12:24:56 2017
  Subject: Bindings: Remove DOMWrapperWorld::fromWorldId() for cleanup

Bisect Details
  Configuration: android_nexus5_perf_bisect
  Benchmark    : blink_perf.paint
  Metric       : complex-content-slow-scroll/complex-content-slow-scroll
  Change       : 14.83% | 38.273 -> 43.9481666667

Revision             Result                   N
chromium@457710      38.273 +- 1.58094        6      good
chromium@457729      38.0792 +- 0.946489      6      good
chromium@457738      38.1372 +- 0.832841      6      good
chromium@457739      43.6574 +- 3.03112       6      bad       <--
chromium@457740      43.8799 +- 2.74143       6      bad
chromium@457741      42.6599 +- 2.33508       6      bad
chromium@457743      42.78 +- 1.29046         6      bad
chromium@457747      42.9478 +- 0.784935      6      bad
chromium@457783      43.9482 +- 1.84136       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

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

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


| 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 11 by bugdroid1@chromium.org, Mar 29 2017

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

commit 3e5d3fc45d06833189606b0ad750b9dee95ef50a
Author: nhiroki <nhiroki@chromium.org>
Date: Wed Mar 29 05:53:18 2017

Bindings: Manage the main world separately from other worlds

This CL speculatively reverts a part of the CL[1].

The CL[1] pushed the main world into the world map for cleanup, but that CL
seems to cause a performance regression on a perf test (see  issue 704778 )
probably because of changing the hashmap traits and/or the world map management.

[1] https://codereview.chromium.org/2753013003/

BUG= 697622 ,  697629 ,  704778 

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

[modify] https://crrev.com/3e5d3fc45d06833189606b0ad750b9dee95ef50a/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp
[modify] https://crrev.com/3e5d3fc45d06833189606b0ad750b9dee95ef50a/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorldTest.cpp

A possible fix was landed. I'll monitor the graph for a while...
Status: Fixed (was: Started)
After the patch, results of the perf test returned to the original level.

Sign in to add a comment