memory-infra: remove debugging flow-events

memory-infra: remove debugging flow-events

The Trace Viewer UI can hang / OOM if the flow events are not
consistent. Very likely this started happening after memory-infra
started supported queueing of memory dump requests.
Flow events never added too much value, so ditching them to avoid
UI issues. Also this CL switches to the new TRACE_ID_LOCAL trace
macro for the remaining debugging events.

BUG=706042
Review-Url: https://codereview.chromium.org/2778743005
Cr-Commit-Position: refs/heads/master@{#460238}
Committed: https://chromium.googlesource.com/chromium/src/+/21ea0c9b1eb01f5cd463d812c6b9a759d5227239

[Primiano Tucci (use gerrit), 2017-03-28 19:31:28]

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

[commit-bot: I haz the power, 2017-03-28 19:32:21]

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-03-28 19:32:42]

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

[ssid, 2017-03-28 20:21:56]

lgtm, some not very important comments.

https://codereview.chromium.org/2778743005/diff/1/base/trace_event/memory_dum...
File base/trace_event/memory_dump_manager.cc (right):

https://codereview.chromium.org/2778743005/diff/1/base/trace_event/memory_dum...
base/trace_event/memory_dump_manager.cc:89: sprintf(guid_str, "0x%" PRIx64,
dump_guid);
Do we need 0x?
base::Uint64ToString looks better, though it creates std::string.

https://codereview.chromium.org/2778743005/diff/1/base/trace_event/memory_dum...
base/trace_event/memory_dump_manager.cc:507: TRACE_STR_COPY(guid_str));
For ProcessMemoryDump the guid is added at the BEGIN and for GlobalMemoryDump it
is added at the END events. Is there a reason?

[commit-bot: I haz the power, 2017-03-28 21:16:08]

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

[commit-bot: I haz the power, 2017-03-28 21:16:09]

Dry run: CQ has no permission to schedule in bucket
master.tryserver.chromium.linux

[Primiano Tucci (use gerrit), 2017-03-28 23:13:15]

thanks

https://codereview.chromium.org/2778743005/diff/1/base/trace_event/memory_dum...
File base/trace_event/memory_dump_manager.cc (right):

https://codereview.chromium.org/2778743005/diff/1/base/trace_event/memory_dum...
base/trace_event/memory_dump_manager.cc:89: sprintf(guid_str, "0x%" PRIx64,
dump_guid);
On 2017/03/28 20:21:56, ssid wrote:
> Do we need 0x?
not too strong but helps debuggin to copy/paste and search in the trace json.

> base::Uint64ToString looks better, though it creates std::string.
I know, but that is a back and forth of string copies. This one seemed simple
enough

https://codereview.chromium.org/2778743005/diff/1/base/trace_event/memory_dum...
base/trace_event/memory_dump_manager.cc:507: TRACE_STR_COPY(guid_str));
On 2017/03/28 20:21:56, ssid wrote:
> For ProcessMemoryDump the guid is added at the BEGIN and for GlobalMemoryDump
it
> is added at the END events. 
Aha I knew, well spotted.

> Is there a reason?
Yes, cause I am a cheat :P
Each nestable async event can carry at most 2 args. But when you have a
begin/end pair the trace viewer ui will merge the args together, so by having 2
here and 2 there I can have the full four args in total :P

[Primiano Tucci (use gerrit), 2017-03-28 23:13:23]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2017-03-28 23:13:59]

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

[ssid, 2017-03-28 23:21:45]

https://codereview.chromium.org/2778743005/diff/1/base/trace_event/memory_dum...
File base/trace_event/memory_dump_manager.cc (right):

https://codereview.chromium.org/2778743005/diff/1/base/trace_event/memory_dum...
base/trace_event/memory_dump_manager.cc:507: TRACE_STR_COPY(guid_str));
On 2017/03/28 23:13:14, Primiano Tucci wrote:
> On 2017/03/28 20:21:56, ssid wrote:
> > For ProcessMemoryDump the guid is added at the BEGIN and for
GlobalMemoryDump
> it
> > is added at the END events. 
> Aha I knew, well spotted.
> 
> > Is there a reason?
> Yes, cause I am a cheat :P
> Each nestable async event can carry at most 2 args. But when you have a
> begin/end pair the trace viewer ui will merge the args together, so by having
2
> here and 2 there I can have the full four args in total :P

LOL!

[commit-bot: I haz the power, 2017-03-28 23:38:10]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1490742803452990, "parent_rev":
"772204b647bb8089ed2017718cc4afdeac1d80d1", "commit_rev":
"21ea0c9b1eb01f5cd463d812c6b9a759d5227239"}

[commit-bot: I haz the power, 2017-03-28 23:39:48]

Description was changed from

==========
memory-infra: remove debugging flow-events

The Trace Viewer UI can hang / OOM if the flow events are not
consistent. Very likely this started happening after memory-infra
started supported queueing of memory dump requests.
Flow events never added too much value, so ditching them to avoid
UI issues. Also this CL switches to the new TRACE_ID_LOCAL trace
macro for the remaining debugging events.

BUG=706042
==========

to

==========
memory-infra: remove debugging flow-events

The Trace Viewer UI can hang / OOM if the flow events are not
consistent. Very likely this started happening after memory-infra
started supported queueing of memory dump requests.
Flow events never added too much value, so ditching them to avoid
UI issues. Also this CL switches to the new TRACE_ID_LOCAL trace
macro for the remaining debugging events.

BUG=706042

Review-Url: https://codereview.chromium.org/2778743005
Cr-Commit-Position: refs/heads/master@{#460238}
Committed:
https://chromium.googlesource.com/chromium/src/+/21ea0c9b1eb01f5cd463d812c6b9...
==========

[commit-bot: I haz the power, 2017-03-28 23:39:49]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/21ea0c9b1eb01f5cd463d812c6b9...