Add tracing/bin/strip_memory_infra_trace

Add tracing/bin/strip_memory_infra_trace

We are running frequently in cases where traces w/ memory-infra
are too big and fail to load, as their uncompressed size > 256M.
While this is going to be fixed by bit.ly/TracingV2 we need a
short term solution to open those traces, to deal with cases like
crbug.com/623945.
This script strips a trace with memory-infra keeping only the last
two global dumps. It still guarantees that all processes for a dump
are seen, and tries to maintain the detailed dumps, if any.

BUG=chromium:630965
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/cbd0aa3300c54a918c9c18d3340e4630cda24b81

[Primiano Tucci (use gerrit), 2016-07-26 15:36:36]

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

[Primiano Tucci (use gerrit), 2016-07-26 15:36:37]

Juan can I ask you a small review? Petr is out this week.
Nothing too urgent though, so low prio here.

[perezju, 2016-07-27 10:28:14]

Just a few comments here and there

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
File tracing/tracing_build/strip_memory_infra_trace.py (right):

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:16: units = units[1:]
nit: units.pop(0)

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:18: 
nit: extra blank line needed

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:30: out_path =
in_path.split('.json')[0] + '-filtered.json'
I would move this line to either before opening the file, or right before
needing it when writing the out file below.

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:35: fin.close()
Here you could still do

  with fin:
    print 'Loading ...'
    trace = json.load(fin)
    print 'Done ...'

to ensure that the file is always properly closed.

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:70: ndumps = 2
should this be an arg one can pass from the command line?

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:75: lod =
global_dump[0].get('args', {}).get('dumps', {}).get('level_of_detail')
here this would be clearer/easier:

  try:
    lod = global_dump[0]['args']['dumps']['level_of_detail']
  except KeyError:
    lod = None

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:81: dumps_to_preserve =
detailed_dumps[-ndumps:]
nit: you could

  ndumps -= len(dumps_to_preserve)

after this line and then the if condition and the array slicing below become
both cleaner.

[Primiano Tucci (use gerrit), 2016-07-27 10:38:03]

Thanks Juan

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
File tracing/tracing_build/strip_memory_infra_trace.py (right):

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:16: units = units[1:]
On 2016/07/27 10:28:14, perezju wrote:
> nit: units.pop(0)

Done.

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:18: 
On 2016/07/27 10:28:13, perezju wrote:
> nit: extra blank line needed

Done.

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:30: out_path =
in_path.split('.json')[0] + '-filtered.json'
On 2016/07/27 10:28:14, perezju wrote:
> I would move this line to either before opening the file, or right before
> needing it when writing the out file below.

Done.

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:35: fin.close()
On 2016/07/27 10:28:14, perezju wrote:
> Here you could still do
> 
>   with fin:
>     print 'Loading ...'
>     trace = json.load(fin)
>     print 'Done ...'
> 
> to ensure that the file is always properly closed.

ah, TIL

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:70: ndumps = 2
On 2016/07/27 10:28:14, perezju wrote:
> should this be an arg one can pass from the command line?

yeah I was thinking to that, but honestly want to keep it simple until somebody
has the need of this.
The best would be inferring the best size which keeps the trace < 256M, but that
is too much effort :)

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:75: lod =
global_dump[0].get('args', {}).get('dumps', {}).get('level_of_detail')
On 2016/07/27 10:28:13, perezju wrote:
> here this would be clearer/easier:
> 
>   try:
>     lod = global_dump[0]['args']['dumps']['level_of_detail']
>   except KeyError:
>     lod = None

Ah brilliant.

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:81: dumps_to_preserve =
detailed_dumps[-ndumps:]
On 2016/07/27 10:28:14, perezju wrote:
> nit: you could
> 
>   ndumps -= len(dumps_to_preserve)
> 
> after this line and then the if condition and the array slicing below become
> both cleaner.

Done.

[perezju, 2016-07-27 11:02:23]

lgtm, thanks!

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
File tracing/tracing_build/strip_memory_infra_trace.py (right):

https://codereview.chromium.org/2184783002/diff/1/tracing/tracing_build/strip...
tracing/tracing_build/strip_memory_infra_trace.py:70: ndumps = 2
On 2016/07/27 10:38:03, Primiano Tucci wrote:
> On 2016/07/27 10:28:14, perezju wrote:
> > should this be an arg one can pass from the command line?
> 
> yeah I was thinking to that, but honestly want to keep it simple until
somebody
> has the need of this.
> The best would be inferring the best size which keeps the trace < 256M, but
that
> is too much effort :)

stgm :)

[Primiano Tucci (use gerrit), 2016-07-27 12:13:23]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2016-07-27 12:13:34]

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, 2016-07-27 12:38:14]

Description was changed from

==========
Add tracing/bin/strip_memory_infra_trace

We are running frequently in cases where traces w/ memory-infra
are too big and fail to load, as their uncompressed size > 256M.
While this is going to be fixed by bit.ly/TracingV2 we need a
short term solution to open those traces, to deal with cases like
crbug.com/623945.
This script strips a trace with memory-infra keeping only the last
two global dumps. It still guarantees that all processes for a dump
are seen, and tries to maintain the detailed dumps, if any.

BUG=chromium:630965
==========

to

==========
Add tracing/bin/strip_memory_infra_trace

We are running frequently in cases where traces w/ memory-infra
are too big and fail to load, as their uncompressed size > 256M.
While this is going to be fixed by bit.ly/TracingV2 we need a
short term solution to open those traces, to deal with cases like
crbug.com/623945.
This script strips a trace with memory-infra keeping only the last
two global dumps. It still guarantees that all processes for a dump
are seen, and tries to maintain the detailed dumps, if any.

BUG=chromium:630965

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-07-27 12:38:14]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...