Bug 37820 - Web Inspector: Aggregated time stats should be visible in Timeline record Popup
Summary: Web Inspector: Aggregated time stats should be visible in Timeline record Popup
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-19 13:59 PDT by Ilya Tikhonovsky
Modified: 2010-04-26 06:39 PDT (History)
3 users (show)

See Also:

Attachments
screenshot of Popup with aggregated time info (27.35 KB, image/png)
2010-04-19 13:59 PDT, Ilya Tikhonovsky 		no flags Details
[patch] initial version. (7.10 KB, patch)
2010-04-19 14:15 PDT, Ilya Tikhonovsky 		no flags Details | Formatted Diff | Diff
screenshot of Popup with aggregated time info and with self|cpu|total bars (14.15 KB, image/png)
2010-04-20 08:00 PDT, Ilya Tikhonovsky 		no flags Details
[patch] second iteration. (14.08 KB, patch)
2010-04-20 14:21 PDT, Ilya Tikhonovsky 		loislo: commit-queue-
Details | Formatted Diff | Diff
[patch] Second iteration. (16.45 KB, patch)
2010-04-23 08:11 PDT, Ilya Tikhonovsky 		pfeldman: review+
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Tikhonovsky 2010-04-19 13:59:07 PDT 
Created attachment 53716 [details]
screenshot of Popup with aggregated time info

%SUBJ%
Comment 1 Ilya Tikhonovsky 2010-04-19 14:15:02 PDT 
Created attachment 53721 [details]
[patch] initial version.
Comment 2 Pavel Feldman 2010-04-19 14:41:13 PDT 
Comment on attachment 53721 [details]
[patch] initial version.

I don't think that popover is informative enough. You might want to do a little pie chart or use legend with corresponding colors instead.
Comment 3 Ilya Tikhonovsky 2010-04-20 08:00:57 PDT 
Created attachment 53819 [details]
screenshot of Popup with aggregated time info and with self|cpu|total bars
Comment 4 Timothy Hatcher 2010-04-20 11:33:08 PDT 
I really like the second version.
Comment 5 Timothy Hatcher 2010-04-20 11:33:29 PDT 
Maybe we should hide anything with 0?
Comment 6 Ilya Tikhonovsky 2010-04-20 14:21:17 PDT 
Created attachment 53880 [details]
[patch] second iteration.

has binary changes.
1) Legend boxes was added to the Aggregated Time row data in Popup;
2) three bars with different opacity are used for one record. Solid is for Self Time, with opacity 0.6 for aggregated cpu time of the record and it's children and with opacity 0.2 for duration; 
3) Duration property is changed. Now it is time between startTime and lastChildEndTime (was time between start and end of real event);
4) Title of popup is fixed.
Comment 7 Ilya Tikhonovsky 2010-04-23 08:11:44 PDT 
Created attachment 54160 [details]
[patch] Second iteration.

pushGCEvents crash was fixed.
aggregated stats propagation to the parents was fixed.
incorrect calculation of visible records count was fixed.
Comment 8 Pavel Feldman 2010-04-23 08:24:02 PDT 
Comment on attachment 54160 [details]
[patch] Second iteration.

r+ with comments. please address them prior to landing.

WebCore/inspector/InspectorTimelineAgent.cpp:59
 +  	if (m_gcEvents.size()) {
Replace with guard expression?

WebCore/inspector/front-end/TimelinePanel.js:1005
 +      _calculateAggregatedStats: function(categories)
It would be great to remove the 0.00001 hack and to do as much of this as possible lazily (i.e. only calculate self time on record add).
Comment 9 Ilya Tikhonovsky 2010-04-26 06:39:31 PDT 
Committed r58242: <http://trac.webkit.org/changeset/58242>