785922 - Profiler: Support column info

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Benoit Girard (:BenWa)]

Bug 568142 added column info support to the JS engine. It isn't hard now to add column info the profiling which will let us better support minified scripts.

Comment 1

[Benoit Girard (:BenWa)]

Attachment: WIP

Alex gave me his WIP over IRC

Comment 2

[Markus Stange [:mstange]]

Attachment: Bug 785922 - Add column information for JS frames to the profile.

Review commit: https://reviewboard.mozilla.org/r/223630/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223630/

Comment 3

[Markus Stange [:mstange]]

Attachment: Bug 785922 - Add column information for JS functions to the profile.

These are the strings that are used to describe a function; the line and column
numbers describe where the function *starts*.
I don't know why we have basically the same code in two places, one instance for
the interpreter and one for JIT frames.

Review commit: https://reviewboard.mozilla.org/r/223632/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/223632/

Comment 4

[Markus Stange [:mstange]]

The second patch changes the format for JS function names from "functionName (fileName:lineNo)" to "functionName (fileName:lineNo:column)". We will need to update code in the devtools performance panel and in perf.html to be able to parse the new string format.

Comment 5

[Denis Palmeiro [:denispal]]

Most of the changes in the patches are still relevant and work once the frontend is updated.  The only extra change I had to add was a new column property in the frameTable schema.

Comment 6

[Denis Palmeiro [:denispal]]

Attachment: bug 785922: Emit column numbers for JS frames and functions in the gecko profiler

Add support for column numbers when profiling JS stack frames and functions.  This will help debug minified scripts when inspecting performance profiles.  The column information will be emitted as a new column property that is part of the frameTable in the profile output, and will also be appended in the descriptive profiler string.

Comment 7

[Markus Stange [:mstange]]

Comment on attachment 8998985 [details]
bug 785922: Emit column numbers for JS frames and functions in the gecko profiler

Markus Stange [:mstange] has approved the revision.

Comment 8

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8998985 [details]
bug 785922: Emit column numbers for JS frames and functions in the gecko profiler

Steve Fink [:sfink] [:s:] has approved the revision.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bb09f2ec0d28
Emit column numbers for JS frames and functions in the gecko profiler r=sfink,mstange

Comment 10

[Arthur Iakab [arthur_iakab]]

Backed out changeset bb09f2ec0d28 (bug 785922)for causing Android build bustages on profiler/core/platform.cpp CLOSED TREE

Backout revision: https://hg.mozilla.org/integration/autoland/rev/51881a2e2f9e1407f91ac39609685894c51fe729

Failed push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=bb09f2ec0d284f79b4a49df1db8b455c53430be4&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified

Link to the failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=194551962&repo=autoland

Comment 11

[Denis Palmeiro [:denispal]]

(In reply to Arthur Iakab [arthur_iakab] from comment #10)
> Backed out changeset bb09f2ec0d28 (bug 785922)for causing Android build
> bustages on profiler/core/platform.cpp CLOSED TREE
> 
> Backout revision:
> https://hg.mozilla.org/integration/autoland/rev/
> 51881a2e2f9e1407f91ac39609685894c51fe729
> 
> Failed push:
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=bb09f2ec0d284f79b4a49df1db8b455c53430be4&filter-
> resultStatus=testfailed&filter-resultStatus=busted&filter-
> resultStatus=exception&filter-classifiedState=unclassified
> 
> Link to the failure log:
> https://treeherder.mozilla.org/logviewer.html#?job_id=194551962&repo=autoland

Fixed a reference that needed updating which was only taken in the android path.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ef6a3b493405
Emit column numbers for JS frames and functions in the gecko profiler r=sfink,mstange

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/ef6a3b493405