All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=625462

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


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

android-galaxy-s5

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

Hi treib@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 : Create a --disable-top-sites flag for use by telemetry tests
Author  : treib
Commit description:
  
BUG=588745

Review-Url: https://codereview.chromium.org/1809603005
Cr-Commit-Position: refs/heads/master@{#402758}
Commit  : f7d11512831495b87ab1fa9cc3bcd92b432d29fc
Date    : Wed Jun 29 09:08:03 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N   Good?
chromium@402749  1665.89  10.321   5   good
chromium@402754  1646.11  12.2553  12  good
chromium@402757  1617.03  19.9543  18  good
chromium@402758  1594.61  20.3451  18  bad    <--
chromium@402759  1588.48  22.5476  12  bad
chromium@402769  1615.98  12.6144  5   bad

Bisect job ran on: android_s5_perf_bisect
Bug ID: 625462

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.css
Test Metric: ClassInvalidation/ClassInvalidation
Relative Change: 3.00%
Score: 99.8

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/759
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9008185824158540640


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

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

Not mine.

The bisect looks like it identified the wrong CL, assigning back to the original perf sheriff (on advice from another perf sheriff).

Trying one more bisect job...

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


===== TESTED REVISIONS =====
Revision                       Mean     Std Dev  N  Good?
chromium@402740                1663.17  10.4489  5  good
chromium@402760                1601.17  6.21176  5  good
chromium@402771                1557.96  6.58447  5  good
chromium@402772                1557.7   5.24351  5  good
chromium@402772,v8@85cebe7389  1550.7   10.8078  5  good
chromium@402772,v8@b1063f7a41  1551.42  7.54376  5  good
chromium@402772,v8@15498e16c8  1550.02  2.36026  5  good
chromium@402772,v8@a7159577b7  1547.29  5.68035  5  good
chromium@402772,v8@85c9f0018a  1548.21  7.56629  5  good
chromium@402773                1468.85  4.28799  5  bad
chromium@402774                1467.76  3.0257   5  bad
chromium@402776                1473.3   3.0462   5  bad
chromium@402780                1476.25  1.58551  5  bad

Bisect job ran on: android_s5_perf_bisect
Bug ID: 625462

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.css
Test Metric: ClassInvalidation/ClassInvalidation
Relative Change: 11.24%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/775
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9007832711977769328


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

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

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/9007036727951789280

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

Hi kochi@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 : Implement closed shadow adjustment for Element.offsetParent
Author  : kochi
Commit description:
  
If |offsetParent| is not an unclosed node of the context
object, search the layout tree up until an unclosed node
is found.

|offsetTop| etc. will be adjusted relative to this
|offsetParent| value.

See also
https://github.com/w3c/webcomponents/issues/497
https://github.com/w3c/csswg-drafts/issues/159
for the discussions about spec clarifications.

BUG= 531990 

Review-Url: https://codereview.chromium.org/2051703002
Cr-Commit-Position: refs/heads/master@{#402757}
Commit  : 18d455ee833f6a30dcbe2755380861eb75cd9f6f
Date    : Wed Jun 29 08:55:36 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@402740  1647.03  6.54531  5  good
chromium@402755  1678.48  7.83236  5  good
chromium@402756  1669.4   18.5774  5  good
chromium@402757  1623.88  12.0541  5  bad    <--
chromium@402759  1595.84  19.5853  5  bad
chromium@402762  1596.13  10.325   5  bad
chromium@402769  1624.24  15.4742  5  bad

Bisect job ran on: android_s5_perf_bisect
Bug ID: 625462

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.css
Test Metric: ClassInvalidation/ClassInvalidation
Relative Change: 1.38%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/805
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9007036727951789280


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

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

Pinging kochi@

How likely do you think it is that your patch above (Implement closed shadow adjustment...) is related to the regression on blink_perf.css?

Looks like mine is the culprit.
I'll take a look at the benchmark and the perf asap.

I took benchmarks on my dev linux machine, but found no significant regression
with the CLs (specifically, tried with reverting r402757 and r404081).

My change affects |HTMLElement.offsetTop| (internally calls |offsetParent|)
which appears twice in the tight loop in the ClassInvalidation benchmark,
but should not affect the essential complexity unless the context object
element is in a shadow.

The |offsetTop| is used to trigger the style recalculation which takes the
dominant time of |offsetTop| call, and I experimented to change |offsetTop|
just trigger style recalc and always return 0 (e.g. bypasses |offsetTop|/
|offsetParent| logic).

The result was that it did not improve much, about ~5% on my machine.
So if |offsetTop| regressed 10%, it would mean non-style recalc logic
slowed down 3x.

Although I haven't done the experiment with Android phones, I don't think
my CL affected that much.

I checked the revision range 402750-402769, but nothing was so suspicious,
although r402770 looks suspicious (in terms of changing the style invalidation
/ recalc performance).
https://codereview.chromium.org/2089063005

The original bisect (in c#8) and c#10 were different, and taking a look
at the c#10's result:

===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@402740  1647.03  6.54531  5  good
chromium@402755  1678.48  7.83236  5  good
chromium@402756  1669.4   18.5774  5  good
chromium@402757  1623.88  12.0541  5  bad    <--
chromium@402759  1595.84  19.5853  5  bad
chromium@402762  1596.13  10.325   5  bad
chromium@402769  1624.24  15.4742  5  bad

std dev is 6~19, which looks quite stable (my local runs were
like ~4000runs/s, stddev=400 which fluctuated much worse).
The regression at 402756:402757 is within 3%.

Assigning back to qyearsley@, could you help how to interpret
this result?

+robertocn fyi

Just taking a quick look, it seems possible that there could have been a regression caused by r402757, but maybe not. The differences between some of the other means for other revisions were also >20 runs/s... anyway, I'm not really sure what to think.

For more information about the results the bisect bot got, you can see:
https://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/805/steps/Debug%20Info/logs/Debug%20Info

kochi, would you be OK with speculatively reverting r402757 to see if the results on the android-galaxy-s5 perf bot change back to previous levels?

qyearsley@ r402757 is a spec-compliance change and should not be reverted
forever, but if it is temporal I think it is okay.
Note that there is a follow-up fix (r404081), so you need to revert these
2 CLs.

Could you do that?

Hm, reverting seems a little troublesome given that we know we want to reland it -- but it would still be nice to know the performance effects.

I made a revert CL that reverts both of those together (https://codereview.chromium.org/2194793006), and then ran:

  tools/perf/run_benchmark trybot android-s5 blink_perf.css

which produced: https://codereview.chromium.org/2197653002

From this we should be able to see the performance effects on that particular try bot.

Then I tried running:

  tools/perf/run_benchmark trybot all blink_perf.css

Which produced: https://codereview.chromium.org/2196763002

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/9005747967433900592

Well, https://codereview.chromium.org/2197653002 didn't really reproduce the regression. Given that this is just one bot and there is some noise in the graph (https://chromeperf.appspot.com/group_report?bug_id=625462), there's still the possibility that there's no real regression. Still, I started one more bisect job.

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


===== SUSPECTED CL(s) =====
Subject : Implement closed shadow adjustment for Element.offsetParent
Author  : kochi
Commit description:
  
If |offsetParent| is not an unclosed node of the context
object, search the layout tree up until an unclosed node
is found.

|offsetTop| etc. will be adjusted relative to this
|offsetParent| value.

See also
https://github.com/w3c/webcomponents/issues/497
https://github.com/w3c/csswg-drafts/issues/159
for the discussions about spec clarifications.

BUG= 531990 

Review-Url: https://codereview.chromium.org/2051703002
Cr-Commit-Position: refs/heads/master@{#402757}
Commit  : 18d455ee833f6a30dcbe2755380861eb75cd9f6f
Date    : Wed Jun 29 08:55:36 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@402740  1660.22  15.9555  5  good
chromium@402756  1685.23  6.76193  5  good
chromium@402757  1624.73  8.16638  5  bad    <--
chromium@402758  1599.31  11.3266  5  bad
chromium@402760  1605.04  10.885   5  bad
chromium@402764  1577.73  5.80447  5  bad
chromium@402772  1564.93  2.19557  5  bad

Bisect job ran on: android_s5_perf_bisect
Bug ID: 625462

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.css
Test Metric: ClassInvalidation/ClassInvalidation
Relative Change: 5.74%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/857
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9005747967433900592


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

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

Thanks Quinten, for taking more closer look!

The interpreting for the result got more complicated, as the problem
didn't reproduce in the revert patch but the bisect caught my CL again.

Anyway, the change was to be compliant with the spec, we do not want to
revert the change. The choice is either:
1. Leave the change as is, set the new expected performance after r402757.
2. Investigate more about which part changed the perf, more precisely.
3. other?

"element.offsetTop" was used in the benchmark, to kick the style
recalculation to happen after making some style dirty.  I.e. the
benchmark just expects the side-effect, and ignoring the real
return value of "offsetTop".

Style recalculation performance itself should not be affected by my change,
but only the part to return a meaningful value for ".offsetTop"
should be affected.  So that part should be excluded from the benchmark,
if mine is the real cause for the perf regression, could we rebaseline the
perf expectation?

+rune@opera.com, owner of blink_perf.css benchmark, for advice on the best path forward (see questions in #21)

I took a manual stab at the graph and logged the range where the graph was steep (6aa0504..c125a9b).

This looks like a likely suspect for that regression:

https://chromium.googlesource.com/chromium/src/+/1f82047b13f02be39b8104b6afda0615e60a7cee

The commit I mentioned only adds code to StyleInvalidator which is not exercised for this test (and StyleInvalidator traversing the DOM applying  invalidation sets is what the test exercises). Given that this regression happen on some Android devices and not others, this might be due to CPU instruction cache differences and that added code may change the performance. Also, since the bisect identifies something unrelated, I'll wontfix this.