Bug 1091795. Unregress octane-box2d by not treating some cases when we can't generate a baseline stub as unoptimizable accesses. r=jandem
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 30 Oct 2014 17:36:08 -0400
changeset 213244 7d8529406248e2e49c9faa1c569654bb5d0678a2
parent 213243 8ebe9660d12eb15ee8e6226223c95304f35df859
child 213245 aec14e7888f0281086438f28e50a44ad1159b67e
push id27745
push usercbook@mozilla.com
push dateFri, 31 Oct 2014 13:09:12 +0000
treeherdermozilla-central@6bd2071b373f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1091795
milestone36.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
Bug 1091795. Unregress octane-box2d by not treating some cases when we can't generate a baseline stub as unoptimizable accesses. r=jandem In particular, if the access is unoptimizable for temporary reason, like a scripted accessor not having jitcode compiled yet or an accessor being in the nursery, we don't want to permanently mark the access spot unoptimizable. At some point the accessor will gain jitcode or be tenured and then we can optimize the access.
js/src/jit/BaselineIC.cpp
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -3412,18 +3412,18 @@ IsCacheableGetPropReadSlot(JSObject *obj
 
     if (!shape->hasSlot() || !shape->hasDefaultGetter())
         return false;
 
     return true;
 }
 
 static bool
