Speed up the EQT metric computation.

Speed up the EQT metric computation.

Performance profiling shows that computing TTI is expensive.

This patch shares the computed TTI between the main EQT metric and
the diagnostic metrics. The performance win is ~10 seconds per story
in v8.infinite_scroll_tbmv2 benchmark.

BUG=chromium:625701
Review-Url: https://codereview.chromium.org/2769643002
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/7c7b8a1defa3e74e8c947461b6a4a5aa705594a1

[ulan, 2017-03-22 12:29:21]

Description was changed from

==========
Speed up expectedQueueingTimeMetric.

Performance profiling shows that computing TTI is expensive.

This patch shares the computed TTI between the main EQT metric and
the diagnostic metrics. The performance win is ~10 seconds per story
in v8.infinite_scroll_tbmv2 benchmark.

BUG=chromium:625701
==========

to

==========
Speed up the EQT metric computation.

Performance profiling shows that computing TTI is expensive.

This patch shares the computed TTI between the main EQT metric and
the diagnostic metrics. The performance win is ~10 seconds per story
in v8.infinite_scroll_tbmv2 benchmark.

BUG=chromium:625701
==========

[ulan, 2017-03-22 12:29:48]

ulan@chromium.org changed reviewers:
+ benjhayden@chromium.org

[ulan, 2017-03-22 12:29:49]

PTAL.

https://codereview.chromium.org/2769643002/diff/1/tracing/tracing/metrics/sys...
File tracing/tracing/metrics/system_health/expected_queueing_time_metric.html
(right):

https://codereview.chromium.org/2769643002/diff/1/tracing/tracing/metrics/sys...
tracing/tracing/metrics/system_health/expected_queueing_time_metric.html:107:
const rendererToInteractiveTimestamps =
This is the main change.

[benjhayden, 2017-03-22 22:56:47]

Wow, 10s/story! LGTM!

Can you share a little bit about how you measured that? We could add a section
to how-to-write-metrics about profiling metric performance.

Are there any next steps? More low-hanging fruit?

[ulan, 2017-03-23 09:18:36]

On 2017/03/22 22:56:47, benjhayden wrote:
> Wow, 10s/story! LGTM!
> 
> Can you share a little bit about how you measured that? We could add a section
> to how-to-write-metrics about profiling metric performance.
> 
I recorded a trace of a benchmark run using --output-format=json. Then loaded
the trace in Chrome and used the DevTools CPU profiler on the EQT metric.

For measuring metric run-time before and after the change I used
./catapult/tracing/bin/run_metric on the same trace.

> Are there any next steps? More low-hanging fruit?
There is definitely room for optimization in the metrics that I know (V8,
loading, EQT). The lowest-hanging fruit is to avoid iterating all the events
multiple times. This would require optimizations similar to what I did in this
patch. So this is a trade-off between code complexity and performance.

Is there a project on improving metrics performance? If not, should we start it?
I think the first step would be to track metrics performance in telemetry.

[ulan, 2017-03-23 09:18:41]

The CQ bit was checked by ulan@chromium.org

[commit-bot: I haz the power, 2017-03-23 09:18:48]

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-03-23 09:41:05]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1490260721332800, "parent_rev":
"1ee9f83bf02dd0421dfd927c41d0163318ae421c", "commit_rev":
"7c7b8a1defa3e74e8c947461b6a4a5aa705594a1"}

[commit-bot: I haz the power, 2017-03-23 09:41:09]

Description was changed from

==========
Speed up the EQT metric computation.

Performance profiling shows that computing TTI is expensive.

This patch shares the computed TTI between the main EQT metric and
the diagnostic metrics. The performance win is ~10 seconds per story
in v8.infinite_scroll_tbmv2 benchmark.

BUG=chromium:625701
==========

to

==========
Speed up the EQT metric computation.

Performance profiling shows that computing TTI is expensive.

This patch shares the computed TTI between the main EQT metric and
the diagnostic metrics. The performance win is ~10 seconds per story
in v8.infinite_scroll_tbmv2 benchmark.

BUG=chromium:625701

Review-Url: https://codereview.chromium.org/2769643002
Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2017-03-23 09:41:10]

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

[benjhayden, 2017-03-23 20:12:37]

On 2017/03/23 at 09:18:36, ulan wrote:
> On 2017/03/22 22:56:47, benjhayden wrote:
> > Wow, 10s/story! LGTM!
> > 
> > Can you share a little bit about how you measured that? We could add a
section
> > to how-to-write-metrics about profiling metric performance.
> > 
> I recorded a trace of a benchmark run using --output-format=json. Then loaded
the trace in Chrome and used the DevTools CPU profiler on the EQT metric.
> 
> For measuring metric run-time before and after the change I used
./catapult/tracing/bin/run_metric on the same trace.

Thanks! Want to add some text along these lines to how-to-write-metrics.md?

> 
> > Are there any next steps? More low-hanging fruit?
> There is definitely room for optimization in the metrics that I know (V8,
loading, EQT). The lowest-hanging fruit is to avoid iterating all the events
multiple times. This would require optimizations similar to what I did in this
patch. So this is a trade-off between code complexity and performance.
> 
> Is there a project on improving metrics performance? If not, should we start
it? I think the first step would be to track metrics performance in telemetry.

No project, it's just something that I'm keeping an eye on.
We could add some timing code to metric_map_function and add a per-metric
histogram. I'm not sure if it would be worth it.
Maybe we can just pick some low-hanging fruit and wait until running metrics is
a significant portion of the bot cycle time?