[Oilpan] Measure time for forced/idle GC

This CL starts to track measurements of time for forced GC
and idle GC.

Before this CL, OilpanGCTimes metric measured time for
'precise' and 'conservative' GC, but now Oilpan GC can run for
other reasons, and some tests need to measure such 'other' GC runs.

In short, we had ['precise', 'conervative'] metrics, and
we will have ['precise', 'conervative', 'forced', 'idle']
metrics.

BUG=438074
TEST=./tools/perf/run_tests oilpan --browser=content-shell-release

Committed: https://crrev.com/829f2d19573f0d33e94f76686b5904f7db4c1b09
Cr-Commit-Position: refs/heads/master@{#323651}

[peria, 2015-03-02 04:53:58]

peria@chromium.org changed reviewers:
+ kouhei@chromium.org, oilpan-reviews@chromium.org

[peria, 2015-03-02 04:53:58]

PTL

[haraken, 2015-03-02 05:01:49]

> In performance test, most GCs are triggered by telemetry.
> So we have to measure forced GC time duration to check GC performance.

Would you elaborate on why? In my understanding, we want to measure only GCs
triggered during benchmarks, excluding forced GCs triggered between two
benchmarks by telemetry.

[kouhei (in TOK), 2015-03-02 05:03:57]

On 2015/03/02 05:01:49, haraken wrote:
> > In performance test, most GCs are triggered by telemetry.
> > So we have to measure forced GC time duration to check GC performance.
> 
> Would you elaborate on why? In my understanding, we want to measure only GCs
> triggered during benchmarks, excluding forced GCs triggered between two
> benchmarks by telemetry.

I think we should add a flag. We should only include "forced gcs" on stress gc
benchmarks.

https://codereview.chromium.org/968773002/diff/1/tools/perf/measurements/oilp...
File tools/perf/measurements/oilpan_gc_times.py (right):

https://codereview.chromium.org/968773002/diff/1/tools/perf/measurements/oilp...
tools/perf/measurements/oilpan_gc_times.py:6: import time
Remove.

[peria, 2015-03-02 05:27:58]

Add a flag to include/exclude forced GCs.
CL description is also updated.

PTAL.

https://codereview.chromium.org/968773002/diff/1/tools/perf/measurements/oilp...
File tools/perf/measurements/oilpan_gc_times.py (right):

https://codereview.chromium.org/968773002/diff/1/tools/perf/measurements/oilp...
tools/perf/measurements/oilpan_gc_times.py:6: import time
On 2015/03/02 05:03:57, kouhei wrote:
> Remove.

Done.

[kouhei (in TOK), 2015-03-02 05:29:21]

lgtm

Nit: s/include_force_gc/include_forced_gc/

[haraken, 2015-03-02 05:29:55]

LGTM

[peria, 2015-03-02 05:33:41]

The CQ bit was checked by peria@chromium.org

[peria, 2015-03-02 05:33:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, kouhei@chromium.org
Link to the patchset: https://codereview.chromium.org/968773002/#ps40001 (title:
"Rebase and work for a nit")

[commit-bot: I haz the power, 2015-03-02 05:34:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/968773002/40001

[peria, 2015-03-02 05:40:27]

peria@chromium.org changed reviewers:
+ skyostil@chromium.org

[peria, 2015-03-02 05:40:28]

Sami, could you take a look as an owner?

[commit-bot: I haz the power, 2015-03-02 05:50:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-02 05:50:52]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Sami, 2015-03-02 12:37:59]

Thanks! Would you mind adding a test for this in OilpanGCTimesTest? The test
could use synthetic data instead of the real browser if it makes things simpler.

https://codereview.chromium.org/968773002/diff/40001/tools/perf/measurements/...
File tools/perf/measurements/oilpan_gc_times.py (right):

https://codereview.chromium.org/968773002/diff/40001/tools/perf/measurements/...
tools/perf/measurements/oilpan_gc_times.py:109: def __init__(self, action_name =
'', include_forced_gc = False):
nit: no spaces around the '=' for default parameters.

https://codereview.chromium.org/968773002/diff/40001/tools/perf/measurements/...
tools/perf/measurements/oilpan_gc_times.py:174: super(OilpanGCTimesForInternals,
self).__init__(include_forced_gc = True)
No spaces here either.

[peria, 2015-03-09 08:03:25]

Thank you for the review.

I added a test and a backdoor method for the test.
I think the test should be more smart, though...

https://codereview.chromium.org/968773002/diff/40001/tools/perf/measurements/...
File tools/perf/measurements/oilpan_gc_times.py (right):

https://codereview.chromium.org/968773002/diff/40001/tools/perf/measurements/...
tools/perf/measurements/oilpan_gc_times.py:109: def __init__(self, action_name =
'', include_forced_gc = False):
On 2015/03/02 12:37:59, Sami wrote:
> nit: no spaces around the '=' for default parameters.

Done.

https://codereview.chromium.org/968773002/diff/40001/tools/perf/measurements/...
tools/perf/measurements/oilpan_gc_times.py:174: super(OilpanGCTimesForInternals,
self).__init__(include_forced_gc = True)
On 2015/03/02 12:37:59, Sami wrote:
> No spaces here either.

Done.

[Sami, 2015-03-09 16:29:52]

Many thanks for adding the test. I've got a suggestion to simplify it a little
bit.

https://codereview.chromium.org/968773002/diff/80001/tools/perf/measurements/...
File tools/perf/measurements/oilpan_gc_times.py (right):

https://codereview.chromium.org/968773002/diff/80001/tools/perf/measurements/...
tools/perf/measurements/oilpan_gc_times.py:142: def _ParseEventsForTest(self,
events, include_forced_gc, options):
Instead of adding this helper here, could we follow the pattern of using
ValidateAndMeasurePage with mocked data like for example here:

https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/measure...

[peria, 2015-04-02 08:26:01]

I'm sorry for leaving this CL so long.

While I was working for other urgent tasks, the structures
of Oilpan GC and its tracing data changed.

Please review it again, all reviewers.

https://codereview.chromium.org/968773002/diff/80001/tools/perf/measurements/...
File tools/perf/measurements/oilpan_gc_times.py (right):

https://codereview.chromium.org/968773002/diff/80001/tools/perf/measurements/...
tools/perf/measurements/oilpan_gc_times.py:142: def _ParseEventsForTest(self,
events, include_forced_gc, options):
On 2015/03/09 16:29:52, Sami wrote:
> Instead of adding this helper here, could we follow the pattern of using
> ValidateAndMeasurePage with mocked data like for example here:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/measure...
> 

Done.

[Sami, 2015-04-02 17:21:30]

Thanks, the new test looks great!

https://codereview.chromium.org/968773002/diff/100001/tools/perf/measurements...
File tools/perf/measurements/oilpan_gc_times.py (right):

https://codereview.chromium.org/968773002/diff/100001/tools/perf/measurements...
tools/perf/measurements/oilpan_gc_times.py:25: _GC_WORKS = ['mark',
'lazy_sweep', 'complete_sweep']
nit: would _GC_STAGES be a more descriptive name here? The current name works
too.

[Sami, 2015-04-02 17:21:45]

Oops, meant to also say lgtm.

[peria, 2015-04-03 02:51:11]

Thank you for your reviews and advices!!

https://codereview.chromium.org/968773002/diff/100001/tools/perf/measurements...
File tools/perf/measurements/oilpan_gc_times.py (right):

https://codereview.chromium.org/968773002/diff/100001/tools/perf/measurements...
tools/perf/measurements/oilpan_gc_times.py:25: _GC_WORKS = ['mark',
'lazy_sweep', 'complete_sweep']
On 2015/04/02 17:21:29, Sami wrote:
> nit: would _GC_STAGES be a more descriptive name here? The current name works
> too.

It sounds good!

[peria, 2015-04-03 02:51:18]

The CQ bit was checked by peria@chromium.org

[peria, 2015-04-03 02:51:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, kouhei@chromium.org, skyostil@chromium.org
Link to the patchset: https://codereview.chromium.org/968773002/#ps120001
(title: "Work for a nit")

[commit-bot: I haz the power, 2015-04-03 04:40:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/968773002/120001

[commit-bot: I haz the power, 2015-04-03 07:39:33]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-04-03 20:33:33]

Patchset 7 (id:??) landed as
https://crrev.com/829f2d19573f0d33e94f76686b5904f7db4c1b09
Cr-Commit-Position: refs/heads/master@{#323651}