Closed Bug 790117 Opened 7 years ago Closed 7 years ago

JavaScript functions to control external profilers should be in js/src/builtin

Categories

(Core :: JavaScript Engine, defect, minor)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: jimb, Assigned: jimb)

Details

Attachments

(2 files, 1 obsolete file)

When configured correctly, the JavaScript shell includes a number of functions to control external profilers like valgrind, perf, Shark, and VTune. These are currently in js/src/jsdbgapi.cpp, but it seems like they would fit better in their own file in js/src/builtin, perhaps Profilers.cpp.
Build-only try push (I expect failures): 
https://tbpl.mozilla.org/?tree=Try&rev=3af6947410f9
Assignee: general → jimb
Status: NEW → ASSIGNED
Those Mac failures are unfiled, and persistent, but I don't believe they're really caused by this patch. Trying again with a fresh M-C pull before I dig in.
https://tbpl.mozilla.org/?tree=Try&rev=6190219fd52d
They're green.
These functions are entirely unused, but we also now have
JS_DefineProfilingFunctions, present in both browser and JavaScript shell,
for doing what these functions were presumably intended for.
Attachment #662248 - Flags: review?(benjamin)
The VTune support needed to be updated for changes to the VTune control API; that work is bug 675098.
Attachment #659919 - Attachment is obsolete: true
Attachment #662250 - Flags: review?(sphink)
Comment on attachment 662250 [details] [diff] [review]
Move external profiler control functions to their own source file in js/src/builtin; drop VTune.

Review of attachment 662250 [details] [diff] [review]:
-----------------------------------------------------------------

The code is a little blecherous, but I'm happy to have it hidden under a shinier rock.
Attachment #662250 - Flags: review?(sphink) → review+
Attachment #662248 - Flags: review?(benjamin) → review+
You need to log in before you can comment on or make changes to this bug.