Closed
Bug 679981
Opened 13 years ago
Closed 13 years ago
IonMonkey: Refactor IonSpewer to handle multiple functions.
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: sstangl, Assigned: sstangl)
Details
Attachments
(2 files)
12.65 KB,
patch
|
rpearl
:
review+
|
Details | Diff | Splinter Review |
3.93 KB,
patch
|
rpearl
:
review+
|
Details | Diff | Splinter Review |
The IonSpewer overwrites /tmp/ion.json at the start of function compilation, which hides output for scripts compiling multiple functions. This is marginally annoying when debugging function calls, since the CFG of the caller is always masked. We should rewrite IonSpewer in the singleton pattern with a global object. Additionally, the IonSpewer should have no effect (not even with respect to memory) on opt builds.
Assignee | ||
Comment 1•13 years ago
|
||
Attachment #554002 -
Flags: review?(rpearl)
Comment 2•13 years ago
|
||
Comment on attachment 554002 [details] [diff] [review] Refactor {Ion,C1,JSON}Spewer to handle multiple functions. Review of attachment 554002 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/C1Spewer.cpp @@ +65,5 @@ > spewout_ = fopen(path, "w"); > if (!spewout_) > + return false; > + > + return true; return (bool)spewout_; or something like that... ::: js/src/ion/Ion.cpp @@ +398,4 @@ > > if (!BuildDominatorTree(graph)) > return false; > + IonSpewPass("Dominator tree"); I used to have this pass in here because I wanted to emit the dominator tree as well. It doesn't actually change the shape of the graph; it just annotates it. Should we remove this spew pass? ::: js/src/ion/IonSpewer.cpp @@ +45,5 @@ > > using namespace js; > using namespace js::ion; > > +// IonSpewer lingleton. lingleton? I hope this is a typo, and not some terrifying design pattern. @@ +51,2 @@ > > +static bool LoggingChecked = false; this doesn't appear to be used. @@ +148,2 @@ > { > + JS_ASSERT(inited_); c1spewer should probably have a no-op endFunction()
Attachment #554002 -
Flags: review?(rpearl) → review+
Assignee | ||
Comment 3•13 years ago
|
||
Thanks! LoggingChecked is used by some other code in IonSpewer.cpp that wasn't touched. This is a small follow-up patch that addresses the comments above and has the JSONSpewer notice when it was asked to terminate or start a new function before the previous one ended. It then correctly terminates the old function. This lets us get rid of the patch-up logic in iongraph, which didn't *really* work anyway.
Attachment #554153 -
Flags: review?(rpearl)
Updated•13 years ago
|
Attachment #554153 -
Flags: review?(rpearl) → review+
Assignee | ||
Comment 4•13 years ago
|
||
http://hg.mozilla.org/projects/ionmonkey/rev/2f95cb807c67
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•