From 87b00dccdb376ffff7544e332504566f12d77db1 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Tue, 30 Aug 2011 06:08:30 -0500 Subject: [PATCH] Bug 669369 - Simplify Parser::setFunctionKinds. r=dmandelin. --HG-- extra : rebase_source : 2c6d99d8cb30b947b0c1192968a30f70ef19178f --- js/src/jsparse.cpp | 206 +++++++++++++++++---------------------------- 1 file changed, 79 insertions(+), 127 deletions(-) diff --git a/js/src/jsparse.cpp b/js/src/jsparse.cpp index 2cd88825e09..ba80573c8df 100644 --- a/js/src/jsparse.cpp +++ b/js/src/jsparse.cpp @@ -2272,8 +2272,9 @@ CanFlattenUpvar(JSDefinition *dn, JSFunctionBox *funbox, uint32 tcflags) /* * If this function is reaching up across an enclosing funarg, then we - * cannot copy dn's value into a flat closure slot (the display stops - * working once the funarg escapes). + * cannot copy dn's value into a flat closure slot. The flat closure + * code assumes the upvars to be copied are in frames still on the + * stack. */ if (!afunbox || afunbox->node->isFunArg()) return false; @@ -2412,48 +2413,56 @@ DeoptimizeUsesWithin(JSDefinition *dn, const TokenPos &pos) return ndeoptimized != 0; } +static void +ConsiderUnbranding(JSFunctionBox *funbox) +{ + /* + * We've already recursively set our kids' kinds, which also classifies + * enclosing functions holding upvars referenced in those descendants' + * bodies. So now we can check our "methods". + * + * Despecialize from branded method-identity-based shape to shape- or + * slot-based shape if this function smells like a constructor and too many + * of its methods are *not* joinable null closures (i.e., they have one or + * more upvars fetched via the display). + */ + bool returnsExpr = !!(funbox->tcflags & TCF_RETURN_EXPR); +#if JS_HAS_EXPR_CLOSURES + { + JSParseNode *pn2 = funbox->node->pn_body; + if (PN_TYPE(pn2) == TOK_UPVARS) + pn2 = pn2->pn_tree; + if (PN_TYPE(pn2) == TOK_ARGSBODY) + pn2 = pn2->last(); + if (PN_TYPE(pn2) != TOK_LC) + returnsExpr = true; + } +#endif + if (!returnsExpr) { + uintN methodSets = 0, slowMethodSets = 0; + + for (JSParseNode *method = funbox->methods; method; method = method->pn_link) { + JS_ASSERT(PN_OP(method) == JSOP_LAMBDA || PN_OP(method) == JSOP_LAMBDA_FC); + ++methodSets; + if (!method->pn_funbox->joinable()) + ++slowMethodSets; + } + + if (funbox->shouldUnbrand(methodSets, slowMethodSets)) + funbox->tcflags |= TCF_FUN_UNBRAND_THIS; + } +} + void Parser::setFunctionKinds(JSFunctionBox *funbox, uint32 *tcflags) { - for (;;) { + for (; funbox; funbox = funbox->siblings) { JSParseNode *fn = funbox->node; JSParseNode *pn = fn->pn_body; if (funbox->kids) { setFunctionKinds(funbox->kids, tcflags); - - /* - * We've unwound from recursively setting our kids' kinds, which - * also classifies enclosing functions holding upvars referenced in - * those descendants' bodies. So now we can check our "methods". - * - * Despecialize from branded method-identity-based shape to shape- - * or slot-based shape if this function smells like a constructor - * and too many of its methods are *not* joinable null closures - * (i.e., they have one or more upvars fetched via the display). - */ - JSParseNode *pn2 = pn; - if (PN_TYPE(pn2) == TOK_UPVARS) - pn2 = pn2->pn_tree; - if (PN_TYPE(pn2) == TOK_ARGSBODY) - pn2 = pn2->last(); - -#if JS_HAS_EXPR_CLOSURES - if (PN_TYPE(pn2) == TOK_LC) -#endif - if (!(funbox->tcflags & TCF_RETURN_EXPR)) { - uintN methodSets = 0, slowMethodSets = 0; - - for (JSParseNode *method = funbox->methods; method; method = method->pn_link) { - JS_ASSERT(PN_OP(method) == JSOP_LAMBDA || PN_OP(method) == JSOP_LAMBDA_FC); - ++methodSets; - if (!method->pn_funbox->joinable()) - ++slowMethodSets; - } - - if (funbox->shouldUnbrand(methodSets, slowMethodSets)) - funbox->tcflags |= TCF_FUN_UNBRAND_THIS; - } + ConsiderUnbranding(funbox); } JSFunction *fun = funbox->function(); @@ -2464,54 +2473,13 @@ Parser::setFunctionKinds(JSFunctionBox *funbox, uint32 *tcflags) /* nothing to do */ } else if (funbox->inAnyDynamicScope()) { JS_ASSERT(!fun->isNullClosure()); - } else if (pn->pn_type != TOK_UPVARS) { - /* - * No lexical dependencies => null closure, for best performance. - * A null closure needs no scope chain, but alas we've coupled - * principals-finding to scope (for good fundamental reasons, but - * the implementation overloads the parent slot and we should fix - * that). See, e.g., the JSOP_LAMBDA case in jsinterp.cpp. - * - * In more detail: the ES3 spec allows the implementation to create - * "joined function objects", or not, at its discretion. But real- - * world implementations always create unique function objects for - * closures, and this can be detected via mutation. Open question: - * do popular implementations create unique function objects for - * null closures? - * - * FIXME: bug 476950. - */ - fun->setKind(JSFUN_NULL_CLOSURE); } else { - AtomDefnMapPtr upvars = pn->pn_names; - JS_ASSERT(!upvars->empty()); + uintN hasUpvars = false; + bool canFlatten = true; - if (!fn->isFunArg()) { - /* - * This function is Algol-like, it never escapes. - * - * Check that at least one outer lexical binding was assigned - * to (global variables don't count). This is conservative: we - * could limit assignments to those in the current function, - * but that's too much work. As with flat closures (handled - * below), we optimize for the case where outer bindings are - * not reassigned anywhere. - */ - AtomDefnRange r = upvars->all(); - for (; !r.empty(); r.popFront()) { - JSDefinition *defn = r.front().value(); - JSDefinition *lexdep = defn->resolve(); - - if (!lexdep->isFreeVar()) { - JS_ASSERT(lexdep->frameLevel() <= funbox->level); - break; - } - } - - if (r.empty()) - fun->setKind(JSFUN_NULL_CLOSURE); - } else { - uintN nupvars = 0, nflattened = 0; + if (pn->pn_type == TOK_UPVARS) { + AtomDefnMapPtr upvars = pn->pn_names; + JS_ASSERT(!upvars->empty()); /* * For each lexical dependency from this closure to an outer @@ -2523,51 +2491,39 @@ Parser::setFunctionKinds(JSFunctionBox *funbox, uint32 *tcflags) JSDefinition *lexdep = defn->resolve(); if (!lexdep->isFreeVar()) { - ++nupvars; - if (CanFlattenUpvar(lexdep, funbox, *tcflags)) { - ++nflattened; - continue; + hasUpvars = true; + if (!CanFlattenUpvar(lexdep, funbox, *tcflags)) { + /* + * Can't flatten. Enclosing functions holding + * variables used by this function will be flagged + * heavyweight below. FIXME bug 545759: re-enable + * partial flat closures. + */ + canFlatten = false; + break; } - - /* - * FIXME bug 545759: to test nflattened != 0 instead of - * nflattened == nupvars below, we'll need to avoid n^2 - * bugs such as 617430 in uncommenting the following: - * - * if (DeoptimizeUsesWithin(lexdep, funbox->node->pn_body->pn_pos)) - * FlagHeavyweights(lexdep, funbox, tcflags); - * - * For now it's best to avoid this tedious, use-wise - * deoptimization and let fun remain an unoptimized - * closure. This is safe because we leave fun's kind - * set to interpreted, so all functions holding its - * upvars will be flagged as heavyweight. - */ } } + } - if (nupvars == 0) { - fun->setKind(JSFUN_NULL_CLOSURE); - } else if (nflattened == nupvars) { - /* - * We made it all the way through the upvar loop, so it's - * safe to optimize to a flat closure. - */ - fun->setKind(JSFUN_FLAT_CLOSURE); - switch (PN_OP(fn)) { - case JSOP_DEFFUN: - fn->pn_op = JSOP_DEFFUN_FC; - break; - case JSOP_DEFLOCALFUN: - fn->pn_op = JSOP_DEFLOCALFUN_FC; - break; - case JSOP_LAMBDA: - fn->pn_op = JSOP_LAMBDA_FC; - break; - default: - /* js_EmitTree's case TOK_FUNCTION: will select op. */ - JS_ASSERT(PN_OP(fn) == JSOP_NOP); - } + if (!hasUpvars) { + /* No lexical dependencies => null closure, for best performance. */ + fun->setKind(JSFUN_NULL_CLOSURE); + } else if (canFlatten) { + fun->setKind(JSFUN_FLAT_CLOSURE); + switch (PN_OP(fn)) { + case JSOP_DEFFUN: + fn->pn_op = JSOP_DEFFUN_FC; + break; + case JSOP_DEFLOCALFUN: + fn->pn_op = JSOP_DEFLOCALFUN_FC; + break; + case JSOP_LAMBDA: + fn->pn_op = JSOP_LAMBDA_FC; + break; + default: + /* js_EmitTree's case TOK_FUNCTION: will select op. */ + JS_ASSERT(PN_OP(fn) == JSOP_NOP); } } } @@ -2595,10 +2551,6 @@ Parser::setFunctionKinds(JSFunctionBox *funbox, uint32 *tcflags) if (funbox->joinable()) fun->setJoinable(); - - funbox = funbox->siblings; - if (!funbox) - break; } }