Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(279)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: tracing/tracing/metrics/estimated_input_latency_metric.html

Issue 2323533003: [Not for landing - CL being split] Add Estimated Input Latency - EQT 90th Percentile definition
Patch Set: Review comments + rebase on master Created 4 years, 3 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:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « tracing/tracing/metrics/all_metrics.html ('k') | tracing/tracing/metrics/estimated_input_latency_metric_test.html » ('j') | tracing/tracing/metrics/estimated_input_latency_metric_test.html » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: tracing/tracing/metrics/estimated_input_latency_metric.html
diff --git a/tracing/tracing/metrics/estimated_input_latency_metric.html b/tracing/tracing/metrics/estimated_input_latency_metric.html
new file mode 100644
index 0000000000000000000000000000000000000000..23e2160ed82516eeaae5b60f444fa277e4cf01b0
--- /dev/null
+++ b/tracing/tracing/metrics/estimated_input_latency_metric.html
@@ -0,0 +1,279 @@
+<!DOCTYPE html>
+<!--
+Copyright (c) 2015 The Chromium Authors. All rights reserved.
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: this should be: Copyright 2016 The Chromium
dproy 2016/09/20 01:11:42 Done.
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+-->
+
+<link rel="import" href="/tracing/base/category_util.html">
+<link rel="import" href="/tracing/metrics/metric_registry.html">
+<link rel="import" href="/tracing/metrics/system_health/utils.html">
+<link rel="import" href="/tracing/model/helpers/chrome_model_helper.html">
+<link rel="import" href="/tracing/model/user_model/load_expectation.html">
+<link rel="import" href="/tracing/value/histogram.html">
+
+<script>
+'use strict';
+
+tr.exportTo('tr.metrics', function() {
+
+ var TOPLEVEL_CATEGORY_NAME = 'toplevel';
+
+ // TODO(dproy): Remove after we close #2784
+ function hasCategoryAndName(event, category, title) {
+ return event.title === title && event.category &&
+ tr.b.getCategoryParts(event.category).indexOf(category) !== -1;
charliea (OOO until 10-5) 2016/09/19 14:35:32 nit: line continuations are indented by 4 characte
dproy 2016/09/20 01:11:42 Done.
+ }
+
+ function runConditionallyOnInnermostDescendants(slice, predicate, cb) {
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: what is an innermost descendant? Are these li
dproy 2016/09/20 01:11:43 This function is a little unfortunately named but
benjhayden 2016/09/20 05:52:35 Thanks for the explanation! Would it be possible t
dproy 2016/09/20 15:21:06 One way to get around this would be to just look f
+ var succeededOnSomeDescendant = false;
+ for (var child of slice.subSlices) {
+ var succeededOnThisChild = runConditionallyOnInnermostDescendants(
+ child, predicate, cb);
charliea (OOO until 10-5) 2016/09/19 14:35:33 same (line continuations)
dproy 2016/09/20 01:11:43 Done.
+ succeededOnSomeDescendant =
+ succeededOnThisChild || succeededOnSomeDescendant;
charliea (OOO until 10-5) 2016/09/19 14:35:33 same (line continuations)
dproy 2016/09/20 01:11:43 Done.
+ }
+
+ if (succeededOnSomeDescendant) {
+ return true;
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: no need for braces around a single line branc
dproy 2016/09/20 01:11:43 Does our style guide forbid this? I really dislike
benjhayden 2016/09/20 05:52:35 For what it's worth, Ethan and I also wish that th
+ }
+
+ if (!predicate(slice)) {
+ return false;
charliea (OOO until 10-5) 2016/09/19 14:35:33 same (single line branch)
+ }
+
+ cb(slice);
+ return true;
+ }
+
+ function forEachInnermostTopLevelSlices(thread, cb) {
+ var topLevelPredicate = slice => tr.b.getCategoryParts(slice.category)
charliea (OOO until 10-5) 2016/09/19 14:35:32 nit: I don't think we ever break at the ., but ins
dproy 2016/09/20 01:11:42 Done.
+ .includes(TOPLEVEL_CATEGORY_NAME);
+
+ for (var slice of thread.sliceGroup.topLevelSlices) {
charliea (OOO until 10-5) 2016/09/19 14:35:33 same (no need for braces)
+ runConditionallyOnInnermostDescendants(slice, topLevelPredicate, cb);
+ }
+ }
+
+ function getAllInteractiveTimestampsSorted(model) {
+ // TODO(dproy): When LoadExpectation v.1.0 is released,
+ // update this function to use the new LoadExpectation rather
+ // than calling loading_metric.html.
+
+ var values = new tr.v.ValueSet();
+ tr.metrics.sh.loadingMetric(values, model);
+ var ttiValues = values.getValuesNamed('timeToFirstInteractive');
charliea (OOO until 10-5) 2016/09/19 14:35:32 wrong indentation?
charliea (OOO until 10-5) 2016/09/19 14:35:32 wrong indentation?
dproy 2016/09/20 01:11:42 Done.
+ var interactiveTsList = [];
+ for (var bin of tr.b.getOnlyElement(ttiValues).allBins) {
+ for (var diagnostics of bin.diagnosticMaps) {
+ var info = diagnostics.get('Navigation infos');
+ interactiveTsList.push(info.value.interactive);
+ }
+ }
+ return interactiveTsList.sort((x, y) => x - y);
+ }
+
+ function getAllNavStartTimesSorted(rendererHelper) {
+ var list = [];
+ for (var ev of rendererHelper.mainThread.sliceGroup.childEvents()) {
+ if (!hasCategoryAndName(ev, 'blink.user_timing', 'navigationStart'))
+ continue;
+ list.push(ev.start);
+ }
+ return list.sort((x, y) => x - y);
+ }
+
+ // A task window is defined as time from TTI until either
+ // 1. beginning of next navigationStart event, or
+ // 2. end of traces
+ function getTaskWindows(interactiveTsList, navStartTimeList, endOfTraces) {
+ var curNavStartTimeIndex = 0;
+ var endOfLastWindow = -Infinity;
+ var taskWindows = [];
+ for (var curTTI of interactiveTsList) {
+ var curNavStartTime = navStartTimeList[curNavStartTimeIndex];
+ while (curNavStartTime !== undefined && curNavStartTime < curTTI) {
+ // There are possibly multiple navigationStart timestamps between
+ // two interactive timestamps - the previous page load could
+ // never reach interactive status.
+ curNavStartTimeIndex++;
+ curNavStartTime = navStartTimeList[curNavStartTimeIndex];
+ }
+
+ if (curNavStartTime === endOfLastWindow) {
+ // This is a violation of core assumption.
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: not need for braces
+ // TODO: When two pages share a render process, we can possibly
+ // have two interactive time stamps between two navigation events.
+ // If both interactive timestamps are reported, it is not clear how
+ // to define estimated input latency.
+ throw new Error("Two TTI timestamps with no navigation between them");
+ }
+
+ if (curNavStartTime === undefined) {
+ taskWindows.push({start: curTTI, end: endOfTraces});
+ endOfLastWindow = endOfTraces;
+ continue;
+ }
+
+ taskWindows.push({start: curTTI, end: curNavStartTime});
+ endOfLastWindow = curNavStartTime;
+ }
+ return taskWindows;
+ }
+
+ /**
+ * Note: Taken from
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: the first line of a jsdoc should be what the
dproy 2016/09/20 01:11:42 Done.
+ * https://github.com/GoogleChrome/lighthouse/blob/a5bbe2338fa474c94bb875849408704c81fec3df/lighthouse-core/lib/traces/tracing-processor.js#L121
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: please create bit.ly or goo.gl shortlink and
dproy 2016/09/20 01:11:42 Done.
+ *
+ * Calculate duration at specified percentiles for given population of
charliea (OOO until 10-5) 2016/09/19 14:35:32 I'm not sure I understand what duration we're calc
dproy 2016/09/20 01:11:42 Clarified.
+ * durations.
+ * If one of the durations overlaps the end of the window, the full
charliea (OOO until 10-5) 2016/09/19 14:35:32 nit: if you want different paragraphs within a jsd
dproy 2016/09/20 01:11:42 Done.
+ * duration should be in the duration array, but the length not included
+ * within the window should be given as `clippedLength`. For instance, if a
+ * 50ms duration occurs 10ms before the end of the window, `50` should be in
+ * the `durations` array, and `clippedLength` should be set to 40.
+ * @see https://docs.google.com/document/d/18gvP-CBA2BiBpi3Rz1I1ISciKGhniTSZ9TY0XCnXS7E/preview
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: line is too long. Please create a shortlink t
dproy 2016/09/20 01:11:42 Done.
+ */
+ function calculateEILRiskPercentiles(
+ durations, totalTime, percentiles, clippedLength) {
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: line continuations get four indents
dproy 2016/09/20 01:11:42 Done.
+ clippedLength = clippedLength || 0;
+
+ var busyTime = tr.b.Statistics.sum(durations);
+ busyTime -= clippedLength;
+
+ // Start with idle time already compvare.
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: compvare => compare
benjhayden 2016/09/19 20:00:48 Actually, I think this is supposed to be "complete
dproy 2016/09/20 01:11:42 Yes this was indeed complete. s/let/var got me aga
+ var completedTime = totalTime - busyTime;
+ var duration = 0;
+ var cdfTime = completedTime;
charliea (OOO until 10-5) 2016/09/19 14:35:33 I can kind of vaguely guess what cdfTime is, but I
dproy 2016/09/20 01:11:42 I didn't write this code, and I'm finding it very
+ var results = new Map();
+
+ var durationIndex = -1;
+ var remainingCount = durations.length + 1;
+ if (clippedLength > 0) {
+ // If there was a clipped duration, one less in count
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: no need for braces
+ // since one hasn't started yet.
+ remainingCount--;
+ }
+
+ // Find percentiles of interest, in order.
+ for (var percentile of percentiles) {
+ // Loop over durations, calculating a CDF value for each until it is above
+ // the target percentile.
+ var percentileTime = percentile * totalTime;
+ while (cdfTime < percentileTime && durationIndex < durations.length - 1) {
+ completedTime += duration;
+ remainingCount += (duration >= 0 ? -1 : 1);
+
+ if (clippedLength > 0 && clippedLength < durations[durationIndex + 1]) {
+ duration = -clippedLength;
+ clippedLength = 0;
+ } else {
+ durationIndex++;
+ duration = durations[durationIndex];
+ }
+
+ // Calculate value of CDF (multiplied by totalTime) for the end of
+ // this duration.
+ cdfTime = completedTime + Math.abs(duration) * remainingCount;
+ }
+
+ // Negative results are within idle time (0ms wait by definition),
+ // so clamp at zero.
+ results.set(
+ percentile,
charliea (OOO until 10-5) 2016/09/19 14:35:32 nit: line continuations get four indents
dproy 2016/09/20 01:11:42 Done.
+ Math.max(0, (percentileTime - completedTime) / remainingCount));
+ }
+
+ return results;
+ }
+
+ /**
+ * Note: This is adapted from
+ * https://github.com/GoogleChrome/lighthouse/blob/a5bbe2338fa474c94bb875849408704c81fec3df/lighthouse-core/lib/traces/tracing-processor.js#L185
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: shortlink.
dproy 2016/09/20 01:11:42 Done.
+ */
+ function getEQTPercentilesForWindow(
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: what the heck is an EQT?
dproy 2016/09/20 01:11:42 Added comments
+ percentiles, rendererHelper, startTime, endTime) {
+ var totalTime = endTime - startTime;
+ percentiles.sort((a, b) => a - b);
+
+ var durations = [];
+ var clippedLength = 0;
+ forEachInnermostTopLevelSlices(rendererHelper.mainThread, slice => {
+ // Discard slices outside range.
+ if (slice.end <= startTime || slice.start >= endTime) {
+ return;
+ }
+
+ // Clip any at edges of range.
+ var duration = slice.duration;
+ var sliceStart = slice.start;
+ if (sliceStart < startTime) {
+ // Any part of task before window can be discarded.
+ sliceStart = startTime;
+ duration = slice.end - sliceStart;
+
+ }
+ if (slice.end > endTime) {
+ // Any part of task after window must be clipped but accounted for.
+ clippedLength = duration - (endTime - sliceStart);
+ }
+ durations.push(duration);
+
+ });
+ durations.sort((a, b) => a - b);
+ return calculateEILRiskPercentiles(
+ durations, totalTime, percentiles, clippedLength);
+ }
+
+ /**
+ * @param {!tr.v.ValueSet} values
+ * @param {!tr.model.Model} model
+ * @param {!Object=} opt_options
charliea (OOO until 10-5) 2016/09/19 14:35:32 what type of options are available? This might als
benjhayden 2016/09/19 20:00:48 This metric doesn't support any opt_options, so I
dproy 2016/09/20 01:11:42 Let me know if I should remove opt_options altoget
+ */
+ function estimatedInputLatencyMetric(values, model, opt_options) {
+
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: unnecessary blank line
dproy 2016/09/20 01:11:43 Done.
+ var chromeHelper = model.getOrCreateHelper(
+ tr.model.helpers.ChromeModelHelper);
+ // The largest pid renderer is probably the one we're interested in
+ var rendererHelper = chromeHelper.rendererWithLargestPid;
+
+ var interactiveTimestamps = getAllInteractiveTimestampsSorted(model);
+
+ // We're assuming children iframes will never emit navigationStart events
+ var navStartTimeList = getAllNavStartTimesSorted(rendererHelper);
+ var taskWindowList = getTaskWindows(
+ interactiveTimestamps,
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: line continuations
dproy 2016/09/20 01:11:43 Done.
+ navStartTimeList,
+ rendererHelper.mainThread.bounds.max);
+
+ // Compute the 90th percentile
+ var eqtTargetPercentiles = [0.9];
+
+ var eqt90thPercentile = new tr.v.Histogram(
+ 'Expected Queueing Time 90th Percentile',
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: line continuations
dproy 2016/09/20 01:11:42 Done.
+ tr.b.Unit.byName.timeDurationInMs_smallerIsBetter,
+ tr.v.HistogramBinBoundaries.createExponential(0.1, 1e3, 100));
charliea (OOO until 10-5) 2016/09/19 14:35:33 nit: I think the best practice is to create these
dproy 2016/09/20 01:11:43 Done: made these top level constants with some com
+
+ for (var taskWindow of taskWindowList) {
+ var eqtPercentiles = getEQTPercentilesForWindow(
+ eqtTargetPercentiles, rendererHelper, taskWindow.start, taskWindow.end);
charliea (OOO until 10-5) 2016/09/19 14:35:32 nit: line continuations
dproy 2016/09/20 01:11:43 Done.
+ var eqt90th = eqtPercentiles.get(0.9);
+ if (eqt90th !== undefined) {
+ eqt90thPercentile.addSample(eqt90th);
charliea (OOO until 10-5) 2016/09/19 14:35:32 nit: single line branch
+ }
+ }
+
+ values.addHistogram(eqt90thPercentile);
+ }
+
+ tr.metrics.MetricRegistry.register(estimatedInputLatencyMetric, {
+ supportsRangeOfInterest: false
+ });
+
+ return {
+ estimatedInputLatencyMetric: estimatedInputLatencyMetric,
+ calculateEILRiskPercentiles: calculateEILRiskPercentiles,
+ getEQTPercentilesForWindow: getEQTPercentilesForWindow
+ };
+});
+</script>
« no previous file with comments | « tracing/tracing/metrics/all_metrics.html ('k') | tracing/tracing/metrics/estimated_input_latency_metric_test.html » ('j') | tracing/tracing/metrics/estimated_input_latency_metric_test.html » ('J')

Powered by Google App Engine
This is Rietveld 408576698