Commit 3265a3b0 authored by Peter Marshall's avatar Peter Marshall Committed by Commit Bot

DevTools: Fix self-time calculation when filtering in Bottom-Up tab

Previously we applied the filtering from the user (_textFilter) before
we calculated the self-time for each of the nodes. The way this
calculation works is that each node is assigned the entire time of
its duration, and then child nodes 'steal' self-time from their parent,
and the parent is left with the remaining non-stolen self-time.

If we filter before calculating time, child nodes won't steal self-time
from their parents, and the self-time for the parents will be wrong as
seen in the bug report.

This change includes all nodes (ignoring _textFilter) in the
calculation step and then filters out the nodes from the final data
structure.

Bug: chromium:970825
Change-Id: I99655877e46671c4e2beb944116aa57eab69b8a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1706694Reviewed-by: default avatarAlexei Filippov <alph@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682720}
parent 16f2837c
......@@ -200,7 +200,7 @@ PerformanceTestRunner.walkTimelineEventTreeUnderNode = function(callback, root,
const event = root.event;
if (event)
callback(event, level);
callback(event, level, root);
for (const child of root.children().values())
PerformanceTestRunner.walkTimelineEventTreeUnderNode(callback, child, (level || 0) + 1);
......
......@@ -128,6 +128,22 @@ Timeline.TimelineTreeView = class extends UI.VBox {
return [this._taskFilter, this._textFilter, ...this._model.filters()];
}
/**
* @protected
* @return {!Array<!TimelineModel.TimelineModelFilter>}
*/
filtersWithoutTextFilter() {
return [this._taskFilter, ...this._model.filters()];
}
/**
* @protected
* @return {!Timeline.TimelineFilters.RegExp}
*/
textFilter() {
return this._textFilter;
}
/**
* @return {boolean}
*/
......@@ -977,7 +993,7 @@ Timeline.BottomUpTimelineTreeView = class extends Timeline.AggregatedTimelineTre
*/
_buildTree() {
return new TimelineModel.TimelineProfileTree.BottomUpRootNode(
this._modelEvents(), this.filters(), this._startTime, this._endTime,
this._modelEvents(), this.textFilter(), this.filtersWithoutTextFilter(), this._startTime, this._endTime,
this._groupingFunction(this._groupBySetting.get()));
}
};
......
......@@ -273,16 +273,18 @@ TimelineModel.TimelineProfileTree.TopDownRootNode = class extends TimelineModel.
TimelineModel.TimelineProfileTree.BottomUpRootNode = class extends TimelineModel.TimelineProfileTree.Node {
/**
* @param {!Array<!SDK.TracingModel.Event>} events
* @param {!TimelineModel.TimelineModelFilter} textFilter
* @param {!Array<!TimelineModel.TimelineModelFilter>} filters
* @param {number} startTime
* @param {number} endTime
* @param {?function(!SDK.TracingModel.Event):string} eventGroupIdCallback
*/
constructor(events, filters, startTime, endTime, eventGroupIdCallback) {
constructor(events, textFilter, filters, startTime, endTime, eventGroupIdCallback) {
super('', null);
/** @type {?Map<string, !TimelineModel.TimelineProfileTree.Node>} */
this._children = null;
this._events = events;
this._textFilter = textFilter;
this._filter = e => filters.every(f => f.accept(e));
this._startTime = startTime;
this._endTime = endTime;
......@@ -298,12 +300,26 @@ TimelineModel.TimelineProfileTree.BottomUpRootNode = class extends TimelineModel
return true;
}
/**
* @param {!Map<string, !TimelineModel.TimelineProfileTree.Node>} children
* @return {!Map<string, !TimelineModel.TimelineProfileTree.Node>}
*/
_filterChildren(children) {
for (const [id, child] of children) {
if (child.event && !this._textFilter.accept(child.event))
children.delete(/** @type {string} */ (id));
}
return children;
}
/**
* @override
* @return {!Map<string, !TimelineModel.TimelineProfileTree.Node>}
*/
children() {
return this._children || this._grouppedTopNodes();
if (!this._children)
this._children = this._filterChildren(this._grouppedTopNodes());
return this._children;
}
/**
......@@ -370,10 +386,8 @@ TimelineModel.TimelineProfileTree.BottomUpRootNode = class extends TimelineModel
*/
_grouppedTopNodes() {
const flatNodes = this._ungrouppedTopNodes();
if (!this._eventGroupIdCallback) {
this._children = flatNodes;
if (!this._eventGroupIdCallback)
return flatNodes;
}
const groupNodes = new Map();
for (const node of flatNodes.values()) {
const groupId = this._eventGroupIdCallback(/** @type {!SDK.TracingModel.Event} */ (node.event));
......@@ -385,7 +399,6 @@ TimelineModel.TimelineProfileTree.BottomUpRootNode = class extends TimelineModel
}
groupNode.addChild(node, node.selfTime, node.selfTime);
}
this._children = groupNodes;
return groupNodes;
}
};
......@@ -445,6 +458,7 @@ TimelineModel.TimelineProfileTree.BottomUpNode = class extends TimelineModel.Tim
this.parent = parent;
this._root = root;
this._depth = (parent._depth || 0) + 1;
/** @type {?Map<string, !TimelineModel.TimelineProfileTree.Node>} */
this._cachedChildren = null;
this._hasChildren = hasChildren;
}
......@@ -525,8 +539,8 @@ TimelineModel.TimelineProfileTree.BottomUpNode = class extends TimelineModel.Tim
lastTimeMarker = Math.min(e.endTime, endTime);
}
this._cachedChildren = nodeById;
return nodeById;
this._cachedChildren = this._root._filterChildren(nodeById);
return this._cachedChildren;
}
/**
......
Test filtering in Bottom-Up Timeline Tree View panel.
Initial:
BBB selfTime: 80
AAA selfTime: 80
AAA selfTime: 20
Filtered by 'AAA':
AAA selfTime: 20
Filtered by 'BBB':
BBB selfTime: 80
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
(async function() {
TestRunner.addResult(
`Test filtering in Bottom-Up Timeline Tree View panel.\n`);
await TestRunner.loadModule('performance_test_runner');
await TestRunner.showPanel('timeline');
var sessionId = '4.20';
var mainThread = 1;
var pid = 100;
var testData = [
{
'args': {
'data': {
'sessionId': sessionId,
'frames':
[{'frame': 'frame1', 'url': 'frameurl', 'name': 'frame-name'}]
}
},
'cat': 'disabled-by-default-devtools.timeline',
'name': 'TracingStartedInPage',
'ph': 'I',
'pid': pid,
'tid': mainThread,
'ts': 100,
},
{
'name': 'top level event name',
'ts': 1000000,
'ph': 'B',
'tid': mainThread,
'pid': pid,
'cat': 'toplevel',
'args': {}
},
{
'name': 'TimeStamp',
'ts': 1010000,
'ph': 'B',
'tid': mainThread,
'pid': pid,
'cat': 'disabled-by-default-devtools.timeline',
'args': {'data': {'message': 'AAA'}}
},
{
'name': 'TimeStamp',
'ts': 1020000,
'ph': 'B',
'tid': mainThread,
'pid': pid,
'cat': 'disabled-by-default-devtools.timeline',
'args': {'data': {'message': 'BBB'}}
},
{
'name': 'TimeStamp',
'ts': 1100000,
'ph': 'E',
'tid': mainThread,
'pid': pid,
'cat': 'disabled-by-default-devtools.timeline',
'args': {}
},
{
'name': 'TimeStamp',
'ts': 1110000,
'ph': 'E',
'tid': mainThread,
'pid': pid,
'cat': 'disabled-by-default-devtools.timeline',
'args': {}
},
{
'name': 'top level event name',
'ts': 1120000,
'ph': 'E',
'tid': mainThread,
'pid': pid,
'cat': 'toplevel',
'args': {}
}
];
var model = PerformanceTestRunner.createPerformanceModelWithEvents(testData);
const tabbedPane = UI.panels.timeline._flameChart._detailsView._tabbedPane;
tabbedPane.selectTab(Timeline.TimelineDetailsView.Tab.BottomUp);
const view = tabbedPane.visibleView;
view.setModel(model, PerformanceTestRunner.mainTrack());
view.updateContents(Timeline.TimelineSelection.fromRange(
model.timelineModel().minimumRecordTime(),
model.timelineModel().maximumRecordTime()));
function printEventMessage(event, level, node) {
const text = event.args['data'] && event.args['data']['message'] ?
event.args['data']['message'] + ' selfTime: ' + node.selfTime :
event.name;
TestRunner.addResult(' '.repeat(level) + text);
}
function dumpRecords() {
PerformanceTestRunner.walkTimelineEventTreeUnderNode(
printEventMessage, view._root);
TestRunner.addResult('');
}
TestRunner.addResult('Initial:');
dumpRecords();
TestRunner.addResult(`Filtered by 'AAA':`);
view._textFilterUI.setValue('AAA', true);
dumpRecords();
TestRunner.addResult(`Filtered by 'BBB':`);
view._textFilterUI.setValue('BBB', true);
dumpRecords();
TestRunner.completeTest();
})();
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment