[Telemetry] Add a profiler based loading measurement.

[Telemetry] Add a profiler based loading measurement.

This measurement reports the hottest functions during page load. I plan to run
it over the top million sites.

It works by loading each page twice, the profile is captured for the second load
only. This is because cross-origin navigations cause the renderer to be swapped
out during profiling and to get more stable results.

This required a couple of core modifications:
1. csv_page_measurement_results should work when the page runner creates a
   dummy results object because discard_first_results is enabled.
2. discard_first_results should discard the first result of each page in
   the page set. Not just the first page.

BUG=None

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=213279

[nduca, 2013-07-23 06:01:27]

lgtm on my part but maybe dtu can look at the first page business

https://codereview.chromium.org/19857003/diff/1/tools/perf/measurements/loadi...
File tools/perf/measurements/loading_measurement_analyzer.py (right):

https://codereview.chromium.org/19857003/diff/1/tools/perf/measurements/loadi...
tools/perf/measurements/loading_measurement_analyzer.py:48: if not value or
value == '-':
i think this script is starting to get to the point where unit tests should
happen.... which might mean introducing a ./run_perf_unittests in this folder...

https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/platform/profiler/perf_profiler.py (right):

https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/platform/profiler/perf_profiler.py:91: def
GetTopSamples(cls, file_name, number):
wdyt about unit test for this? not the full profiler stack, but just the parsing
thing

https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/platform/profiler/perf_profiler.py:91: def
GetTopSamples(cls, file_name, number):
docstring would be good too about what is returned

https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/pag...
File tools/telemetry/telemetry/page/page_runner.py (right):

https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/pag...
tools/telemetry/telemetry/page/page_runner.py:73: if self.first_page[page]:
can dtu look at this? this seems to change the semantics of the first_page quite
a bit...

[nduca, 2013-07-23 06:01:49]

(if the first page stuff is right, lets add a unit test for whatever behavior we
expect)

[tonyg, 2013-07-23 18:41:21]

https://codereview.chromium.org/19857003/diff/1/tools/perf/measurements/loadi...
File tools/perf/measurements/loading_measurement_analyzer.py (right):

https://codereview.chromium.org/19857003/diff/1/tools/perf/measurements/loadi...
tools/perf/measurements/loading_measurement_analyzer.py:48: if not value or
value == '-':
On 2013/07/23 06:01:27, nduca wrote:
> i think this script is starting to get to the point where unit tests should
> happen.... which might mean introducing a ./run_perf_unittests in this
folder...

You are probably right. Would you mind terribly if I did that in a separate
patch to avoid bloating this one?

https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/platform/profiler/perf_profiler.py (right):

https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/platform/profiler/perf_profiler.py:91: def
GetTopSamples(cls, file_name, number):
On 2013/07/23 06:01:27, nduca wrote:
> wdyt about unit test for this? not the full profiler stack, but just the
parsing
> thing

Good idea. Done.

https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/platform/profiler/perf_profiler.py:91: def
GetTopSamples(cls, file_name, number):
On 2013/07/23 06:01:27, nduca wrote:
> docstring would be good too about what is returned

Done.

https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/pag...
File tools/telemetry/telemetry/page/page_runner.py (right):

https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/pag...
tools/telemetry/telemetry/page/page_runner.py:73: if self.first_page[page]:
On 2013/07/23 06:01:27, nduca wrote:
> can dtu look at this? this seems to change the semantics of the first_page
quite
> a bit...

My understanding is that this was added for the startup test and that is still
the only user. The startup test runs with the blank_page.json page set which
only consists of a single page. So an implementation that skips only the first
page is equivalent to one that skips the first cycle of each page in the page
set.

It seems more correct to me for an option called "discard_first_results" to
discard the first run of each page in the page set, not just the first run.

Please let me know if you disagree.

In the meantime I updated the test case to cover this.

[dtu, 2013-07-23 20:09:43]

https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/pag...
File tools/telemetry/telemetry/page/page_runner.py (right):

https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/pag...
tools/telemetry/telemetry/page/page_runner.py:73: if self.first_page[page]:
On 2013/07/23 18:41:21, tonyg wrote:
> On 2013/07/23 06:01:27, nduca wrote:
> > can dtu look at this? this seems to change the semantics of the first_page
> quite
> > a bit...
> 
> My understanding is that this was added for the startup test and that is still
> the only user. The startup test runs with the blank_page.json page set which
> only consists of a single page. So an implementation that skips only the first
> page is equivalent to one that skips the first cycle of each page in the page
> set.
> 
> It seems more correct to me for an option called "discard_first_results" to
> discard the first run of each page in the page set, not just the first run.
> 
> Please let me know if you disagree.
> 
> In the meantime I updated the test case to cover this.

Yeah, StartupWarm is the only user and will be the same either way, so whatever
you decide here is fine.

This also affects WillRunPageSet, but edmundyan's patch moved it somewhere else
entirely so we good.
https://chromiumcodereview.appspot.com/18261009/

[tonyg, 2013-07-23 20:27:53]

On 2013/07/23 20:09:43, Dave Tu wrote:
>
https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/pag...
> File tools/telemetry/telemetry/page/page_runner.py (right):
> 
>
https://codereview.chromium.org/19857003/diff/1/tools/telemetry/telemetry/pag...
> tools/telemetry/telemetry/page/page_runner.py:73: if self.first_page[page]:
> On 2013/07/23 18:41:21, tonyg wrote:
> > On 2013/07/23 06:01:27, nduca wrote:
> > > can dtu look at this? this seems to change the semantics of the first_page
> > quite
> > > a bit...
> > 
> > My understanding is that this was added for the startup test and that is
still
> > the only user. The startup test runs with the blank_page.json page set which
> > only consists of a single page. So an implementation that skips only the
first
> > page is equivalent to one that skips the first cycle of each page in the
page
> > set.
> > 
> > It seems more correct to me for an option called "discard_first_results" to
> > discard the first run of each page in the page set, not just the first run.
> > 
> > Please let me know if you disagree.
> > 
> > In the meantime I updated the test case to cover this.
> 
> Yeah, StartupWarm is the only user and will be the same either way, so
whatever
> you decide here is fine.
> 
> This also affects WillRunPageSet, but edmundyan's patch moved it somewhere
else
> entirely so we good.
> https://chromiumcodereview.appspot.com/18261009/

Thanks for confirming. I'm going to take Nat's previous lgtm then.

[commit-bot: I haz the power, 2013-07-23 20:28:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tonyg@chromium.org/19857003/8001

[commit-bot: I haz the power, 2013-07-23 20:34:00]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2013-07-23 21:11:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tonyg@chromium.org/19857003/8001

[commit-bot: I haz the power, 2013-07-23 23:54:45]

Change committed as 213279