Bug 1176537 - Change how markers are collapsed to prevent issues for folding in deeply nested markers, and actually fix the other waterfall collapsing tests to actually run. r=vp

This commit is contained in:
Jordan Santell 2015-07-12 16:41:22 -07:00
parent 30827b6f14
commit f00f6823d1
8 changed files with 172 additions and 185 deletions

View File

@ -27,6 +27,10 @@ const GECKO_SYMBOL = "(Gecko)";
* determines if this marker should be filtered or not.
*/
function isMarkerValid (marker, filter) {
if (!filter || filter.length === 0) {
return true;
}
let isUnknown = !(marker.name in TIMELINE_BLUEPRINT);
if (isUnknown) {
return filter.indexOf("UNKNOWN") === -1;
@ -292,80 +296,6 @@ const DOM = {
}
};
/**
* A series of collapsers used by the blueprint. These functions are
* invoked on a moving window of two markers.
*
* A function determining how markers are collapsed together.
* Invoked with 3 arguments: the current parent marker, the
* current marker and a method for peeking i markers ahead. If
* nothing is returned, the marker is added as a standalone entry
* in the waterfall. Otherwise, an object needs to be returned
* with the following properties:
* - toParent: The marker to be made a new parent. Can use the current
* marker, becoming a parent itself, or make a new marker-esque
* object.
* - collapse: Whether or not this current marker should be nested within
* the current parent.
* - finalize: Whether or not the current parent should be finalized and popped
* off the stack.
*/
const CollapseFunctions = {
/**
* Combines similar markers that are consecutive into a meta marker.
*/
identical: function (parent, curr, peek) {
let next = peek(1);
// If there is a parent marker currently being filled and the current marker
// should go into the parent marker, make it so.
if (parent && parent.name == curr.name) {
let finalize = next && next.name !== curr.name;
return { collapse: true, finalize };
}
// Otherwise if the current marker is the same type as the next marker type,
// create a new parent marker containing the current marker.
if (next && curr.name == next.name) {
return { toParent: { name: curr.name, start: curr.start }, collapse: true };
}
},
/**
* Combines similar markers that are close to each other in time into a meta marker.
*/
adjacent: function (parent, curr, peek) {
let next = peek(1);
if (next && (next.start < curr.end || next.start - curr.end <= 10 /* ms */)) {
return CollapseFunctions.identical(parent, curr, peek);
}
},
/**
* Folds this marker in parent marker if parent marker fully eclipses
* the current markers' time.
*/
child: function (parent, curr, peek) {
let next = peek(1);
// If this marker is consumed by current parent, collapse
if (parent && curr.end <= parent.end) {
let finalize = next && next.end > parent.end;
return { collapse: true, finalize };
}
},
/**
* Turns this marker into a parent marker if the next marker
* is fully eclipsed by the current marker.
*/
parent: function (parent, curr, peek) {
let next = peek(1);
// If the next marker is fully consumed by this marker, make
// it a parent (do not collapse, the marker becomes a parent).
if (next && curr.end >= next.end) {
return { toParent: curr };
}
},
};
/**
* Mapping of JS marker causes to a friendlier form. Only
* markers that are considered "from content" should be labeled here.
@ -480,6 +410,5 @@ exports.getMarkerLabel = getMarkerLabel;
exports.getMarkerClassName = getMarkerClassName;
exports.getMarkerFields = getMarkerFields;
exports.DOM = DOM;
exports.CollapseFunctions = CollapseFunctions;
exports.Formatters = Formatters;
exports.getBlueprintFor = getBlueprintFor;

View File

@ -7,17 +7,32 @@
* Utility functions for collapsing markers into a waterfall.
*/
loader.lazyRequireGetter(this, "extend",
"sdk/util/object", true);
loader.lazyRequireGetter(this, "MarkerUtils",
"devtools/performance/marker-utils");
/**
* Creates a parent marker, which functions like a regular marker,
* but is able to hold additional child markers.
*
* The marker is seeded with values from `marker`.
* @param object marker
* @return object
*/
function createParentNode (marker) {
return extend(marker, { submarkers: [] });
}
/**
* Collapses markers into a tree-like structure.
* @param object markerNode
* @param object rootNode
* @param array markersList
* @param array filter
*/
function collapseMarkersIntoNode({ markerNode, markersList, filter }) {
let { getCurrentParentNode, collapseMarker, addParentNode, popParentNode } = createParentNodeFactory(markerNode);
function collapseMarkersIntoNode({ rootNode, markersList, filter }) {
let { getCurrentParentNode, pushNode, popParentNode } = createParentNodeFactory(rootNode);
for (let i = 0, len = markersList.length; i < len; i++) {
let curr = markersList[i];
@ -29,50 +44,54 @@ function collapseMarkersIntoNode({ markerNode, markersList, filter }) {
let parentNode = getCurrentParentNode();
let blueprint = MarkerUtils.getBlueprintFor(curr);
let collapse = blueprint.collapseFunc || (() => null);
let peek = distance => markersList[i + distance];
let collapseInfo = collapse(parentNode, curr, peek);
if (collapseInfo) {
let { collapse, toParent, finalize } = collapseInfo;
let nestable = "nestable" in blueprint ? blueprint.nestable : true;
let collapsible = "collapsible" in blueprint ? blueprint.collapsible : true;
// If `toParent` is an object, use it as the next parent marker
if (typeof toParent === "object") {
addParentNode(toParent);
let finalized = null;
// If this marker is collapsible, turn it into a parent marker.
// If there are no children within it later, it will be turned
// back into a normal node.
if (collapsible) {
curr = createParentNode(curr);
}
// If not nestible, just push it inside the root node,
// like console.time/timeEnd.
if (!nestable) {
pushNode(rootNode, curr);
continue;
}
// First off, if any parent nodes exist, finish them off
// recursively upwards if this marker is outside their ranges and nestable.
while (!finalized && parentNode) {
// If this marker is eclipsed by the current parent marker,
// make it a child of the current parent and stop
// going upwards.
if (nestable && curr.end <= parentNode.end) {
pushNode(parentNode, curr);
finalized = true;
break;
}
if (collapse) {
collapseMarker(curr);
}
// If the marker specifies this parent marker is full,
// pop it from the stack.
if (finalize) {
// If this marker is still nestable, but outside of the range
// of the current parent, iterate upwards on the next parent
// and finalize the current parent.
if (nestable) {
popParentNode();
parentNode = getCurrentParentNode();
continue;
}
} else {
markerNode.submarkers.push(curr);
}
if (!finalized) {
pushNode(rootNode, curr);
}
}
}
/**
* Creates a parent marker, which functions like a regular marker,
* but is able to hold additional child markers.
*
* The marker is seeded with values from `marker`.
* @param object marker
* @return object
*/
function makeParentMarkerNode (marker) {
let node = Object.create(null);
for (let prop in marker) {
node[prop] = marker[prop];
}
node.submarkers = [];
return node;
}
/**
* Takes a root marker node and creates a hash of functions used
* to manage the creation and nesting of additional parent markers.
@ -98,6 +117,14 @@ function createParentNodeFactory (root) {
if (lastParent.end == void 0) {
lastParent.end = lastParent.submarkers[lastParent.submarkers.length - 1].end;
}
// If no children were ever pushed into this parent node,
// remove it's submarkers so it behaves like a non collapsible
// node.
if (!lastParent.submarkers.length) {
delete lastParent.submarkers;
}
return lastParent;
},
@ -106,29 +133,22 @@ function createParentNodeFactory (root) {
*/
getCurrentParentNode: () => parentMarkers.length ? parentMarkers[parentMarkers.length - 1] : null,
/**
* Push a new parent node onto the stack and nest it with the
* next most recent parent node, or root if no other parent nodes.
*/
addParentNode: (marker) => {
let parentMarker = makeParentMarkerNode(marker);
(factory.getCurrentParentNode() || root).submarkers.push(parentMarker);
parentMarkers.push(parentMarker);
},
/**
* Push this marker into the most recent parent node.
*/
collapseMarker: (marker) => {
if (parentMarkers.length === 0) {
throw new Error("Cannot collapse marker with no parents.");
pushNode: (parent, marker) => {
parent.submarkers.push(marker);
// If pushing a parent marker, track it as the top of
// the parent stack.
if (marker.submarkers) {
parentMarkers.push(marker);
}
factory.getCurrentParentNode().submarkers.push(marker);
}
};
return factory;
}
exports.makeParentMarkerNode = makeParentMarkerNode;
exports.createParentNode = createParentNode;
exports.collapseMarkersIntoNode = collapseMarkersIntoNode;

View File

@ -4,7 +4,7 @@
"use strict";
const { L10N } = require("devtools/performance/global");
const { Formatters, CollapseFunctions: collapse } = require("devtools/performance/marker-utils");
const { Formatters } = require("devtools/performance/marker-utils");
/**
* A simple schema for mapping markers to the timeline UI. The keys correspond
@ -23,19 +23,11 @@ const { Formatters, CollapseFunctions: collapse } = require("devtools/performanc
* for `.marker-details-bullet.{COLORNAME}` for the equivilent
* entry in ./browser/themes/shared/devtools/performance.inc.css
* https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors
* - collapseFunc: A function determining how markers are collapsed together.
* Invoked with 3 arguments: the current parent marker, the
* current marker and a method for peeking i markers ahead. If
* nothing is returned, the marker is added as a standalone entry
* in the waterfall. Otherwise, an object needs to be returned
* with the following properties:
* - toParent: The marker to be made a new parent. Can use the current
* marker, becoming a parent itself, or make a new marker-esque
* object.
* - collapse: Whether or not this current marker should be nested within
* the current parent.
* - finalize: Whether or not the current parent should be finalized and popped
* off the stack.
* - collapsible: Whether or not this marker can contain other markers it
* eclipses, and becomes collapsible to reveal its nestable children.
* Defaults to true.
* - nestable: Whether or not this marker can be nested inside an eclipsing
* collapsible marker. Defaults to true.
* - fields: An optional array of marker properties you wish to display in the
* marker details view. For example, a field in the array such as
* { property: "aCauseName", label: "Cause" } would render a string
@ -61,28 +53,24 @@ const TIMELINE_BLUEPRINT = {
"UNKNOWN": {
group: 2,
colorName: "graphs-grey",
collapseFunc: collapse.child,
label: Formatters.UnknownLabel
label: Formatters.UnknownLabel,
},
/* Group 0 - Reflow and Rendering pipeline */
"Styles": {
group: 0,
colorName: "graphs-purple",
collapseFunc: collapse.child,
label: L10N.getStr("timeline.label.styles2"),
fields: Formatters.StylesFields,
},
"Reflow": {
group: 0,
colorName: "graphs-purple",
collapseFunc: collapse.child,
label: L10N.getStr("timeline.label.reflow2"),
},
"Paint": {
group: 0,
colorName: "graphs-green",
collapseFunc: collapse.child,
label: L10N.getStr("timeline.label.paint"),
},
@ -90,33 +78,28 @@ const TIMELINE_BLUEPRINT = {
"DOMEvent": {
group: 1,
colorName: "graphs-yellow",
collapseFunc: collapse.parent,
label: L10N.getStr("timeline.label.domevent"),
fields: Formatters.DOMEventFields,
},
"Javascript": {
group: 1,
colorName: "graphs-yellow",
collapseFunc: either(collapse.parent, collapse.child),
label: Formatters.JSLabel,
fields: Formatters.JSFields
},
"Parse HTML": {
group: 1,
colorName: "graphs-yellow",
collapseFunc: either(collapse.parent, collapse.child),
label: L10N.getStr("timeline.label.parseHTML"),
},
"Parse XML": {
group: 1,
colorName: "graphs-yellow",
collapseFunc: either(collapse.parent, collapse.child),
label: L10N.getStr("timeline.label.parseXML"),
},
"GarbageCollection": {
group: 1,
colorName: "graphs-red",
collapseFunc: either(collapse.parent, collapse.child),
label: Formatters.GCLabel,
fields: [
{ property: "causeName", label: "Reason:" },
@ -126,14 +109,12 @@ const TIMELINE_BLUEPRINT = {
"nsCycleCollector::Collect": {
group: 1,
colorName: "graphs-red",
collapseFunc: either(collapse.parent, collapse.child),
label: "Cycle Collection",
fields: Formatters.CycleCollectionFields,
},
"nsCycleCollector::ForgetSkippable": {
group: 1,
colorName: "graphs-red",
collapseFunc: either(collapse.parent, collapse.child),
label: "Cycle Collection",
fields: Formatters.CycleCollectionFields,
},
@ -147,34 +128,21 @@ const TIMELINE_BLUEPRINT = {
property: "causeName",
label: L10N.getStr("timeline.markerDetail.consoleTimerName")
}],
nestable: false,
collapsible: false,
},
"TimeStamp": {
group: 2,
colorName: "graphs-blue",
collapseFunc: collapse.child,
label: sublabelForProperty(L10N.getStr("timeline.label.timestamp"), "causeName"),
fields: [{
property: "causeName",
label: "Label:"
}],
collapsible: false,
},
};
/**
* Helper for creating a function that returns the first defined result from
* a list of functions passed in as params, in order.
* @param ...function fun
* @return any
*/
function either(...fun) {
return function() {
for (let f of fun) {
let result = f.apply(null, arguments);
if (result !== undefined) return result;
}
}
}
/**
* Takes a main label (like "Timestamp") and a property,
* and returns a marker that will print out the property

View File

@ -5,13 +5,17 @@
* Tests if the waterfall collapsing logic works properly.
*/
function test() {
function run_test() {
run_next_test();
}
add_task(function test() {
const WaterfallUtils = devtools.require("devtools/performance/waterfall-utils");
let rootMarkerNode = WaterfallUtils.makeParentMarkerNode({ name: "(root)" });
let rootMarkerNode = WaterfallUtils.createParentNode({ name: "(root)" });
WaterfallUtils.collapseMarkersIntoNode({
markerNode: rootMarkerNode,
rootNode: rootMarkerNode,
markersList: gTestMarkers
});
@ -22,14 +26,13 @@ function test() {
compare(marker.submarkers[i], expected.submarkers[i]);
}
} else if (prop !== "uid") {
is(marker[prop], expected[prop], `${expected.name} matches ${prop}`);
equal(marker[prop], expected[prop], `${expected.name} matches ${prop}`);
}
}
}
compare(rootMarkerNode, gExpectedOutput);
finish();
}
});
const gTestMarkers = [
{ start: 1, end: 18, name: "DOMEvent" },
@ -44,7 +47,7 @@ const gTestMarkers = [
{ start: 12, end: 13, name: "Parse XML" },
{ start: 14, end: 15, name: "GarbageCollection" },
// Test that JS markers can be parents without being a child of DOM events
{ start: 25, end: 30, name: "JavaScript" },
{ start: 25, end: 30, name: "Javascript" },
{ start: 26, end: 27, name: "Paint" },
];
@ -61,7 +64,7 @@ const gExpectedOutput = {
{ start: 14, end: 15, name: "GarbageCollection" },
]}
]},
{ start: 25, end: 30, name: "JavaScript", submarkers: [
{ start: 25, end: 30, name: "Javascript", submarkers: [
{ start: 26, end: 27, name: "Paint" },
]}
]};

View File

@ -6,13 +6,17 @@
* markers, as they should ignore any sort of collapsing.
*/
function test() {
function run_test() {
run_next_test();
}
add_task(function test() {
const WaterfallUtils = devtools.require("devtools/performance/waterfall-utils");
let rootMarkerNode = WaterfallUtils.makeParentMarkerNode({ name: "(root)" });
let rootMarkerNode = WaterfallUtils.createParentNode({ name: "(root)" });
WaterfallUtils.collapseMarkersIntoNode({
markerNode: rootMarkerNode,
rootNode: rootMarkerNode,
markersList: gTestMarkers
});
@ -23,14 +27,13 @@ function test() {
compare(marker.submarkers[i], expected.submarkers[i]);
}
} else if (prop !== "uid") {
is(marker[prop], expected[prop], `${expected.name} matches ${prop}`);
equal(marker[prop], expected[prop], `${expected.name} matches ${prop}`);
}
}
}
compare(rootMarkerNode, gExpectedOutput);
finish();
}
});
const gTestMarkers = [
{ start: 2, end: 9, name: "Javascript" },

View File

@ -0,0 +1,63 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
/**
* Tests that the waterfall collapsing works when atleast two
* collapsible markers downward, and the following marker is outside of both ranges.
*/
function run_test() {
run_next_test();
}
add_task(function test() {
const WaterfallUtils = devtools.require("devtools/performance/waterfall-utils");
let rootMarkerNode = WaterfallUtils.createParentNode({ name: "(root)" });
WaterfallUtils.collapseMarkersIntoNode({
rootNode: rootMarkerNode,
markersList: gTestMarkers
});
function compare (marker, expected) {
for (let prop in expected) {
if (prop === "submarkers") {
for (let i = 0; i < expected.submarkers.length; i++) {
compare(marker.submarkers[i], expected.submarkers[i]);
}
} else if (prop !== "uid") {
equal(marker[prop], expected[prop], `${expected.name} matches ${prop}`);
}
}
}
compare(rootMarkerNode, gExpectedOutput);
});
const gTestMarkers = [
{ start: 2, end: 10, name: "DOMEvent" },
{ start: 3, end: 9, name: "Javascript" },
{ start: 4, end: 8, name: "GarbageCollection" },
{ start: 11, end: 12, name: "Styles" },
{ start: 13, end: 14, name: "Styles" },
{ start: 15, end: 25, name: "DOMEvent" },
{ start: 17, end: 24, name: "Javascript" },
{ start: 18, end: 19, name: "GarbageCollection" },
];
const gExpectedOutput = {
name: "(root)", submarkers: [
{ start: 2, end: 10, name: "DOMEvent", submarkers: [
{ start: 3, end: 9, name: "Javascript", submarkers: [
{ start: 4, end: 8, name: "GarbageCollection" }
]}
]},
{ start: 11, end: 12, name: "Styles" },
{ start: 13, end: 14, name: "Styles" },
{ start: 15, end: 25, name: "DOMEvent", submarkers: [
{ start: 17, end: 24, name: "Javascript", submarkers: [
{ start: 18, end: 19, name: "GarbageCollection" }
]}
]},
]};

View File

@ -24,3 +24,4 @@ skip-if = toolkit == 'android' || toolkit == 'gonk'
[test_tree-model-09.js]
[test_waterfall-utils-collapse-01.js]
[test_waterfall-utils-collapse-02.js]
[test_waterfall-utils-collapse-03.js]

View File

@ -136,10 +136,10 @@ let WaterfallView = Heritage.extend(DetailsSubview, {
return cached;
}
let rootMarkerNode = WaterfallUtils.makeParentMarkerNode({ name: "(root)" });
let rootMarkerNode = WaterfallUtils.createParentNode({ name: "(root)" });
WaterfallUtils.collapseMarkersIntoNode({
markerNode: rootMarkerNode,
rootNode: rootMarkerNode,
markersList: markers,
filter: this._hiddenMarkers
});