Closed
Bug 790117
Opened 13 years ago
Closed 13 years ago
JavaScript functions to control external profilers should be in js/src/builtin
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: jimb, Assigned: jimb)
Details
Attachments
(2 files, 1 obsolete file)
|
2.17 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
|
36.16 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
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.
| Assignee | ||
Comment 1•13 years ago
|
||
Build-only try push (I expect failures):
https://tbpl.mozilla.org/?tree=Try&rev=3af6947410f9
| Assignee | ||
Comment 2•13 years ago
|
||
Assignee: general → jimb
Status: NEW → ASSIGNED
| Assignee | ||
Comment 3•13 years ago
|
||
| Assignee | ||
Comment 4•13 years ago
|
||
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
| Assignee | ||
Comment 5•13 years ago
|
||
They're green.
| Assignee | ||
Comment 6•13 years ago
|
||
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)
| Assignee | ||
Comment 7•13 years ago
|
||
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 8•13 years ago
|
||
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+
Updated•13 years ago
|
Attachment #662248 -
Flags: review?(benjamin) → review+
| Assignee | ||
Comment 9•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/87bcca77b857
https://hg.mozilla.org/integration/mozilla-inbound/rev/6fd36867bac7
Flags: in-testsuite-
Target Milestone: --- → mozilla18
Comment 10•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/87bcca77b857
https://hg.mozilla.org/mozilla-central/rev/6fd36867bac7
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•