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

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


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

android-nexus7v2

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

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

Hi mlippautz@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 : Reland "[wrapper-tracing] Enable flag per default"
Author  : mlippautz
Commit description:
  
BUG= chromium:468240 

Review-Url: https://codereview.chromium.org/2503043002
Cr-Commit-Position: refs/heads/master@{#433823}
Commit  : e23de6962be8b4967127ae95040f32bbb533b4d1
Date    : Tue Nov 22 09:28:34 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N   Good?
chromium@433798  46929.9  1778.95  15  good
chromium@433813  46866.7  1345.04  15  good
chromium@433820  47021.4  1358.62  15  good
chromium@433822  47004.1  1343.69  15  good
chromium@433823  48897.4  2820.2   15  bad    <--
chromium@433824  48635.9  1969.47  15  bad
chromium@433827  48657.1  2652.53  15  bad

Bisect job ran on: android_nexus7_perf_bisect
Bug ID: 667811

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests speedometer
Test Metric: Total/Total
Relative Change: 3.68%

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus7_perf_bisect/builds/3511
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8995272559618923136


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

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

A lot more alerts for other platforms added. What is the plan regarding this CL?

Short term: Trade off. The improvements are basically the inverse of the revert which are visible in issue 669144.

Longer term: Profile why we are slower on speedometer. My guess is that we do non-incremental back-to-back GCs which shouldn't happen.

#6 Also, I just realized that the a lot of the graphs didn't recover when I reverted the flag yesterday. So there is definitely something screwed up here. 

This is definitely not all wrapper tracing, since it is currently off.

There are much bigger (up to 300%) regressions for blink_perf.paint:
https://chromeperf.appspot.com/report?sid=47011993e2884faee8136c99202c112d737df5c05e0633b635922f5c906c0755

This is really a big regression.

Here is the bisect result: https://build.chromium.org/p/tryserver.chromium.perf/builders/linux_perf_bisect/builds/6907/steps/Post%20bisect%20results/logs/json.output

This is already on my list, but I need to sort the crashers out first.

I am quite surprised to see that paint benchmarks cause full V8 GCs.

Issue 671615 has been merged into this issue.

We are landing some important painting changes, and accurate painting performance data is really important to us. Would be there be any problem if I disable TraceWrappables for blink_perf tests?

I would like to understand first why a full V8 GC happens on your critical painting path. Are the tests JS-based, or where is JS involved? Are you somehow forcing GCs?

Yes, they are forcing GCs before each iteration of a test. See https://cs.chromium.org/chromium/src/third_party/WebKit/PerformanceTests/resources/runner.js?q=runner.js&sq=package:chromium&l=102.

It was added to stabilize some unreliable perf test:
https://chromium.googlesource.com/chromium/src/+/77923c638f721d9345a45d61a66cf2556aef1aec

I think it's to reduce interference between iterations of tests because garbage produced in the previous iteration may affect performance of the current iteration.

Thanks for clearing up that mystery.

I totally understand the reasoning behind doing the GCs between iterations. However, they probably should not be included in the painting metric that is ultimately reported. 

The advantage of wrapper tracing is that it works really well in an incremental setting with the downside that it can be slower than object grouping in those forced GC settings. Even if we speed up the GCs in this case a bit (which honestly might still leave us with a regression), any change in V8s GC heuristics will have a direct impact onto that metric.

Since we really want to get rid of object grouping asap I'd really prefer not disabling wrapper tracing for those tests but would rather advise to excluding the GCs from the timeframes.

Thanks for the explanation. We have several types of measurements: time duration of each iteration, time difference between frame start times, runs per second, etc. For the first type we already excluded GC time. For the other types, we can force GC before all of the iterations. Will create a patch soon.

Yeah, we should exclude time for forced GCs from the performance metric.

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

commit a61df787643d5238a3f902a2acfafc85c0e6690f
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Thu Dec 08 01:21:36 2016

Force GC in different ways for different measurements in blink_perf tests

Previously we force GC before each iteration of test, but for frame time
measurement, the result is much affected by GC performance. Now force
GC in different ways:

- measureTime: force GC before recording the start time of each
  iteration;
- measureRunsPerSecond: same as above;
- measureFrameTime: force GC before starting the whole test;

Other measurements either call one of the above measurement methods, or
don't call scheduleNextRun() which previously forced GC so not affected.

BUG= 667811 

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

[modify] https://crrev.com/a61df787643d5238a3f902a2acfafc85c0e6690f/third_party/WebKit/PerformanceTests/resources/runner.js

 Issue 672559  has been merged into this issue.

 Issue 674973  has been merged into this issue.

This regression really is only about Speedometer now. 

https://codereview.chromium.org/2585433003/ is the first fix that is in flight for this one.

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

commit 4f161eac0b1f1b76ef9cd36de5a6dce3c9002a71
Author: mlippautz <mlippautz@chromium.org>
Date: Wed Dec 21 12:29:54 2016

[wrapper-tracing] Reduce marking overhead: Tri-color marking without 3 colors

This CL implements marking very similar to tri-color marking, with the
difference to standard GC literature being that the grey state is being made
explicit. The colors are logically represented by the tuple (color,
in_marking_deque):
- white: (0, false)
- grey: (1, true)
- black: (1, false)

This works because there is currently no need to query whether an object is grey
as we don't need a path to recover from e.g. a marking deque overflow.

Performance for known regressions:
- blink.perf_bindings structured-clone-long-string-serialize: -17% instead of
  +17%
- speedometer: Reducing marking deque pushes from 3.3M to 1.7M. This is means
  that ~50% of all marking deque pushes were duplicates.

BUG= chromium:468240 ,  chromium:667811 ,  chromium:669188 

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

[modify] https://crrev.com/4f161eac0b1f1b76ef9cd36de5a6dce3c9002a71/third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitor.h
[modify] https://crrev.com/4f161eac0b1f1b76ef9cd36de5a6dce3c9002a71/third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitorTest.cpp
[modify] https://crrev.com/4f161eac0b1f1b76ef9cd36de5a6dce3c9002a71/third_party/WebKit/Source/platform/heap/GarbageCollected.h
[modify] https://crrev.com/4f161eac0b1f1b76ef9cd36de5a6dce3c9002a71/third_party/WebKit/Source/platform/heap/TraceTraits.h
[modify] https://crrev.com/4f161eac0b1f1b76ef9cd36de5a6dce3c9002a71/third_party/WebKit/Source/platform/heap/WrapperVisitor.h

The regression recovered with the fix in #23.