sandwich: Builds a script to pull the metrics from the traces

sandwich: Builds a script to pull the metrics from the traces

BUG=582080
Committed: https://crrev.com/7c2b4ef2202045b91929d8d9d80ff75b48aa8676
Cr-Commit-Position: refs/heads/master@{#375181}

[gabadie, 2016-02-11 18:26:31]

gabadie@chromium.org changed reviewers:
+ lizeb@chromium.org, mattcary@chromium.org, pasko@chromium.org

[gabadie, 2016-02-11 18:26:32]

As wanted by pasko, this script generates a csv with metrics we are interested
in (page load time, onload event time, heap avg size and heap max size) per
column from all the available traces in a run_sandwich output directory.

lizeb@chromium.org & mattcary@chromium.org: PTAL.

[mattcary, 2016-02-12 09:59:47]

lgtm

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
File tools/android/loading/pull_sandwich_metrics.py (right):

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:8: python
pull_sandwich_metrics.py -h
With the #! line, this can be run directly without python.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:148: except Exception:
ValueError would be better than catching all exceptions.

[Benoit L, 2016-02-12 10:26:42]

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
File tools/android/loading/pull_sandwich_metrics.py (right):

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:29: def _GetBrowserPID(trace):
nit: Here and below, 2 lines between top-level declarations.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:61: # Ignore dumps event for
processes other than the browser process
s/dumps event/dump events/

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:66: raise Exception('No browser
dump events found.')
ValueError, as pointed out below.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:110: browser_dumps_events =
_GetBrowserDumpEvents(trace)
s/browser_dumps_events/browser_dump_events/

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:113: browser_malloc_avg = 0
Arguably, this is not an average, but OK.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:118: size = int(attr['value'],
16)
size is serialized as an hex string? Weird.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:148: except Exception:
On 2016/02/12 09:59:47, mattcary wrote:
> ValueError would be better than catching all exceptions.

Pokemon exception handling: gotta catch'em all!

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
File tools/android/loading/pull_sandwich_metrics_unittest.py (right):

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics_unittest.py:22:
_MINIMALIST_STACK_TRACE = {'traceEvents': [
Stack trace?

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics_unittest.py:51: try:
self.assertRaises() is cleaner here.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics_unittest.py:103: try:
Ditto, assertRaises()

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics_unittest.py:167: if __name__ ==
'__main__':
2 lines between top-level declarations.

[gabadie, 2016-02-12 10:39:11]

New revision uploaded addressing reviews' concerns.

lizeb@chromium.org: PTAL.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
File tools/android/loading/pull_sandwich_metrics.py (right):

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:8: python
pull_sandwich_metrics.py -h
On 2016/02/12 09:59:47, mattcary wrote:
> With the #! line, this can be run directly without python.

Oups yes indeed... Done.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:29: def _GetBrowserPID(trace):
On 2016/02/12 10:26:41, Benoit L wrote:
> nit: Here and below, 2 lines between top-level declarations.

Done.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:61: # Ignore dumps event for
processes other than the browser process
On 2016/02/12 10:26:41, Benoit L wrote:
> s/dumps event/dump events/

Done.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:66: raise Exception('No browser
dump events found.')
On 2016/02/12 10:26:41, Benoit L wrote:
> ValueError, as pointed out below.

Done.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:110: browser_dumps_events =
_GetBrowserDumpEvents(trace)
On 2016/02/12 10:26:41, Benoit L wrote:
> s/browser_dumps_events/browser_dump_events/

Done.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:113: browser_malloc_avg = 0
On 2016/02/12 10:26:41, Benoit L wrote:
> Arguably, this is not an average, but OK.

Renaming as *_sum. Done.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:118: size = int(attr['value'],
16)
On 2016/02/12 10:26:41, Benoit L wrote:
> size is serialized as an hex string? Weird.

Totally agree.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:148: except Exception:
On 2016/02/12 09:59:47, mattcary wrote:
> ValueError would be better than catching all exceptions.

Done.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
File tools/android/loading/pull_sandwich_metrics_unittest.py (right):

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics_unittest.py:22:
_MINIMALIST_STACK_TRACE = {'traceEvents': [
On 2016/02/12 10:26:41, Benoit L wrote:
> Stack trace?

Oups... Done.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics_unittest.py:51: try:
On 2016/02/12 10:26:42, Benoit L wrote:
> self.assertRaises() is cleaner here.

Done.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics_unittest.py:103: try:
On 2016/02/12 10:26:42, Benoit L wrote:
> Ditto, assertRaises()

Done.

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics_unittest.py:167: if __name__ ==
'__main__':
On 2016/02/12 10:26:41, Benoit L wrote:
> 2 lines between top-level declarations.

Done.

[mattcary, 2016-02-12 10:41:11]

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
File tools/android/loading/pull_sandwich_metrics.py (right):

https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
tools/android/loading/pull_sandwich_metrics.py:148: except Exception:
On 2016/02/12 10:39:11, gabadie wrote:
> On 2016/02/12 09:59:47, mattcary wrote:
> > ValueError would be better than catching all exceptions.
> 
> Done.

But continuing isn't actually catching them... it's misleading as the idiom here
is to ignore node names that aren't ints. If an unexpected exception happens,
we'll incorrectly squash it.

[Benoit L, 2016-02-12 14:17:12]

On 2016/02/12 10:41:11, mattcary wrote:
>
https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
> File tools/android/loading/pull_sandwich_metrics.py (right):
> 
>
https://codereview.chromium.org/1690813003/diff/1/tools/android/loading/pull_...
> tools/android/loading/pull_sandwich_metrics.py:148: except Exception:
> On 2016/02/12 10:39:11, gabadie wrote:
> > On 2016/02/12 09:59:47, mattcary wrote:
> > > ValueError would be better than catching all exceptions.
> > 
> > Done.
> 
> But continuing isn't actually catching them... it's misleading as the idiom
here
> is to ignore node names that aren't ints. If an unexpected exception happens,
> we'll incorrectly squash it.

lgtm

[gabadie, 2016-02-12 14:26:05]

The CQ bit was checked by gabadie@chromium.org

[gabadie, 2016-02-12 14:26:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
mattcary@chromium.org
Link to the patchset: https://codereview.chromium.org/1690813003/#ps20001
(title: "Addresses all review concerns")

[commit-bot: I haz the power, 2016-02-12 14:26:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1690813003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1690813003/20001

[commit-bot: I haz the power, 2016-02-12 14:48:24]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-16 22:43:11]

Description was changed from

==========
sandwich: Builds a script to pull the metrics from the traces

BUG=582080
==========

to

==========
sandwich: Builds a script to pull the metrics from the traces

BUG=582080

Committed: https://crrev.com/7c2b4ef2202045b91929d8d9d80ff75b48aa8676
Cr-Commit-Position: refs/heads/master@{#375181}
==========

[commit-bot: I haz the power, 2016-02-16 22:43:12]

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