Use distinct types and scripts for generic inner wrapper functions, bug 638794. r=jandem
authorBrian Hackett <bhackett1024@gmail.com>
Wed, 18 Jul 2012 06:35:00 -0600
changeset 99659 120e3d9d86f118582506db60bca3028d3d8468a8
parent 99658 90ff0bdfdc2c1ee178fcd8019f468d48cf926198
child 99660 9a11f086069adeb0524c223cce8289950ecbff56
push id23146
push useremorley@mozilla.com
push dateThu, 19 Jul 2012 12:27:27 +0000
treeherdermozilla-central@e1dcf7c892d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs638794
milestone17.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Use distinct types and scripts for generic inner wrapper functions, bug 638794. r=jandem
js/src/jsfun.cpp
js/src/jsinfer.cpp
js/src/jsinferinlines.h
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -1263,53 +1263,58 @@ js_CloneFunctionObject(JSContext *cx, Ha
     }
     clone->atom.init(fun->atom);
 
     if (kind == JSFunction::ExtendedFinalizeKind) {
         clone->flags |= JSFUN_EXTENDED;
         clone->initializeExtended();
     }
 
-    if (cx->compartment == fun->compartment()) {
+    if (cx->compartment == fun->compartment() && !types::UseNewTypeForClone(fun)) {
         /*
          * We can use the same type as the original function provided that (a)
          * its prototype is correct, and (b) its type is not a singleton. The
          * first case will hold in all compileAndGo code, and the second case
          * will have been caught by CloneFunctionObject coming from function
          * definitions or read barriers, so will not get here.
          */
         if (fun->getProto() == proto && !fun->hasSingletonType())
             clone->setType(fun->type());
     } else {
+        if (!clone->setSingletonType(cx))
+            return NULL;
+
         /*
          * Across compartments we have to clone the script for interpreted
          * functions. Cross-compartment cloning only happens via JSAPI
          * (JS_CloneFunctionObject) which dynamically ensures that 'script' has
          * no enclosing lexical scope (only the global scope).
          */
         if (clone->isInterpreted()) {
             RootedScript script(cx, clone->script());
             JS_ASSERT(script);
             JS_ASSERT(script->compartment() == fun->compartment());
-            JS_ASSERT(script->compartment() != cx->compartment);
-            JS_ASSERT(!script->enclosingStaticScope());
+            JS_ASSERT_IF(script->compartment() != cx->compartment,
+                         !script->enclosingStaticScope() && !script->compileAndGo);
+
+            RootedObject scope(cx, script->enclosingStaticScope());
 
             clone->mutableScript().init(NULL);
 
-            JSScript *cscript = CloneScript(cx, NullPtr(), clone, script);
+            JSScript *cscript = CloneScript(cx, scope, clone, script);
             if (!cscript)
                 return NULL;
 
             clone->setScript(cscript);
             cscript->setFunction(clone);
-            if (!clone->setTypeForScriptedFunction(cx))
-                return NULL;
+
+            GlobalObject *global = script->compileAndGo ? &script->global() : NULL;
 
             js_CallNewScriptHook(cx, clone->script(), clone);
-            Debugger::onNewScript(cx, clone->script(), NULL);
+            Debugger::onNewScript(cx, clone->script(), global);
         }
     }
     return clone;
 }
 
 JSFunction *
 js_DefineFunction(JSContext *cx, HandleObject obj, HandleId id, Native native,
                   unsigned nargs, unsigned attrs, AllocKind kind)
