From bed3611385ee41e08fbad569ae9fbb424f099d6e Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Sat, 14 Mar 2009 00:58:27 -0700 Subject: [PATCH] Bug 480132: Clone lexical blocks only when needed. r=igor Terminology: A "script block" is an object of class Block allocated by the byte compiler and associated with a script. Script blocks are never modified, and may be used as a prototype for a "closure block": A "closure block" is an object of class Block that holds variables that have been closed over (although we actually leave the variables on the stack until we leave their dynamic scope). A closure block is a clone of a script block (its prototype is a script block). Adjust the meanings of fp->blockChain and fp->scopeChain: fp->blockChain is always the innermost script block in whose static scope we're executing. fp->scopeChain is the current scope chain, including 'call' objects and closure blocks for those function calls and blocks in whose static scope we are currently executing, and 'with' objects for with statements; the chain is typically terminated by a global object. However, as an optimization, the young end of the chain omits block objects we have not yet needed to clone. Closures need fully reified scope chains, so have js_GetScopeChain reify any closure blocks missing from the young end of fp->scopeChain by cloning script blocks as needed from fp->blockChain. Thus, if we never actually close over a particular block, we never place a closure block for it on fp->scopeChain. Have JSOP_ENTERBLOCK and JSOP_LEAVEBLOCK always keep fp->blockChain current. When JSOP_LEAVEBLOCK pops a block from fp->blockChain that has been cloned on fp->scopeChain, pop fp->scopeChain as well. Remove the JSFRAME_POP_BLOCKS flag, as it is no longer needed. Ensure that the JIT won't have to create closure blocks or call js_PutBlockObject; it can't handle those things yet. Note our current script block when we begin recording. Abort recording if we leave that block; we can't tell in advance whether it will need to be "put" in future trace invocations. Leave trace if we call js_GetScopeChain while in the static scope of lexical blocks. Remove JIT tests based on JSFRAME_POP_BLOCKS. Verify that generators capture the correct value for blockChain. Add a constructor to JSAutoTempValueRooter for rooting JSObject pointers. --- js/src/jscntxt.h | 4 + js/src/jsinterp.cpp | 210 +++++++++++++++++++++++++++----------------- js/src/jsinterp.h | 43 ++++++++- js/src/jsiter.cpp | 2 + js/src/jstracer.cpp | 11 +-- js/src/jstracer.h | 1 + 6 files changed, 180 insertions(+), 91 deletions(-) diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 74797da0e20..c3203c3d389 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -1038,6 +1038,10 @@ class JSAutoTempValueRooter : mContext(cx) { JS_PUSH_TEMP_ROOT_STRING(mContext, str, &mTvr); } + JSAutoTempValueRooter(JSContext *cx, JSObject *obj) + : mContext(cx) { + JS_PUSH_TEMP_ROOT_OBJECT(mContext, obj, &mTvr); + } ~JSAutoTempValueRooter() { JS_POP_TEMP_ROOT(mContext, &mTvr); diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index df1e092eb72..ba721bd8372 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -663,11 +663,9 @@ js_FreeStack(JSContext *cx, void *mark) JSObject * js_GetScopeChain(JSContext *cx, JSStackFrame *fp) { - JSObject *obj, *cursor, *clonedChild, *parent; - JSTempValueRooter tvr; + JSObject *sharedBlock = fp->blockChain; - obj = fp->blockChain; - if (!obj) { + if (!sharedBlock) { /* * Don't force a call object for a lightweight function call, but do * insist that there is a call object for a heavyweight function call. @@ -679,70 +677,115 @@ js_GetScopeChain(JSContext *cx, JSStackFrame *fp) return fp->scopeChain; } + /* We don't handle cloning blocks on trace. */ + js_LeaveTrace(cx); + /* * We have one or more lexical scopes to reflect into fp->scopeChain, so * make sure there's a call object at the current head of the scope chain, * if this frame is a call frame. + * + * Also, identify the innermost compiler-allocated block we needn't clone. */ + JSObject *limitBlock, *limitClone; if (fp->fun && !fp->callobj) { JS_ASSERT(OBJ_GET_CLASS(cx, fp->scopeChain) != &js_BlockClass || OBJ_GET_PRIVATE(cx, fp->scopeChain) != fp); if (!js_GetCallObject(cx, fp)) return NULL; + + /* We know we must clone everything on blockChain. */ + limitBlock = limitClone = NULL; + } else { + /* + * scopeChain includes all blocks whose static scope we're within that + * have already been cloned. Find the innermost such block. Its + * prototype should appear on blockChain; we'll clone blockChain up + * to, but not including, that prototype. + */ + limitClone = fp->scopeChain; + while (OBJ_GET_CLASS(cx, limitClone) == &js_WithClass) + limitClone = OBJ_GET_PARENT(cx, limitClone); + JS_ASSERT(limitClone); + + /* + * It may seem like we don't know enough about limitClone to be able + * to just grab its prototype as we do here, but it's actually okay. + * + * If limitClone is a block object belonging to this frame, then its + * prototype is the innermost entry in blockChain that we have already + * cloned, and is thus the place to stop when we clone below. + * + * Otherwise, there are no blocks for this frame on scopeChain, and we + * need to clone the whole blockChain. In this case, limitBlock can + * point to any object known not to be on blockChain, since we simply + * loop until we hit limitBlock or NULL. If limitClone is a block, it + * isn't a block from this function, since blocks can't be nested + * within themselves on scopeChain (recursion is dynamic nesting, not + * static nesting). If limitClone isn't a block, its prototype won't + * be a block either. So we can just grab limitClone's prototype here + * regardless of its type or which frame it belongs to. + */ + limitBlock = OBJ_GET_PROTO(cx, limitClone); + + /* If the innermost block has already been cloned, we are done. */ + if (limitBlock == sharedBlock) + return fp->scopeChain; } /* - * Clone the block chain. To avoid recursive cloning we set the parent of - * the cloned child after we clone the parent. In the following loop when - * clonedChild is null it indicates the first iteration when no special GC - * rooting is necessary. On the second and the following iterations we - * have to protect cloned so far chain against the GC during cloning of - * the cursor object. + * Special-case cloning the innermost block; this doesn't have enough in + * common with subsequent steps to include in the loop. + * + * We pass fp->scopeChain and not null even if we override the parent slot + * later as null triggers useless calculations of slot's value in + * js_NewObject that js_CloneBlockObject calls. */ - cursor = obj; - clonedChild = NULL; + JSObject *innermostNewChild + = js_CloneBlockObject(cx, sharedBlock, fp->scopeChain, fp); + if (!innermostNewChild) + return NULL; + JSAutoTempValueRooter tvr(cx, innermostNewChild); + + /* + * Clone our way towards outer scopes until we reach the innermost + * enclosing function, or the innermost block we've already cloned. + */ + JSObject *newChild = innermostNewChild; for (;;) { - parent = OBJ_GET_PARENT(cx, cursor); + JS_ASSERT(OBJ_GET_PROTO(cx, newChild) == sharedBlock); + sharedBlock = OBJ_GET_PARENT(cx, sharedBlock); + + /* Sometimes limitBlock will be NULL, so check that first. */ + if (sharedBlock == limitBlock || !sharedBlock) + break; + + /* As in the call above, we don't know the real parent yet. */ + JSObject *clone + = js_CloneBlockObject(cx, sharedBlock, fp->scopeChain, fp); + if (!clone) + return NULL; /* - * We pass fp->scopeChain and not null even if we override the parent - * slot later as null triggers useless calculations of slot's value in - * js_NewObject that js_CloneBlockObject calls. + * Avoid OBJ_SET_PARENT overhead as newChild cannot escape to + * other threads. */ - cursor = js_CloneBlockObject(cx, cursor, fp->scopeChain, fp); - if (!cursor) { - if (clonedChild) - JS_POP_TEMP_ROOT(cx, &tvr); - return NULL; - } - if (!clonedChild) { - /* - * The first iteration. Check if other follow and root obj if so - * to protect the whole cloned chain against GC. - */ - obj = cursor; - if (!parent) - break; - JS_PUSH_TEMP_ROOT_OBJECT(cx, obj, &tvr); - } else { - /* - * Avoid OBJ_SET_PARENT overhead as clonedChild cannot escape to - * other threads. - */ - STOBJ_SET_PARENT(clonedChild, cursor); - if (!parent) { - JS_ASSERT(tvr.u.value == OBJECT_TO_JSVAL(obj)); - JS_POP_TEMP_ROOT(cx, &tvr); - break; - } - } - clonedChild = cursor; - cursor = parent; + STOBJ_SET_PARENT(newChild, clone); + newChild = clone; } - fp->flags |= JSFRAME_POP_BLOCKS; - fp->scopeChain = obj; - fp->blockChain = NULL; - return obj; + + /* + * If we found a limit block belonging to this frame, then we should have + * found it in blockChain. + */ + JS_ASSERT_IF(limitBlock && + OBJ_GET_CLASS(cx, limitBlock) == &js_BlockClass && + OBJ_GET_PRIVATE(cx, limitClone) == fp, + sharedBlock); + + /* Place our newly cloned blocks at the head of the scope chain. */ + fp->scopeChain = innermostNewChild; + return fp->scopeChain; } JSBool @@ -6741,51 +6784,58 @@ js_Interpret(JSContext *cx) regs.sp++; } +#ifdef DEBUG + JS_ASSERT(fp->blockChain == OBJ_GET_PARENT(cx, obj)); + /* - * If this frame had to reflect the compile-time block chain into - * the runtime scope chain, we can't optimize block scopes out of - * runtime any longer, because an outer block that parents obj has - * been cloned onto the scope chain. To avoid re-cloning such a - * parent and accumulating redundant clones via js_GetScopeChain, - * we must clone each block eagerly on entry, and push it on the - * scope chain, until this frame pops. + * The young end of fp->scopeChain may omit blocks if we + * haven't closed over them, but if there are any closure + * blocks on fp->scopeChain, they'd better be (clones of) + * ancestors of the block we're entering now; anything + * else we should have popped off fp->scopeChain when we + * left its static scope. */ - if (fp->flags & JSFRAME_POP_BLOCKS) { - JS_ASSERT(!fp->blockChain); - obj = js_CloneBlockObject(cx, obj, fp->scopeChain, fp); - if (!obj) - goto error; - fp->scopeChain = obj; - } else { - JS_ASSERT(!fp->blockChain || - OBJ_GET_PARENT(cx, obj) == fp->blockChain); - fp->blockChain = obj; + obj2 = fp->scopeChain; + while ((clasp = OBJ_GET_CLASS(cx, obj2)) == &js_WithClass) + obj2 = OBJ_GET_PARENT(cx, obj2); + if (clasp == &js_BlockClass && + OBJ_GET_PRIVATE(cx, obj2) == fp) { + JSObject *youngestProto = OBJ_GET_PROTO(cx, obj2); + JS_ASSERT(!OBJ_IS_CLONED_BLOCK(youngestProto)); + parent = obj; + while ((parent = OBJ_GET_PARENT(cx, parent)) != youngestProto) + JS_ASSERT(parent); } +#endif + + fp->blockChain = obj; END_CASE(JSOP_ENTERBLOCK) BEGIN_CASE(JSOP_LEAVEBLOCKEXPR) BEGIN_CASE(JSOP_LEAVEBLOCK) { #ifdef DEBUG - uintN blockDepth = OBJ_BLOCK_DEPTH(cx, - fp->blockChain - ? fp->blockChain - : fp->scopeChain); - - JS_ASSERT(blockDepth <= StackDepth(script)); + JS_ASSERT(OBJ_GET_CLASS(cx, fp->blockChain) == &js_BlockClass); + uintN blockDepth = OBJ_BLOCK_DEPTH(cx, fp->blockChain); + + JS_ASSERT(blockDepth <= StackDepth(script)); #endif - if (fp->blockChain) { - JS_ASSERT(OBJ_GET_CLASS(cx, fp->blockChain) == &js_BlockClass); - fp->blockChain = OBJ_GET_PARENT(cx, fp->blockChain); - } else { - /* - * This block was cloned into fp->scopeChain, so clear its - * private data and sync its locals to their property slots. - */ + /* + * If we're about to leave the dynamic scope of a block that has + * been cloned onto fp->scopeChain, clear its private data, move + * its locals from the stack into the clone, and pop it off the + * chain. + */ + obj = fp->scopeChain; + if (OBJ_GET_PROTO(cx, obj) == fp->blockChain) { + JS_ASSERT (OBJ_GET_CLASS(cx, obj) == &js_BlockClass); if (!js_PutBlockObject(cx, JS_TRUE)) goto error; } + /* Pop the block chain, too. */ + fp->blockChain = OBJ_GET_PARENT(cx, fp->blockChain); + /* * We will move the result of the expression to the new topmost * stack slot. diff --git a/js/src/jsinterp.h b/js/src/jsinterp.h index 115530a83fe..31e111edae4 100644 --- a/js/src/jsinterp.h +++ b/js/src/jsinterp.h @@ -82,13 +82,51 @@ struct JSStackFrame { jsval rval; /* function return value */ JSStackFrame *down; /* previous frame */ void *annotation; /* used by Java security */ - JSObject *scopeChain; /* scope chain */ + + /* + * We can't determine in advance which local variables can live on + * the stack and be freed when their dynamic scope ends, and which + * will be closed over and need to live in the heap. So we place + * variables on the stack initially, note when they are closed + * over, and copy those that are out to the heap when we leave + * their dynamic scope. + * + * The bytecode compiler produces a tree of block objects + * accompanying each JSScript representing those lexical blocks in + * the script that have let-bound variables associated with them. + * These block objects are never modified, and never become part + * of any function's scope chain. Their parent slots point to the + * innermost block that encloses them, or are NULL in the + * outermost blocks within a function or in eval or global code. + * + * When we are in the static scope of such a block, blockChain + * points to its compiler-allocated block object; otherwise, it is + * NULL. + * + * scopeChain is the current scope chain, including 'call' and + * 'block' objects for those function calls and lexical blocks + * whose static scope we are currently executing in, and 'with' + * objects for with statements; the chain is typically terminated + * by a global object. However, as an optimization, the young end + * of the chain omits block objects we have not yet cloned. To + * create a closure, we clone the missing blocks from blockChain + * (which is always current), place them at the head of + * scopeChain, and use that for the closure's scope chain. If we + * never close over a lexical block, we never place a mutable + * clone of it on scopeChain. + * + * This lazy cloning is implemented in js_GetScopeChain, which is + * also used in some other cases --- entering 'with' blocks, for + * example. + */ + JSObject *scopeChain; + JSObject *blockChain; + uintN sharpDepth; /* array/object initializer depth */ JSObject *sharpArray; /* scope for #n= initializer vars */ uint32 flags; /* frame flags -- see below */ JSStackFrame *dormantNext; /* next dormant frame chain */ JSObject *xmlNamespace; /* null or default xml namespace in E4X */ - JSObject *blockChain; /* active compile-time block scopes */ JSStackFrame *displaySave; /* previous value of display entry for script->staticDepth */ #ifdef DEBUG @@ -140,7 +178,6 @@ typedef struct JSInlineFrame { #define JSFRAME_ROOTED_ARGV 0x20 /* frame.argv is rooted by the caller */ #define JSFRAME_YIELDING 0x40 /* js_Interpret dispatched JSOP_YIELD */ #define JSFRAME_ITERATOR 0x80 /* trying to get an iterator for for-in */ -#define JSFRAME_POP_BLOCKS 0x100 /* scope chain contains blocks to pop */ #define JSFRAME_GENERATOR 0x200 /* frame belongs to generator-iterator */ #define JSFRAME_IMACRO_START 0x400 /* imacro starting -- see jstracer.h */ diff --git a/js/src/jsiter.cpp b/js/src/jsiter.cpp index b7539a8f35a..b1e95ca175a 100644 --- a/js/src/jsiter.cpp +++ b/js/src/jsiter.cpp @@ -786,6 +786,8 @@ js_NewGenerator(JSContext *cx, JSStackFrame *fp) gen->frame.flags = (fp->flags & ~JSFRAME_ROOTED_ARGV) | JSFRAME_GENERATOR; gen->frame.dormantNext = NULL; gen->frame.xmlNamespace = NULL; + /* JSOP_GENERATOR appears in the prologue, outside all blocks. */ + JS_ASSERT(!fp->blockChain); gen->frame.blockChain = NULL; /* Note that gen is newborn. */ diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp index cedbfce1b4f..f5a036c029c 100644 --- a/js/src/jstracer.cpp +++ b/js/src/jstracer.cpp @@ -1206,6 +1206,7 @@ TraceRecorder::TraceRecorder(JSContext* cx, VMSideExit* _anchor, Fragment* _frag this->cx = cx; this->traceMonitor = &JS_TRACE_MONITOR(cx); this->globalObj = JS_GetGlobalForObject(cx, cx->fp->scopeChain); + this->lexicalBlock = cx->fp->blockChain; this->anchor = _anchor; this->fragment = _fragment; this->lirbuf = _fragment->lirbuf; @@ -3208,7 +3209,6 @@ js_SynthesizeFrame(JSContext* cx, const FrameInfo& fi) newifp->callerRegs.sp = fp->slots + fi.s.spdist; fp->imacpc = fi.imacpc; - JS_ASSERT(!(fp->flags & JSFRAME_POP_BLOCKS)); #ifdef DEBUG if (fi.block != fp->blockChain) { for (JSObject* obj = fi.block; obj != fp->blockChain; obj = STOBJ_GET_PARENT(obj)) @@ -3948,7 +3948,6 @@ js_ExecuteTree(JSContext* cx, Fragment* f, uintN& inlineCallCount, return NULL; #ifdef DEBUG - state.jsframe_pop_blocks_set_on_entry = ((cx->fp->flags & JSFRAME_POP_BLOCKS) != 0); memset(stack_buffer, 0xCD, sizeof(stack_buffer)); memset(state.global, 0xCD, (globalFrameSize+1)*sizeof(double)); #endif @@ -4134,8 +4133,6 @@ LeaveTree(InterpState& state, VMSideExit* lr) the side exit struct. */ JSStackFrame* fp = cx->fp; - JS_ASSERT_IF(fp->flags & JSFRAME_POP_BLOCKS, - calldepth == 0 && state.jsframe_pop_blocks_set_on_entry); fp->blockChain = innermost->block; /* If we are not exiting from an inlined frame the state->sp is spbase, otherwise spbase @@ -9133,9 +9130,6 @@ TraceRecorder::record_JSOP_TYPEOFEXPR() JS_REQUIRES_STACK bool TraceRecorder::record_JSOP_ENTERBLOCK() { - if (cx->fp->flags & JSFRAME_POP_BLOCKS) - ABORT_TRACE("can't trace after js_GetScopeChain"); - JSScript* script = cx->fp->script; JSFrameRegs& regs = *cx->fp->regs; JSObject* obj; @@ -9150,7 +9144,8 @@ TraceRecorder::record_JSOP_ENTERBLOCK() JS_REQUIRES_STACK bool TraceRecorder::record_JSOP_LEAVEBLOCK() { - return true; + /* We mustn't exit the lexical block we began recording in. */ + return cx->fp->blockChain != lexicalBlock; } JS_REQUIRES_STACK bool diff --git a/js/src/jstracer.h b/js/src/jstracer.h index 0cbcb567bb6..b9548b46958 100644 --- a/js/src/jstracer.h +++ b/js/src/jstracer.h @@ -382,6 +382,7 @@ class TraceRecorder : public avmplus::GCObject { JSContext* cx; JSTraceMonitor* traceMonitor; JSObject* globalObj; + JSObject* lexicalBlock; Tracker tracker; Tracker nativeFrameTracker; char* entryTypeMap;