Bug 739808: Disable method cloning optimization and method barrier, r=luke, a=akeybl
authorDavid Mandelin <dmandelin@mozilla.com>
Thu, 10 May 2012 18:38:23 -0700
changeset 92206 6f0397098e05
parent 92205 4ed154f44f8d
child 92207 909c744af251
push id840
push userdmandelin@mozilla.com
push dateMon, 14 May 2012 23:54:43 +0000
treeherdermozilla-beta@6f0397098e05 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke, akeybl
bugs739808
milestone13.0
Bug 739808: Disable method cloning optimization and method barrier, r=luke, a=akeybl
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/SemanticAnalysis.cpp
js/src/jit-test/tests/basic/testInitDictionary.js
js/src/jsapi-tests/testLookup.cpp
js/src/jsfun.h
js/src/jsfuninlines.h
js/src/jsinterp.cpp
js/src/methodjit/Compiler.cpp
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -4667,53 +4667,16 @@ EmitWith(JSContext *cx, BytecodeEmitter 
     if (!EmitTree(cx, bce, pn->pn_right))
         return false;
     if (Emit1(cx, bce, JSOP_LEAVEWITH) < 0)
         return false;
     return PopStatementBCE(cx, bce);
 }
 
 static bool
-SetMethodFunction(JSContext *cx, FunctionBox *funbox, JSAtom *atom)
-{
-    RootedVarObject parent(cx);
-    parent = funbox->function()->getParent();
-
-    /*
-     * Replace a boxed function with a new one with a method atom. Methods
-     * require a function with the extended size finalize kind, which normal
-     * functions don't have. We don't eagerly allocate functions with the
-     * expanded size for boxed functions, as most functions are not methods.
-     */
-    JSFunction *fun = js_NewFunction(cx, NULL, NULL,
-                                     funbox->function()->nargs,
-                                     funbox->function()->flags,
-                                     parent,
-                                     funbox->function()->atom,
-                                     JSFunction::ExtendedFinalizeKind);
-    if (!fun)
-        return false;
-
-    JSScript *script = funbox->function()->script();
-    if (script) {
-        fun->setScript(script);
-        if (!script->typeSetFunction(cx, fun))
-            return false;
-    }
-
-    JS_ASSERT(funbox->function()->joinable());
-    fun->setJoinable();
-
-    fun->setMethodAtom(atom);
-
-    funbox->object = fun;
-    return true;
-}
-
-static bool
 EmitForIn(JSContext *cx, BytecodeEmitter *bce, ParseNode *pn, ptrdiff_t top)
 {
     StmtInfo stmtInfo;
     PushStatement(bce, &stmtInfo, STMT_FOR_IN_LOOP, top);
 
     ParseNode *forHead = pn->pn_left;
     ParseNode *forBody = pn->pn_right;
 
@@ -5468,33 +5431,16 @@ EmitStatement(JSContext *cx, BytecodeEmi
         if (!wantval &&
             pn2->isKind(PNK_ASSIGN) &&
             !MaybeEmitGroupAssignment(cx, bce, op, pn2, &op))
         {
             return false;
         }
 #endif
         if (op != JSOP_NOP) {
-            /*
-             * Specialize JSOP_SETPROP to JSOP_SETMETHOD to defer or
-             * avoid null closure cloning. Do this only for assignment
-             * statements that are not completion values wanted by a
-             * script evaluator, to ensure that the joined function
-             * can't escape directly.
-             */
-            if (!wantval &&
-                pn2->isKind(PNK_ASSIGN) &&
-                pn2->pn_left->isOp(JSOP_SETPROP) &&
-                pn2->pn_right->isOp(JSOP_LAMBDA) &&
-                pn2->pn_right->pn_funbox->joinable())
-            {
-                if (!SetMethodFunction(cx, pn2->pn_right->pn_funbox, pn2->pn_left->pn_atom))
-                    return false;
-                pn2->pn_left->setOp(JSOP_SETMETHOD);
-            }
             if (!EmitTree(cx, bce, pn2))
                 return false;
             if (Emit1(cx, bce, op) < 0)
                 return false;
         }
     } else if (!pn->isDirectivePrologueMember()) {
         /* Don't complain about directive prologue members; just don't emit their code. */
         bce->current->currentLine = pn2->pn_pos.begin.lineno;
@@ -5992,38 +5938,23 @@ EmitObject(JSContext *cx, BytecodeEmitte
             if (Emit1(cx, bce, JSOP_INITELEM) < 0)
                 return false;
         } else {
             JS_ASSERT(pn3->isKind(PNK_NAME) || pn3->isKind(PNK_STRING));
             jsatomid index;
             if (!bce->makeAtomIndex(pn3->pn_atom, &index))
                 return false;
 
-            /* Check whether we can optimize to JSOP_INITMETHOD. */
-            ParseNode *init = pn2->pn_right;
-            bool lambda = init->isOp(JSOP_LAMBDA);
-            if (lambda)
-                ++methodInits;
-            if (op == JSOP_INITPROP && lambda && init->pn_funbox->joinable()) {
+            /*
+             * Disable NEWOBJECT on initializers that set __proto__, which has
+             * a non-standard setter on objects.
+             */
+            if (pn3->pn_atom == cx->runtime->atomState.protoAtom)
                 obj = NULL;
-                op = JSOP_INITMETHOD;
-                if (!SetMethodFunction(cx, init->pn_funbox, pn3->pn_atom))
-                    return JS_FALSE;
-                pn2->setOp(op);
-            } else {
-                /*
-                 * Disable NEWOBJECT on initializers that set __proto__, which has
-                 * a non-standard setter on objects.
-                 */
-                if (pn3->pn_atom == cx->runtime->atomState.protoAtom)
-                    obj = NULL;
-                op = JSOP_INITPROP;
-                if (lambda)
-                    ++slowMethodInits;
-            }
+            op = JSOP_INITPROP;
 
             if (obj) {
                 JS_ASSERT(!obj->inDictionaryMode());
                 if (!DefineNativeProperty(cx, obj, ATOM_TO_JSID(pn3->pn_atom),
                                           UndefinedValue(), NULL, NULL,
                                           JSPROP_ENUMERATE, 0, 0))
                 {
                     return false;
--- a/js/src/frontend/SemanticAnalysis.cpp
+++ b/js/src/frontend/SemanticAnalysis.cpp
@@ -612,19 +612,16 @@ SetFunctionKinds(FunctionBox *funbox, ui
 
             for (AtomDefnRange r = upvars->all(); !r.empty(); r.popFront()) {
                 Definition *defn = r.front().value();
                 Definition *lexdep = defn->resolve();
                 if (!lexdep->isFreeVar())
                     FlagHeavyweights(lexdep, funbox, tcflags);
             }
         }
-
-        if (funbox->joinable())
-            fun->setJoinable();
     }
 }
 
 /*
  * Walk the FunctionBox tree looking for functions whose call objects may
  * acquire new bindings as they execute: non-strict functions that call eval,
  * and functions that contain function statements (definitions not appearing
  * within the top statement list, which don't take effect unless they are
deleted file mode 100644
--- a/js/src/jit-test/tests/basic/testInitDictionary.js
+++ /dev/null
@@ -1,51 +0,0 @@
-
-var shapes = {};
-
-function stringify(a) {
-  assertEq(shapes[shapeOf(a)], undefined);
-  shapes[shapeOf(a)] = 1;
-  var b = "";
-  for (var c in a) {
-    b += c + ":";
-    if (typeof a[c] == "function")
-      b += "function,";
-    else
-      b += a[c] + ",";
-  }
-  return b;
-}
-
-function test1() {
-  return stringify({a: 0, b: 1, a: function() {} });
-}
-for (var i = 0; i < 3; i++)
-  assertEq(test1(), "a:function,b:1,");
-
-// This does not cause the object to go to dictionary mode, unlike the above.
-function test2() {
-  return stringify({a: 0, b: 1, a: 2, b: 3});
-}
-assertEq(test2(), "a:2,b:3,");
-
-function test3() {
-  return stringify({
-    aa:0,ab:1,ac:2,ad:3,ae:4,af:5,ag:6,ah:7,ai:8,aj:9,
-    ba:0,bb:1,bc:2,bd:3,be:4,bf:5,bg:6,bh:7,bi:8,bj:9,
-    ca:0,cb:1,cc:2,cd:3,ce:4,cf:5,cg:6,ch:7,ci:8,cj:9,
-    da:0,db:1,dc:2,dd:3,de:4,df:5,dg:6,dh:7,di:8,dj:9,
-    ea:0,eb:1,ec:2,ed:3,ee:4,ef:5,eg:6,eh:7,ei:8,ej:9,
-    fa:0,fb:1,fc:2,fd:3,fe:4,ff:5,fg:6,fh:7,fi:8,fj:9,
-    ga:0,gb:1,gc:2,gd:3,ge:4,gf:5,gg:6,gh:7,gi:8,gj:9,
-    ha:0,hb:1,hc:2,hd:3,he:4,hf:5,hg:6,hh:7,hi:8,hj:9,
-    ia:0,ib:1,ic:2,id:3,ie:4,if:5,ig:6,ih:7,ii:8,ij:9,
-    ja:0,jb:1,jc:2,jd:3,je:4,jf:5,jg:6,jh:7,ji:8,jj:9,
-    ka:0,kb:1,kc:2,kd:3,ke:4,kf:5,kg:6,kh:7,ki:8,kj:9,
-    la:0,lb:1,lc:2,ld:3,le:4,lf:5,lg:6,lh:7,li:8,lj:9,
-    ma:0,mb:1,mc:2,md:3,me:4,mf:5,mg:6,mh:7,mi:8,mj:9,
-    na:0,nb:1,nc:2,nd:3,ne:4,nf:5,ng:6,nh:7,ni:8,nj:9,
-    oa:0,ob:1,oc:2,od:3,oe:4,of:5,og:6,oh:7,oi:8,oj:9,
-    pa:0,pb:1,pc:2,pd:3,pe:4,pf:5,pg:6,ph:7,pi:8,pj:9,
-        });
-}
-for (var i = 0; i < 10; i++)
-  assertEq(test3(), "aa:0,ab:1,ac:2,ad:3,ae:4,af:5,ag:6,ah:7,ai:8,aj:9,ba:0,bb:1,bc:2,bd:3,be:4,bf:5,bg:6,bh:7,bi:8,bj:9,ca:0,cb:1,cc:2,cd:3,ce:4,cf:5,cg:6,ch:7,ci:8,cj:9,da:0,db:1,dc:2,dd:3,de:4,df:5,dg:6,dh:7,di:8,dj:9,ea:0,eb:1,ec:2,ed:3,ee:4,ef:5,eg:6,eh:7,ei:8,ej:9,fa:0,fb:1,fc:2,fd:3,fe:4,ff:5,fg:6,fh:7,fi:8,fj:9,ga:0,gb:1,gc:2,gd:3,ge:4,gf:5,gg:6,gh:7,gi:8,gj:9,ha:0,hb:1,hc:2,hd:3,he:4,hf:5,hg:6,hh:7,hi:8,hj:9,ia:0,ib:1,ic:2,id:3,ie:4,if:5,ig:6,ih:7,ii:8,ij:9,ja:0,jb:1,jc:2,jd:3,je:4,jf:5,jg:6,jh:7,ji:8,jj:9,ka:0,kb:1,kc:2,kd:3,ke:4,kf:5,kg:6,kh:7,ki:8,kj:9,la:0,lb:1,lc:2,ld:3,le:4,lf:5,lg:6,lh:7,li:8,lj:9,ma:0,mb:1,mc:2,md:3,me:4,mf:5,mg:6,mh:7,mi:8,mj:9,na:0,nb:1,nc:2,nd:3,ne:4,nf:5,ng:6,nh:7,ni:8,nj:9,oa:0,ob:1,oc:2,od:3,oe:4,of:5,og:6,oh:7,oi:8,oj:9,pa:0,pb:1,pc:2,pd:3,pe:4,pf:5,pg:6,ph:7,pi:8,pj:9,");
--- a/js/src/jsapi-tests/testLookup.cpp
+++ b/js/src/jsapi-tests/testLookup.cpp
@@ -23,17 +23,16 @@ BEGIN_TEST(testLookup_bug522590)
 
     // This lookup must not return an internal function object.
     jsvalRoot r(cx);
     CHECK(JS_LookupProperty(cx, xobj, "f", r.addr()));
     CHECK(JSVAL_IS_OBJECT(r));
     JSObject *funobj = JSVAL_TO_OBJECT(r);
     CHECK(funobj->isFunction());
     CHECK(!js::IsInternalFunctionObject(funobj));
-    CHECK(funobj->toFunction()->isClonedMethod());
 
     return true;
 }
 END_TEST(testLookup_bug522590)
 
 JSBool
 document_resolve(JSContext *cx, JSObject *obj, jsid id, unsigned flags, JSObject **objp)
 {
--- a/js/src/jsfun.h
+++ b/js/src/jsfun.h
@@ -80,20 +80,17 @@
  *
  * NB: JSFUN_EXPR_CLOSURE reuses JSFUN_STUB_GSOPS, which is an API request flag
  * bit only, never stored in fun->flags.
  *
  * If we need more bits in the future, all flags for interpreted functions can
  * move to u.i.script->flags. For now we use function flag bits to minimize
  * pointer-chasing.
  */
-#define JSFUN_JOINABLE      0x0001  /* function is null closure that does not
-                                       appear to call itself via its own name
-                                       or arguments.callee */
-
+// 0x0001 was JSFUN_JOINABLE, but was deleted for bug 739808.
 #define JSFUN_PROTOTYPE     0x0800  /* function is Function.prototype for some
                                        global object */
 
 #define JSFUN_EXPR_CLOSURE  0x1000  /* expression closure: function(x) x*x */
 #define JSFUN_EXTENDED      0x2000  /* structure is FunctionExtended */
 #define JSFUN_INTERPRETED   0x4000  /* use u.i if kind >= this value else u.n */
 #define JSFUN_FLAT_CLOSURE  0x8000  /* flat (aka "display") closure */
 #define JSFUN_NULL_CLOSURE  0xc000  /* null closure entrains no scope chain */
@@ -153,31 +150,29 @@ struct JSFunction : public JSObject
 #define JS_LOCAL_NAME_TO_ATOM(nameWord)  ((JSAtom *) ((nameWord) & ~uintptr_t(1)))
 #define JS_LOCAL_NAME_IS_CONST(nameWord) ((((nameWord) & uintptr_t(1))) != 0)
 
     bool mightEscape() const {
         return isInterpreted() && (isFlatClosure() || !script()->bindings.hasUpvars());
     }
 
     bool joinable() const {
-        return flags & JSFUN_JOINABLE;
+        return false;
     }
 
     /*
      * For an interpreted function, accessors for the initial scope object of
      * activations (stack frames) of the function.
      */
     inline JSObject *environment() const;
     inline void setEnvironment(JSObject *obj);
     inline void initEnvironment(JSObject *obj);
 
     static inline size_t offsetOfEnvironment() { return offsetof(JSFunction, u.i.env_); }
 
-    inline void setJoinable();
-
     js::HeapPtrScript &script() const {
         JS_ASSERT(isInterpreted());
         return *(js::HeapPtrScript *)&u.i.script_;
     }
 
     inline void setScript(JSScript *script_);
     inline void initScript(JSScript *script_);
 
--- a/js/src/jsfuninlines.h
+++ b/js/src/jsfuninlines.h
@@ -79,23 +79,16 @@ JSFunction::initializeExtended()
 {
     JS_ASSERT(isExtended());
 
     JS_ASSERT(js::ArrayLength(toExtended()->extendedSlots) == 2);
     toExtended()->extendedSlots[0].init(js::UndefinedValue());
     toExtended()->extendedSlots[1].init(js::UndefinedValue());
 }
 
-inline void
-JSFunction::setJoinable()
-{
-    JS_ASSERT(isInterpreted());
-    flags |= JSFUN_JOINABLE;
-}
-
 inline bool
 JSFunction::isClonedMethod() const
 {
     return joinable() && isExtended() && getExtendedSlot(METHOD_OBJECT_SLOT).isObject();
 }
 
 inline JSAtom *
 JSFunction::methodAtom() const
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -3253,83 +3253,16 @@ BEGIN_CASE(JSOP_LAMBDA)
     JSFunction *fun = script->getFunction(GET_UINT32_INDEX(regs.pc));
     JSObject *obj = fun;
 
     /* do-while(0) so we can break instead of using a goto. */
     do {
         JSObject *parent;
         if (fun->isNullClosure()) {
             parent = &regs.fp()->scopeChain();
-
-            if (fun->joinable()) {
-                jsbytecode *pc2 = regs.pc + JSOP_LAMBDA_LENGTH;
-                JSOp op2 = JSOp(*pc2);
-
-                /*
-                 * Optimize var obj = {method: function () { ... }, ...},
-                 * this.method = function () { ... }; and other significant
-                 * single-use-of-null-closure bytecode sequences.
-                 */
-                if (op2 == JSOP_INITMETHOD) {
-#ifdef DEBUG
-                    const Value &lref = regs.sp[-1];
-                    JS_ASSERT(lref.isObject());
-                    JSObject *obj2 = &lref.toObject();
-                    JS_ASSERT(obj2->isObject());
-#endif
-                    JS_ASSERT(fun->methodAtom() ==
-                              script->getAtom(GET_UINT32_INDEX(regs.pc + JSOP_LAMBDA_LENGTH)));
-                    break;
-                }
-
-                if (op2 == JSOP_SETMETHOD) {
-#ifdef DEBUG
-                    op2 = JSOp(pc2[JSOP_SETMETHOD_LENGTH]);
-                    JS_ASSERT(op2 == JSOP_POP || op2 == JSOP_POPV);
-#endif
-                    const Value &lref = regs.sp[-1];
-                    if (lref.isObject() && lref.toObject().canHaveMethodBarrier()) {
-                        JS_ASSERT(fun->methodAtom() ==
-                                  script->getAtom(GET_UINT32_INDEX(regs.pc + JSOP_LAMBDA_LENGTH)));
-                        break;
-                    }
-                } else if (op2 == JSOP_CALL) {
-                    /*
-                     * Array.prototype.sort and String.prototype.replace are
-                     * optimized as if they are special form. We know that they
-                     * won't leak the joined function object in obj, therefore
-                     * we don't need to clone that compiler-created function
-                     * object for identity/mutation reasons.
-                     */
-                    int iargc = GET_ARGC(pc2);
-
-                    /*
-                     * Note that we have not yet pushed obj as the final argument,
-                     * so regs.sp[1 - (iargc + 2)], and not regs.sp[-(iargc + 2)],
-                     * is the callee for this JSOP_CALL.
-                     */
-                    const Value &cref = regs.sp[1 - (iargc + 2)];
-                    JSFunction *fun;
-
-                    if (IsFunctionObject(cref, &fun)) {
-                        if (Native native = fun->maybeNative()) {
-                            if ((iargc == 1 && native == array_sort) ||
-                                (iargc == 2 && native == str_replace)) {
-                                break;
-                            }
-                        }
-                    }
-                } else if (op2 == JSOP_NULL) {
-                    pc2 += JSOP_NULL_LENGTH;
-                    op2 = JSOp(*pc2);
-
-                    if (op2 == JSOP_CALL && GET_ARGC(pc2) == 0)
-                        break;
-                }
-            }
         } else {
             parent = GetScopeChain(cx, regs.fp());
             if (!parent)
                 goto error;
         }
 
         obj = CloneFunctionObjectIfNotSingleton(cx, fun, parent);
         if (!obj)
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -3076,37 +3076,16 @@ mjit::Compiler::generateMethod()
           BEGIN_CASE(JSOP_LAMBDA)
           {
             JSFunction *fun = script->getFunction(GET_UINT32_INDEX(PC));
 
             JSObjStubFun stub = stubs::Lambda;
             uint32_t uses = 0;
 
             jsbytecode *pc2 = NULL;
-            if (fun->joinable()) {
-                pc2 = PC + JSOP_LAMBDA_LENGTH;
-                JSOp next = JSOp(*pc2);
-
-                if (next == JSOP_INITMETHOD) {
-                    stub = stubs::LambdaJoinableForInit;
-                } else if (next == JSOP_SETMETHOD) {
-                    stub = stubs::LambdaJoinableForSet;
-                    uses = 1;
-                } else if (next == JSOP_CALL) {
-                    int iargc = GET_ARGC(pc2);
-                    if (iargc == 1 || iargc == 2) {
-                        stub = stubs::LambdaJoinableForCall;
-                        uses = frame.frameSlots();
-                    }
-                } else if (next == JSOP_NULL) {
-                    pc2 += JSOP_NULL_LENGTH;
-                    if (JSOp(*pc2) == JSOP_CALL && GET_ARGC(pc2) == 0)
-                        stub = stubs::LambdaJoinableForNull;
-                }
-            }
 
             prepareStubCall(Uses(uses));
             masm.move(ImmPtr(fun), Registers::ArgReg1);
 
             INLINE_STUBCALL(stub, REJOIN_PUSH_OBJECT);
 
             frame.takeReg(Registers::ReturnReg);
             frame.pushTypedPayload(JSVAL_TYPE_OBJECT, Registers::ReturnReg);