Closed
Bug 488874
Opened 12 years ago
Closed 12 years ago
Change tm.onTrace to tm.tracecx
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jorendorff, Assigned: jorendorff)
Details
(Keywords: fixed1.9.1, Whiteboard: fixed-in-tracemonkey)
Attachments
(2 files)
8.06 KB,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
4.11 KB,
patch
|
Details | Diff | Splinter Review |
This would eliminate a linear search in js_DeepBail.
Assignee | ||
Updated•12 years ago
|
Assignee: general → jorendorff
Assignee | ||
Comment 1•12 years ago
|
||
Should have been done in bug 480301. Better late than never!
Attachment #374101 -
Flags: review?(brendan)
Updated•12 years ago
|
Attachment #374101 -
Flags: review?(brendan) → review+
Comment 2•12 years ago
|
||
Comment on attachment 374101 [details] [diff] [review] v1 >+ * The context currently executing JIT-compiled code on this thread, or >+ * NULL if none. Among other things, this can in certain cases prevent >+ * last-ditch garbage collection and suppress calls to >+ * JS_ReportOutOfMemory. Nit: s/garbage collection/GC when in Rome and for prettier wrapping. > uintN prohibitFlush; > JSBool needFlush; > > /* > * reservedObjects is a linked list (via fslots[0]) of preallocated JSObjects. > * The JIT uses this to ensure that leaving a trace tree can't fail. > */ >+ JSPackedBool useReservedObjects; Make needFlush a JSPackedBool too. Could pack prohibitFlush if we can bound tree nesting by 2^16 -- not a big deal, just wondering if there is a tree nesting limit other than memory limits (code cache size, no doubt). >+ * Reconstruct the JS stack and clear cx->tracecx. We must be currently >+ * executing a _FAIL builtin from trace on cx or another context on the same >+ * thread. The machine code for the trace remains on the C stack when >+ * js_DeepBail returns. Nit: s/executing/in/ for better wrapping and conciseness. >+ tm->prohibitFlush++; Is there an assertion to add to LeaveTree after the if (bailed) {...} that prohibitFlush is zero? Or something to handle the questions begged by the asymmetry between the ++ and the --? Looks great, this is wanted1.9.1+, I hope. /be
Updated•12 years ago
|
Flags: wanted1.9.1?
Comment 3•12 years ago
|
||
I think we want this. Carrying a diff against tm and m-c is a small pain in merge overhead but bigger in cognitive load. And this tightens up the code, requiring less reasoning. Especially if we can get that assertion I'm hoping for. /be
Assignee | ||
Comment 4•12 years ago
|
||
Just in case anyone's curious what the assertion looks like.
Assignee | ||
Comment 5•12 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/3ad791b6a1b4
Whiteboard: fixed-in-tracemonkey
Comment 6•12 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/3ad791b6a1b4
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: wanted1.9.1? → wanted1.9.1+
Resolution: --- → FIXED
Comment 7•12 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e18d7c1e49d3
Keywords: fixed1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•