-IsCacheableGetPropCall(JSContext *cx, JSObject *obj, JSObject *holder, Shape *shape, bool *isScripted,
-                       bool isDOMProxy=false)
+IsCacheableGetPropCall(JSContext *cx, JSObject *obj, JSObject *holder, Shape *shape,
+                       bool *isScripted, bool *isTemporarilyUnoptimizable, bool isDOMProxy=false)
 {
     MOZ_ASSERT(isScripted);
 
     if (!shape || !IsCacheableProtoChain(obj, holder, isDOMProxy))
         return false;
 
     if (shape->hasSlot() || shape->hasDefaultGetter())
         return false;
@@ -3434,27 +3434,31 @@ IsCacheableGetPropCall(JSContext *cx, JS
     if (!shape->getterValue().isObject() || !shape->getterObject()->is<JSFunction>())
         return false;
 
     JSFunction *func = &shape->getterObject()->as<JSFunction>();
 
 #ifdef JSGC_GENERATIONAL
     // Information from get prop call ICs may be used directly from Ion code,
     // and should not be nursery allocated.
-    if (IsInsideNursery(holder) || IsInsideNursery(func))
-        return false;
+    if (IsInsideNursery(holder) || IsInsideNursery(func)) {
+        *isTemporarilyUnoptimizable = true;
+        return false;
+    }
 #endif
 
     if (func->isNative()) {
         *isScripted = false;
         return true;
     }
 
-    if (!func->hasJITCode())
-        return false;
+    if (!func->hasJITCode()) {
+        *isTemporarilyUnoptimizable = true;
+        return false;
+    }
 
     *isScripted = true;
     return true;
 }
 
 static bool
 IsCacheableSetPropWriteSlot(JSObject *obj, Shape *oldShape, JSObject *holder, Shape *shape)
 {
@@ -3528,17 +3532,18 @@ IsCacheableSetPropAddSlot(JSContext *cx,
     if (obj->as<NativeObject>().numDynamicSlots() != oldSlots)
         return false;
 
     *protoChainDepth = chainDepth;
     return true;
 }
 
 static bool
-IsCacheableSetPropCall(JSContext *cx, JSObject *obj, JSObject *holder, Shape *shape, Shape* oldShape, bool *isScripted)
+IsCacheableSetPropCall(JSContext *cx, JSObject *obj, JSObject *holder, Shape *shape,
+                       Shape* oldShape, bool *isScripted, bool *isTemporarilyUnoptimizable)
 {
     MOZ_ASSERT(isScripted);
 
     // Currently we only optimize setter calls for setters bound on prototypes.
     if (obj == holder)
         return false;
 
     if (obj->lastProperty() != oldShape) {
@@ -3560,27 +3565,31 @@ IsCacheableSetPropCall(JSContext *cx, JS
     if (!shape->setterValue().isObject() || !shape->setterObject()->is<JSFunction>())
         return false;
 
     JSFunction *func = &shape->setterObject()->as<JSFunction>();
 
 #ifdef JSGC_GENERATIONAL
     // Information from set prop call ICs may be used directly from Ion code,
     // and should not be nursery allocated.
-    if (IsInsideNursery(holder) || IsInsideNursery(func))
-        return false;
+    if (IsInsideNursery(holder) || IsInsideNursery(func)) {
+        *isTemporarilyUnoptimizable = true;
+        return false;
+    }
 #endif
 
     if (func->isNative()) {
         *isScripted = false;
         return true;
     }
 
-    if (!func->hasJITCode())
-        return false;
+    if (!func->hasJITCode()) {
+        *isTemporarilyUnoptimizable = true;
+        return false;
+    }
 
     *isScripted = true;
     return true;
 }
 
 static bool
 LookupNoSuchMethodHandler(JSContext *cx, HandleObject obj, HandleValue id,
                           MutableHandleValue result)
@@ -3812,17 +3821,19 @@ static bool TryAttachNativeGetElemStub(J
         if (!newStub)
             return false;
 
         stub->addNewStub(newStub);
         return true;
     }
 
     bool getterIsScripted = false;
-    if (IsCacheableGetPropCall(cx, obj, holder, shape, &getterIsScripted, /*isDOMProxy=*/false)) {
+    bool isTemporarilyUnoptimizable = false;
+    if (IsCacheableGetPropCall(cx, obj, holder, shape, &getterIsScripted,
+                               &isTemporarilyUnoptimizable, /*isDOMProxy=*/false)) {
         RootedFunction getter(cx, &shape->getterObject()->as<JSFunction>());
 
 #if JS_HAS_NO_SUCH_METHOD
         // It's unlikely that a getter function will be used in callelem locations.
         // Just don't attach stubs in that case to avoid issues with __noSuchMethod__ handling.
         if (isCallElem)
             return true;
 #endif
@@ -4058,16 +4069,19 @@ DoGetElemFallback(JSContext *cx, Baselin
         // But for now we just bail.
         return true;
     }
 
     // Try to attach an optimized stub.
     if (!TryAttachGetElemStub(cx, frame->script(), pc, stub, lhs, rhs, res))
         return false;
 
+    // If we ever add a way to note unoptimizable accesses here, propagate the
+    // isTemporarilyUnoptimizable state from TryAttachNativeGetElemStub to here.
+
     return true;
 }
 
 typedef bool (*DoGetElemFallbackFn)(JSContext *, BaselineFrame *, ICGetElem_Fallback *,
                                     HandleValue, HandleValue, MutableHandleValue);
 static const VMFunction DoGetElemFallbackInfo =
     FunctionInfo<DoGetElemFallbackFn>(DoGetElemFallback, PopValues(2));
 
@@ -5835,17 +5849,19 @@ TryAttachGlobalNameStub(JSContext *cx, H
         stub->addNewStub(newStub);
         return true;
     }
 
     // Try to add a getter stub. We don't handle scripted getters yet; if this
     // changes we need to make sure IonBuilder::getPropTryCommonGetter (which
     // requires a Baseline stub) handles non-outerized this objects correctly.
     bool isScripted;
-    if (IsCacheableGetPropCall(cx, global, current, shape, &isScripted) && !isScripted)
+    bool isTemporarilyUnoptimizable = false;
+    if (IsCacheableGetPropCall(cx, global, current, shape, &isScripted, &isTemporarilyUnoptimizable) &&
+        !isScripted)
     {
         ICStub *monitorStub = stub->fallbackMonitorStub()->firstMonitorStub();
         RootedFunction getter(cx, &shape->getterObject()->as<JSFunction>());
         ICStub *newStub;
         if (current == global) {
             JitSpew(JitSpew_BaselineIC, "  Generating GetName(GlobalName/NativeGetter) stub");
             if (UpdateExistingGetPropCallStubs(stub, ICStub::GetProp_CallNative,
                                                global, JS::NullPtr(), getter)) {
@@ -6015,16 +6031,19 @@ DoGetNameFallback(JSContext *cx, Baselin
     if (js_CodeSpec[*pc].format & JOF_GNAME) {
         if (!TryAttachGlobalNameStub(cx, script, pc, stub, scopeChain.as<GlobalObject>(), name))
             return false;
     } else {
         if (!TryAttachScopeNameStub(cx, script, stub, scopeChain, name))
             return false;
     }
 
+    // If we ever add a way to note unoptimizable accesses here, propagate the
+    // isTemporarilyUnoptimizable state from TryAttachGlobalNameStub to here.
+
     return true;
 }
 
 typedef bool (*DoGetNameFallbackFn)(JSContext *, BaselineFrame *, ICGetName_Fallback *,
                                     HandleObject, MutableHandleValue);
 static const VMFunction DoGetNameFallbackInfo = FunctionInfo<DoGetNameFallbackFn>(DoGetNameFallback);
 
 bool
@@ -6391,19 +6410,21 @@ StripPreliminaryObjectStubs(JSContext *c
             iter.unlink(cx);
     }
 }
 
 static bool
 TryAttachNativeGetPropStub(JSContext *cx, HandleScript script, jsbytecode *pc,
                            ICGetProp_Fallback *stub, HandlePropertyName name,
                            HandleValue val, HandleShape oldShape,
-                           HandleValue res, bool *attached)
+                           HandleValue res, bool *attached,
+                           bool *isTemporarilyUnoptimizable)
 {
     MOZ_ASSERT(!*attached);
+    MOZ_ASSERT(!*isTemporarilyUnoptimizable);
 
     if (!val.isObject())
         return true;
 
     RootedObject obj(cx, &val.toObject());
 
     if (oldShape != obj->lastProperty()) {
         // No point attaching anything, since we know the shape guard will fail
@@ -6454,17 +6475,18 @@ TryAttachNativeGetPropStub(JSContext *cx
             StripPreliminaryObjectStubs(cx, stub);
 
         stub->addNewStub(newStub);
         *attached = true;
         return true;
     }
 
     bool isScripted = false;
-    bool cacheableCall = IsCacheableGetPropCall(cx, obj, holder, shape, &isScripted, isDOMProxy);
+    bool cacheableCall = IsCacheableGetPropCall(cx, obj, holder, shape, &isScripted,
+                                                isTemporarilyUnoptimizable, isDOMProxy);
 
     // Try handling scripted getters.
     if (cacheableCall && isScripted && !isDOMProxy) {
 #if JS_HAS_NO_SUCH_METHOD
         // It's hard to keep the original object alive through a call, and it's unlikely
         // that a getter will be used to generate functions for calling in CALLPROP locations.
         // Just don't attach stubs in that case.
         if (isCallProp)
@@ -6761,33 +6783,38 @@ DoGetPropFallback(JSContext *cx, Baselin
         ICStub *newStub = compiler.getStub(compiler.getStubSpace(frame->script()));
         if (!newStub)
             return false;
         stub->addNewStub(newStub);
         return true;
     }
 
     bool attached = false;
+    // There are some reasons we can fail to attach a stub that are temporary.
+    // We want to avoid calling noteUnoptimizableAccess() if the reason we
+    // failed to attach a stub is one of those temporary reasons, since we might
+    // end up attaching a stub for the exact same access later.
+    bool isTemporarilyUnoptimizable = false;
 
     if (op == JSOP_LENGTH) {
         if (!TryAttachLengthStub(cx, frame->script(), stub, val, res, &attached))
             return false;
         if (attached)
             return true;
     }
 
     if (!TryAttachMagicArgumentsGetPropStub(cx, frame->script(), stub, name, val, res, &attached))
         return false;
     if (attached)
         return true;
 
     RootedScript script(cx, frame->script());
 
     if (!TryAttachNativeGetPropStub(cx, script, pc, stub, name, val, oldShape,
-                                    res, &attached))
+                                    res, &attached, &isTemporarilyUnoptimizable))
         return false;
     if (attached)
         return true;
 
     if (val.isString() || val.isNumber() || val.isBoolean()) {
         if (!TryAttachPrimitiveGetPropStub(cx, script, pc, stub, name, val, res, &attached))
             return false;
         if (attached)
@@ -6798,17 +6825,18 @@ DoGetPropFallback(JSContext *cx, Baselin
         // Try attaching property-not-found optimized stub for undefined results.
         if (!TryAttachNativeDoesNotExistStub(cx, script, pc, stub, name, val, &attached))
             return false;
         if (attached)
             return true;
     }
 
     MOZ_ASSERT(!attached);
-    stub->noteUnoptimizableAccess();
+    if (!isTemporarilyUnoptimizable)
+        stub->noteUnoptimizableAccess();
 
     return true;
 }
 
 typedef bool (*DoGetPropFallbackFn)(JSContext *, BaselineFrame *, ICGetProp_Fallback *,
                                     MutableHandleValue, MutableHandleValue);
 static const VMFunction DoGetPropFallbackInfo =
     FunctionInfo<DoGetPropFallbackFn>(DoGetPropFallback, PopValues(1));
@@ -7694,19 +7722,21 @@ BaselineScript::noteAccessedGetter(uint3
 //
 // SetProp_Fallback
 //
 
 // Attach an optimized stub for a SETPROP/SETGNAME/SETNAME op.
 static bool
 TryAttachSetPropStub(JSContext *cx, HandleScript script, jsbytecode *pc, ICSetProp_Fallback *stub,
                      HandleObject obj, HandleShape oldShape, HandleTypeObject oldType, uint32_t oldSlots,
-                     HandlePropertyName name, HandleId id, HandleValue rhs, bool *attached)
+                     HandlePropertyName name, HandleId id, HandleValue rhs, bool *attached,
+                     bool *isTemporarilyUnoptimizable)
 {
     MOZ_ASSERT(!*attached);
+    MOZ_ASSERT(!*isTemporarilyUnoptimizable);
 
     if (!obj->isNative() || obj->watched())
         return true;
 
     RootedShape shape(cx);
     RootedObject holder(cx);
     if (!EffectlesslyLookupProperty(cx, obj, name, &holder, &shape))
         return false;
@@ -7775,17 +7805,18 @@ TryAttachSetPropStub(JSContext *cx, Hand
             StripPreliminaryObjectStubs(cx, stub);
 
         stub->addNewStub(newStub);
         *attached = true;
         return true;
     }
 
     bool isScripted = false;
-    bool cacheableCall = IsCacheableSetPropCall(cx, obj, holder, shape, oldShape, &isScripted);
+    bool cacheableCall = IsCacheableSetPropCall(cx, obj, holder, shape, oldShape,
+                                                &isScripted, isTemporarilyUnoptimizable);
 
     // Try handling scripted setters.
     if (cacheableCall && isScripted) {
         RootedFunction callee(cx, &shape->setterObject()->as<JSFunction>());
         MOZ_ASSERT(obj != holder);
         MOZ_ASSERT(callee->hasScript());
 
         if (UpdateExistingSetPropCallStubs(stub, ICStub::SetProp_CallScripted,
@@ -7901,26 +7932,32 @@ DoSetPropFallback(JSContext *cx, Baselin
         return true;
 
     if (stub->numOptimizedStubs() >= ICSetProp_Fallback::MAX_OPTIMIZED_STUBS) {
         // TODO: Discard all stubs in this IC and replace with generic setprop stub.
         return true;
     }
 
     bool attached = false;
+    // There are some reasons we can fail to attach a stub that are temporary.
+    // We want to avoid calling noteUnoptimizableAccess() if the reason we
+    // failed to attach a stub is one of those temporary reasons, since we might
+    // end up attaching a stub for the exact same access later.
+    bool isTemporarilyUnoptimizable = false;
     if (!TryAttachSetPropStub(cx, script, pc, stub, obj, oldShape, oldType, oldSlots,
-                              name, id, rhs, &attached))
+                              name, id, rhs, &attached, &isTemporarilyUnoptimizable))
     {
         return false;
     }
     if (attached)
         return true;
 
     MOZ_ASSERT(!attached);
-    stub->noteUnoptimizableAccess();
+    if (!isTemporarilyUnoptimizable)
+        stub->noteUnoptimizableAccess();
 
     return true;
 }
 
 typedef bool (*DoSetPropFallbackFn)(JSContext *, BaselineFrame *, ICSetProp_Fallback *,
                                     HandleValue, HandleValue, MutableHandleValue);
 static const VMFunction DoSetPropFallbackInfo =
     FunctionInfo<DoSetPropFallbackFn>(DoSetPropFallback, PopValues(2));