Bug 111162

Summary:

Web Inspector: implement Flame Chart for CPU profiler

Product:

WebKit

Reporter:

Ilya Tikhonovsky <loislo>

Component:

Web Inspector (Deprecated)

Assignee:

Ilya Tikhonovsky <loislo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

apavlov, keishi, loislo, pfeldman, pmuellr, vsevik, web-inspector-bugs, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
screenshot	none
Patch	none
Patch	none
Patch	none
Patch	yurys: review+

Ilya Tikhonovsky

Reported 2013-03-01 04:33:26 PST

Flame Chart may give to the developer a better clue what is going on with the performance without expanding the entire tree. http://dtrace.org/blogs/brendan/2011/12/16/flame-graphs/

Attachments
screenshot (17.16 KB, image/png) 2013-03-01 04:35 PST, Ilya Tikhonovsky	no flags	Details
Patch (11.19 KB, patch) 2013-03-01 04:37 PST, Ilya Tikhonovsky	no flags	Details Formatted Diff Diff
Patch (11.16 KB, patch) 2013-03-01 04:49 PST, Ilya Tikhonovsky	no flags	Details Formatted Diff Diff
Patch (17.44 KB, patch) 2013-03-04 04:00 PST, Ilya Tikhonovsky	no flags	Details Formatted Diff Diff
Patch (17.00 KB, patch) 2013-03-04 04:04 PST, Ilya Tikhonovsky	yurys: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Ilya Tikhonovsky

Comment 1 2013-03-01 04:35:00 PST

Created attachment 190933 [details] screenshot

Ilya Tikhonovsky

Comment 2 2013-03-01 04:37:34 PST

Created attachment 190934 [details] Patch

Ilya Tikhonovsky

Comment 3 2013-03-01 04:49:57 PST

Created attachment 190938 [details] Patch

Yury Semikhatsky

Comment 4 2013-03-03 22:56:17 PST

Comment on attachment 190938 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190938&action=review > Source/WebCore/inspector/front-end/CPUProfileView.js:65 > + } else { style: no braces for one-line block. > Source/WebCore/inspector/front-end/CPUProfileView.js:101 > +WebInspector.FlameChart = function(cpuProfileView) Why not move it into its own file? > Source/WebCore/inspector/front-end/CPUProfileView.js:107 > + this._canvas = document.createElement("canvas"); this._canvas = this.element.createChild("canvas"); > Source/WebCore/inspector/front-end/CPUProfileView.js:183 > + draw: function(height, width) It is usually width, heigh (in reverse order). > Source/WebCore/inspector/front-end/CPUProfileView.js:194 > + this._color = initialColor; Consider using WebInspector.Color > Source/WebCore/inspector/front-end/CPUProfileView.js:211 > + _forEach: function(callback) Please rename _forEach -> _forEachNode > Source/WebCore/inspector/front-end/CPUProfileView.js:227 > + nodes = nodes.concat(node.children); This way the nodes will appear in reverse order, won't they? > Source/WebCore/inspector/front-end/CPUProfileView.js:241 > + * @param {!ProfilerAgent.CPUProfileNode} node color annotation is missing, also I'd rename it to rgbColor > Source/WebCore/inspector/front-end/CPUProfileView.js:900 > + Revert this? > Source/WebCore/inspector/front-end/CPUProfileView.js:934 > +//@ sourceURL=http://localhost/inspector/front-end/CPUProfileView.js Remove this.

Ilya Tikhonovsky

Comment 5 2013-03-04 04:00:22 PST

Created attachment 191199 [details] Patch

Ilya Tikhonovsky

Comment 6 2013-03-04 04:02:57 PST

Comment on attachment 190938 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190938&action=review >> Source/WebCore/inspector/front-end/CPUProfileView.js:194 >> + this._color = initialColor; > > Consider using WebInspector.Color done >> Source/WebCore/inspector/front-end/CPUProfileView.js:211 >> + _forEach: function(callback) > > Please rename _forEach -> _forEachNode done >> Source/WebCore/inspector/front-end/CPUProfileView.js:227 >> + nodes = nodes.concat(node.children); > > This way the nodes will appear in reverse order, won't they? It is not a problem because children have no order. >> Source/WebCore/inspector/front-end/CPUProfileView.js:241 >> + * @param {!ProfilerAgent.CPUProfileNode} node > > color annotation is missing, also I'd rename it to rgbColor done: hslColor >> Source/WebCore/inspector/front-end/CPUProfileView.js:900 >> + > > Revert this? done >> Source/WebCore/inspector/front-end/CPUProfileView.js:934 >> +//@ sourceURL=http://localhost/inspector/front-end/CPUProfileView.js > > Remove this. done

Ilya Tikhonovsky

Comment 7 2013-03-04 04:04:40 PST

Created attachment 191200 [details] Patch

Ilya Tikhonovsky

Comment 8 2013-03-04 04:41:57 PST

Committed r144618: <http://trac.webkit.org/changeset/144618>

Note You need to log in before you can comment on or make changes to this bug.