Closed
Bug 790117
Opened 10 years ago
Closed 10 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•10 years ago
|
||
Build-only try push (I expect failures): https://tbpl.mozilla.org/?tree=Try&rev=3af6947410f9
Assignee | ||
Comment 2•10 years ago
|
||
Assignee: general → jimb
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•10 years ago
|
||
Try: https://tbpl.mozilla.org/?tree=Try&rev=d333a752eec2
Assignee | ||
Comment 4•10 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•10 years ago
|
||
They're green.
Assignee | ||
Comment 6•10 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•10 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•10 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•10 years ago
|
Attachment #662248 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 9•10 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•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/87bcca77b857 https://hg.mozilla.org/mozilla-central/rev/6fd36867bac7
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•