--- a/js/src/jsinfer.cpp
+++ b/js/src/jsinfer.cpp
@@ -3624,17 +3624,17 @@ ScriptAnalysis::analyzeTypesBytecode(JSC
       case JSOP_DEFFUN: {
         JSObject *obj = script->getObject(GET_UINT32_INDEX(pc));
 
         TypeSet *res = NULL;
         if (op == JSOP_LAMBDA)
             res = &pushed[0];
 
         if (res) {
-            if (script->hasGlobal())
+            if (script->hasGlobal() && !UseNewTypeForClone(obj->toFunction()))
                 res->addType(cx, Type::ObjectType(obj));
             else
                 res->addType(cx, Type::UnknownType());
         } else {
             cx->compartment->types.monitorBytecode(cx, script, offset);
         }
         break;
       }
@@ -3921,22 +3921,24 @@ ScriptAnalysis::analyzeTypesBytecode(JSC
       case JSOP_ANYNAME:
       case JSOP_GETFUNNS:
       case JSOP_FILTER:
         /* Note: the second value pushed by filter is a hole, and not modelled. */
       case JSOP_ENDFILTER:
         pushed[0].addType(cx, Type::UnknownType());
         break;
 
-      case JSOP_CALLEE:
-        if (script->hasGlobal())
-            pushed[0].addType(cx, Type::ObjectType(script->function()));
+      case JSOP_CALLEE: {
+        JSFunction *fun = script->function();
+        if (script->hasGlobal() && !UseNewTypeForClone(fun))
+            pushed[0].addType(cx, Type::ObjectType(fun));
         else
             pushed[0].addType(cx, Type::UnknownType());
         break;
+      }
 
       default:
         /* Display fine-grained debug information first */
         fprintf(stderr, "Unknown bytecode %02x at #%u:%05u\n", op, script->id(), offset);
         TypeFailure(cx, "Unknown bytecode %02x", op);
     }
 
     return true;
@@ -5035,16 +5037,21 @@ JSFunction::setTypeForScriptedFunction(J
     JS_ASSERT(script()->function() == this);
 
     if (!cx->typeInferenceEnabled())
         return true;
 
     if (singleton) {
         if (!setSingletonType(cx))
             return false;
+    } else if (UseNewTypeForClone(this)) {
+        /*
+         * Leave the default unknown-properties type for the function, it
+         * should not be used by scripts or appear in type sets.
+         */
     } else {
         RootedFunction self(cx, this);
 
         TypeObject *type = cx->compartment->types.newTypeObject(cx, script(),
                                                                 JSProto_Function, getProto());
         if (!type)
             return false;
 
--- a/js/src/jsinferinlines.h
+++ b/js/src/jsinferinlines.h
@@ -484,16 +484,76 @@ extern void TypeDynamicResult(JSContext 
 inline bool
 UseNewTypeAtEntry(JSContext *cx, StackFrame *fp)
 {
     return fp->isConstructing() && cx->typeInferenceEnabled() &&
            fp->prev() && fp->prev()->isScriptFrame() &&
            UseNewType(cx, fp->prev()->script(), fp->prevpc());
 }
 
+inline bool
+UseNewTypeForClone(JSFunction *fun)
+{
+    if (fun->hasSingletonType() || !fun->isInterpreted())
+        return false;
+
+    /*
+     * When a function is being used as a wrapper for another function, it
+     * improves precision greatly to distinguish between different instances of
+     * the wrapper; otherwise we will conflate much of the information about
+     * the wrapped functions.
+     *
+     * An important example is the Class.create function at the core of the
+     * Prototype.js library, which looks like:
+     *
+     * var Class = {
+     *   create: function() {
+     *     return function() {
+     *       this.initialize.apply(this, arguments);
+     *     }
+     *   }
+     * };
+     *
+     * Each instance of the innermost function will have a different wrapped
+     * initialize method. We capture this, along with similar cases, by looking
+     * for short scripts which use both .apply and arguments. For such scripts,
+     * whenever creating a new instance of the function we both give that
+     * instance a singleton type and clone the underlying script.
+     */
+
+    JSScript *script = fun->script();
+
+    if (script->length >= 50)
+        return false;
+
+    if (script->hasConsts() ||
+        script->hasObjects() ||
+        script->hasRegexps() ||
+        script->hasClosedArgs() ||
+        script->hasClosedVars())
+    {
+        return false;
+    }
+
+    bool hasArguments = false;
+    bool hasApply = false;
+
+    for (jsbytecode *pc = script->code;
+         pc != script->code + script->length;
+         pc += GetBytecodeLength(pc))
+    {
+        if (*pc == JSOP_ARGUMENTS)
+            hasArguments = true;
+        if (*pc == JSOP_FUNAPPLY)
+            hasApply = true;
+    }
+
+    return hasArguments && hasApply;
+}
+
 /////////////////////////////////////////////////////////////////////
 // Script interface functions
 /////////////////////////////////////////////////////////////////////
 
 /* static */ inline unsigned
 TypeScript::NumTypeSets(JSScript *script)
 {
     return script->nTypeSets + analyze::TotalSlots(script);