Closed Bug 785922 Opened 8 years ago Closed 2 years ago

Profiler: Support column info

Categories

(Core :: Gecko Profiler, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: BenWa, Assigned: denispal)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

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.
Attached patch WIP (obsolete) — Splinter Review
Alex gave me his WIP over IRC
Blocks: 761253
Attachment #655627 - Attachment is obsolete: true
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.
Assignee: nobody → dpalmeiro
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.
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 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.
Attachment #8998985 - Flags: review+
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.
Attachment #8998985 - Flags: review+
Attachment #8954574 - Attachment is obsolete: true
Attachment #8954576 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed by rvandermeulen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bb09f2ec0d28
Emit column numbers for JS frames and functions in the gecko profiler r=sfink,mstange
Keywords: checkin-needed
(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.
Flags: needinfo?(dpalmeiro)
Keywords: checkin-needed
Pushed by dvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ef6a3b493405
Emit column numbers for JS frames and functions in the gecko profiler r=sfink,mstange
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ef6a3b493405
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.