New issue
Advanced search Search tips

Issue 609125 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 608475
Owner:
Closed: May 2016
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

11.3% regression in blink_perf.shadow_dom at 390890:390895

Project Member Reported by mustaq@chromium.org, May 4 2016

Issue description

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

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


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

chromium-rel-win7-dual
Cc: yosin@chromium.org
Owner: yosin@chromium.org

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

Hi yosin@chromium.org, the bisect results pointed to your CL below as possibly
causing a regression. Please have a look at this info and see whether
your CL be related.


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Introduce NodeTraversal::ancestorsOf() and inclusiveAncestors() for range-based for loop
Author  : yosin
Commit description:
  
This patch introduces |NodeTraversal::ancestorsOf()| and |inclusiveAncestors()|
to reduce source code size by common code pattern for ease of writing code
in addition to |NodeTraversal::childrenOf()|[1], |NodeTraversal::descendansOf()|
and so on.

Before this patch, we should write:
// Loop for ancestors
for (Node* runner = node->parentNode(); runner; runner = runner->parentNode()) {
 ...
}

// Loop for ancestors or self
for (Node* runner = node; runner; runner = runner->parentNode()) {
 ...
}

After this patch, you can write:
for (Node& runner : NodeTraversal::ancestorsOf(*node)) {
 ...
}

for (Node& runner : NodeTraversal::inclusiveAncestorsOf(*node)) {
 ...
}

[1] https://codereview.chromium.org/642973003 Introduce typed Node/Element
iterators for range-based for loops of C++11

BUG=n/a
TEST=n/a; no behavior changes

Review-Url: https://codereview.chromium.org/1932523003
Cr-Commit-Position: refs/heads/master@{#390895}
Commit  : 95d6f1792e11fefa919d81d1ed7c0d233deafa76
Date    : Mon May 02 05:28:03 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev    N  Good?
chromium@390889  13.3398  0.140738   5  good
chromium@390892  13.3938  0.181035   5  good
chromium@390894  13.7896  0.0696118  5  good
chromium@390895  14.1926  0.19392    5  bad    <--

Bisect job ran on: win_perf_bisect
Bug ID: 609125

Test Command: python src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.shadow_dom
Test Metric: SmallDistributionWithLayout/SmallDistributionWithLayout
Relative Change: 6.39%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/win_perf_bisect/builds/6501
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9013584210928507072


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5862659526754304

| 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 Tests>AutoBisect.  Thank you!
yosin: Ping!

Comment 4 by yosin@chromium.org, May 11 2016

Cc: -yosin@chromium.org
Owner: tdres...@chromium.org
I don't think range-based for makes perf regression. Generated code should be same for for-statement and range-based for-statement.

This perf regression occurred only on one bot, it seems it is noise.

Owner: mustaq@chromium.org
Reassigning to mustaq@.

Kicked off another bisect here:
https://chromeperf.appspot.com/buildbucket_job_status/9012958417016325120

It doesn't look like noise to me, though there could be something bot specific going on.

Hopefully another bisect will shed some light on things.
Project Member

Comment 6 by 42576172...@developer.gserviceaccount.com, May 11 2016

Mergedinto: 608475
Status: Duplicate (was: Assigned)

===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : binding: Makes Window/Location's attributes accessor-type properties.
Author  : yukishiino
Commit description:
  
Makes almost all the attributes (except for cross-origin accessible
attributes) accessor-type properties.  The target attributes in this
CL are DOM attributes of Window, Location and workers not annotated
as [DoNotCheckSecurity], which are cross-origin accessible.

BUG= 43394 ,  516274 

Review-Url: https://codereview.chromium.org/1380503002
Cr-Commit-Position: refs/heads/master@{#390893}
Commit  : 9ac1750adf85027985c7af3468e0b972c2086235
Date    : Mon May 02 05:05:22 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev    N  Good?
chromium@390889  13.2744  0.0802764  5  good
chromium@390892  13.2208  0.0435798  5  good
chromium@390893  14.2622  0.49913    5  bad    <--
chromium@390894  13.7664  0.0893017  5  bad
chromium@390895  14.0268  0.168313   5  bad

Bisect job ran on: win_perf_bisect
Bug ID: 609125

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.shadow_dom
Test Metric: SmallDistributionWithLayout/SmallDistributionWithLayout
Relative Change: 5.67%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/win_perf_bisect/builds/6510
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9012958417016325120


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5843697581686784

| 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 Tests>AutoBisect.  Thank you!

Sign in to add a comment