memory-infra: remove light dumps and increment default dump time to 5s

memory-infra: remove light dumps and increment default dump time to 5s

This CL changes the default behavior (i.e. when no trace config is
passed) of memory-infra, by enabling only detailed dumps and only
every 5 seconds (instead of 2). Rationale:
- Nobody seems to make a use of light dumps in the UI.
- 2 seconds is too aggressive for detailed dumps, especially when using
  the heap profiler.
The support for light dumps itself is NOT removed. They can still be
requested via the trace config. This is to support some telemetry
benchmarks (v8) that rely on them.

Review-Url: https://codereview.chromium.org/2902413002
Cr-Commit-Position: refs/heads/master@{#474982}
Committed: https://chromium.googlesource.com/chromium/src/+/bddc4ba03f742af167194a52a0a92516976fa9fb

[Primiano Tucci (use gerrit), 2017-05-25 17:26:40]

The CQ bit was checked by primiano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-25 17:27:20]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Primiano Tucci (use gerrit), 2017-05-25 17:29:04]

primiano@chromium.org changed reviewers:
+ erikchen@chromium.org

[Primiano Tucci (use gerrit), 2017-05-25 17:29:05]

Let's start with just 5 s and see how it goes?

+ulan/hpayer FYI: this change should NOT affect your v8 benchmarks as I am not
removing the support for light dumps, just making them not default. 

From what I see from
https://cs.chromium.org/chromium/src/tools/perf/benchmarks/v8_browsing.py?rcl...
 you are explicitly setting the trace config, so this change shoudl not affect
you.

However, if there is a bug somewhere, and you see some shift in your metrics,
keep an eye on this. (So I guess what I am saying is that there is a 1%
probability that this CL might accidentally screw your benchmark. if that's the
case file a bug/revert)

[erikchen, 2017-05-25 17:31:42]

lgtm

[Primiano Tucci (use gerrit), 2017-05-25 17:48:24]

+ssid fiy

[commit-bot: I haz the power, 2017-05-25 17:55:59]

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

[commit-bot: I haz the power, 2017-05-25 17:56:00]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ssid, 2017-05-25 18:20:53]

On 2017/05/25 17:48:24, Primiano Tucci wrote:
> +ssid fiy

lg!

[Primiano Tucci (use gerrit), 2017-05-26 10:02:48]

ssid: I am afraid I am going to undo  your
https://codereview.chromium.org/2815963002.
With 5 seconds delay it breaks the workflow people are used to that is: click on
record and then almost immediately click stop.
Also I realized only now that https://codereview.chromium.org/2815963002 is what
very likely caused the new flakiness on TraceOnTap (crbug.com/698400). WIth 2
seconds of pre-delay TraceOnTap is left with only 1 second of margin. 5 seconds
here is going to screw it up completely.

The reason mentioned in that CL is "It is makes startup tracing worse in case we
want a single dump and not the dump at start."
But I think in that case we should just use
third_party/catapult/tracing/bin/memory_infra_remote_dump

Speculatively landing to check if this creates any flakiness. Happy to
re-iterate on this on a next CL if you are not happy with the resolution.
We should just fix crbug.com/726393 asap

[Primiano Tucci (use gerrit), 2017-05-26 10:03:17]

The CQ bit was checked by primiano@chromium.org

[Primiano Tucci (use gerrit), 2017-05-26 10:03:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
erikchen@chromium.org
Link to the patchset: https://codereview.chromium.org/2902413002/#ps40001
(title: "Undo crrev.com/2815963002, fire first timer immediately")

[commit-bot: I haz the power, 2017-05-26 10:03:30]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-26 11:32:57]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495792997243400,
"parent_rev": "063f170eccba918132f6ec4c5248c95e5e7f51b5", "commit_rev":
"bddc4ba03f742af167194a52a0a92516976fa9fb"}

[commit-bot: I haz the power, 2017-05-26 11:33:06]

Description was changed from

==========
memory-infra: remove light dumps and increment default dump time to 5s

This CL changes the default behavior (i.e. when no trace config is
passed) of memory-infra, by enabling only detailed dumps and only
every 5 seconds (instead of 2). Rationale:
- Nobody seems to make a use of light dumps in the UI.
- 2 seconds is too aggressive for detailed dumps, especially when using
  the heap profiler.
The support for light dumps itself is NOT removed. They can still be
requested via the trace config. This is to support some telemetry
benchmarks (v8) that rely on them.
==========

to

==========
memory-infra: remove light dumps and increment default dump time to 5s

This CL changes the default behavior (i.e. when no trace config is
passed) of memory-infra, by enabling only detailed dumps and only
every 5 seconds (instead of 2). Rationale:
- Nobody seems to make a use of light dumps in the UI.
- 2 seconds is too aggressive for detailed dumps, especially when using
  the heap profiler.
The support for light dumps itself is NOT removed. They can still be
requested via the trace config. This is to support some telemetry
benchmarks (v8) that rely on them.

Review-Url: https://codereview.chromium.org/2902413002
Cr-Commit-Position: refs/heads/master@{#474982}
Committed:
https://chromium.googlesource.com/chromium/src/+/bddc4ba03f742af167194a52a0a9...
==========

[commit-bot: I haz the power, 2017-05-26 11:33:07]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/bddc4ba03f742af167194a52a0a9...

[ssid, 2017-05-26 13:30:23]

On 2017/05/26 11:33:07, commit-bot: I haz the power wrote:
> Committed patchset #3 (id:40001) as
>
https://chromium.googlesource.com/chromium/src/+/bddc4ba03f742af167194a52a0a9...

Actually realized that the first dump will be triggered after 5 seconds. So it
might make each tracing season uncomfortably long

[Primiano Tucci (use gerrit), 2017-05-26 14:29:17]

On 2017/05/26 13:30:23, ssid wrote:
> On 2017/05/26 11:33:07, commit-bot: I haz the power wrote:
> > Committed patchset #3 (id:40001) as
> >
>
https://chromium.googlesource.com/chromium/src/+/bddc4ba03f742af167194a52a0a9...
> 
> Actually realized that the first dump will be triggered after 5 seconds. So it
> might make each tracing season uncomfortably long

See my comment #10: not anymore :)