tracing/tracing/metrics/tracing_metric.html - Issue 2658143002: Create a trace import duration metric

Side by Side Diff: tracing/tracing/metrics/tracing_metric.html

Issue 2658143002: Create a trace import duration metric (Closed)

Patch Set: Created 3 years, 10 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

OLD	NEW
1 <!DOCTYPE html>	1 <!DOCTYPE html>

2 <!--	2 <!--

3 Copyright 2016 The Chromium Authors. All rights reserved.	3 Copyright 2016 The Chromium Authors. All rights reserved.

4 Use of this source code is governed by a BSD-style license that can be	4 Use of this source code is governed by a BSD-style license that can be

5 found in the LICENSE file.	5 found in the LICENSE file.

6 -->	6 -->

7	7

8 <link rel="import" href="/tracing/base/iteration_helpers.html">	8 <link rel="import" href="/tracing/base/iteration_helpers.html">

9 <link rel="import" href="/tracing/metrics/metric_registry.html">	9 <link rel="import" href="/tracing/metrics/metric_registry.html">

10 <link rel="import" href="/tracing/value/diagnostics/diagnostic_map.html">	10 <link rel="import" href="/tracing/value/diagnostics/diagnostic_map.html">

(...skipping 74 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
85 memoryInfraTraceBytesValue.addSample(memoryInfraEventsSize);	85 memoryInfraTraceBytesValue.addSample(memoryInfraEventsSize);

86 histograms.addHistogram(memoryInfraTraceBytesValue);	86 histograms.addHistogram(memoryInfraTraceBytesValue);

87	87

88 var traceBytesPerDumpValue = new tr.v.Histogram(	88 var traceBytesPerDumpValue = new tr.v.Histogram(

89 'Average trace size of memory-infra dumps in bytes',	89 'Average trace size of memory-infra dumps in bytes',

90 tr.b.Unit.byName.sizeInBytes_smallerIsBetter, BYTE_BOUNDARIES);	90 tr.b.Unit.byName.sizeInBytes_smallerIsBetter, BYTE_BOUNDARIES);

91 traceBytesPerDumpValue.addSample(memoryInfraEventsSize / memoryDumpCount);	91 traceBytesPerDumpValue.addSample(memoryInfraEventsSize / memoryDumpCount);

92 histograms.addHistogram(traceBytesPerDumpValue);	92 histograms.addHistogram(traceBytesPerDumpValue);

93 }	93 }

94	94

	95 // TODO(charliea): The metrics in this file should be renamed to have names

	96 // more consistent with those in the rest of the codebase

	97 // (e.g. 'trace_size_growth_rate' with a unit of bytes/second, not 'Max event
	benjhayden 2017/02/17 16:48:55 This comment is fine as is, but I wanted to make a This comment is fine as is, but I wanted to make a note that it might require more discussion before fixing: All of the current units are simple: ms, B, W, J. Do we really need to define complex units like bytes/second? Should it be bytes/ms for consistency? Or should ms be changed to seconds? How would conversion work for complex units? Would we need to be able to convert to e.g. TB/year? Do we need to define a calculus of complex Units to express B/s, J/s? Again, no need to change the comment, just thoughts to keep in mind. charliea (OOO until 10-5) 2017/02/17 20:26:04 Hmmm, yea, you're completely right. I hadn't reall Show quoted text On 2017/02/17 16:48:55, benjhayden wrote: > This comment is fine as is, but I wanted to make a note that it might require > more discussion before fixing: > All of the current units are simple: ms, B, W, J. > Do we really need to define complex units like bytes/second? > Should it be bytes/ms for consistency? Or should ms be changed to seconds? > How would conversion work for complex units? Would we need to be able to convert > to e.g. TB/year? > Do we need to define a calculus of complex Units to express B/s, J/s? > > Again, no need to change the comment, just thoughts to keep in mind. Hmmm, yea, you're completely right. I hadn't really given much thought about how this would complicate our existing unit system. And what if the numerator is something that we don't have a unit for, like "garbage collections" if we're measuring GCs/second? It seems like keeping this as "bytes" and just renaming the metric to something like "trace_size_growth_per_second" is a better idea. As for the millisecond/second thing, I think there are definitely a lot of ideas that are more easily expressed in seconds than milliseconds (e.g. power is energy per second, and bytes/second for trace size growth makes more second to me than bytes/millisecond). We can sort that out when we implement it, though.
	98 // size in bytes per second').

	99 // https://github.com/catapult-project/catapult/issues/3233

95 function tracingMetric(histograms, model) {	100 function tracingMetric(histograms, model) {

96 if (!model.stats.hasEventSizesinBytes) {	101 addTimeDurationHistogram(
	charliea (OOO until 10-5) 2017/02/17 16:16:58 As described above, I erred on the side of consist As described above, I erred on the side of consistency with the rest of the codebase with the metric name rather than consistency with the other metrics in this file, which should almost certainly be renamed.
97 throw new Error('Model stats does not have event size information. ' +	102 'trace_import_duration', model.stats.traceImportDurationMs, histograms);

98 'Please enable ImportOptions.trackDetailedModelStats.');	103

99 }	104 if (!model.stats.hasEventSizesinBytes) return;
	charliea (OOO until 10-5) 2017/02/17 16:16:58 This brings the metric more in line with other met This brings the metric more in line with other metrics, which just don't add their histograms if the required data isn't available. Without this change, we could never actually turn on the tracing metric for a benchmark because hasEventSizesInBytes can only be turned on for interactive traces (I think).
100	105

101 var eventStats = model.stats.allTraceEventStatsInTimeIntervals;	106 var eventStats = model.stats.allTraceEventStatsInTimeIntervals;

102 eventStats.sort(function(a, b) {	107 eventStats.sort(function(a, b) {

103 return a.timeInterval - b.timeInterval;	108 return a.timeInterval - b.timeInterval;

104 });	109 });

105	110

106 var totalTraceBytes = eventStats.reduce((a, b) =>	111 var totalTraceBytes = eventStats.reduce((a, b) =>

107 (a + b.totalEventSizeinBytes), 0);	112 (a + b.totalEventSizeinBytes), 0);

108	113

109 // We maintain a sliding window of records [start ... end-1] where end	114 // We maintain a sliding window of records [start ... end-1] where end

(...skipping 85 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
195	200

196 tr.metrics.MetricRegistry.register(tracingMetric);	201 tr.metrics.MetricRegistry.register(tracingMetric);

197	202

198 return {	203 return {

199 tracingMetric,	204 tracingMetric,

200 // For testing only:	205 // For testing only:

201 MEMORY_INFRA_TRACING_CATEGORY,	206 MEMORY_INFRA_TRACING_CATEGORY,

202 };	207 };

203 });	208 });

204 </script>	209 </script>

OLD	NEW

« no previous file with comments | « no previous file | tracing/tracing/metrics/tracing_metric_test.html » ('j') | tracing/tracing/metrics/tracing_metric_test.html » ('J')