Implement method thrash counting, at limit unbrand and stick count to limit; also, brand and unbrand return false on shape overflow, so call them that way instead of as if infallible (597864, r=dvander).
authorBrendan Eich <brendan@mozilla.org>
Mon, 22 Nov 2010 17:58:53 -0800
changeset 59227 1002cba2f2d6e7e8518546dfbc868468758ac9e0
parent 59226 fb3b0fd656bf7b9b33aec7405515781bf3b2e878
child 59228 e1ef77fd860547d59834fe53b21411e674ab2324
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersdvander
bugs597864
milestone2.0b8pre
Implement method thrash counting, at limit unbrand and stick count to limit; also, brand and unbrand return false on shape overflow, so call them that way instead of as if infallible (597864, r=dvander).
js/src/jsinterp.cpp
js/src/jsobj.cpp
js/src/jsobj.h
js/src/jsobjinlines.h
js/src/jsscope.cpp
js/src/methodjit/StubCalls.cpp
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -4070,18 +4070,18 @@ END_CASE(JSOP_THIS)
 
 BEGIN_CASE(JSOP_UNBRANDTHIS)
 {
     if (!regs.fp->computeThis(cx))
         goto error;
     Value &thisv = regs.fp->thisValue();
     if (thisv.isObject()) {
         JSObject *obj = &thisv.toObject();
-        if (obj->isNative() && !obj->unbrand(cx))
-            goto error;
+        if (obj->isNative())
+            obj->unbrand(cx);
     }
 }
 END_CASE(JSOP_UNBRANDTHIS)
 
 {
     JSObject *obj;
     Value *vp;
     jsint i;
@@ -4285,18 +4285,17 @@ BEGIN_CASE(JSOP_CALLPROP)
             goto error;
     }
 #endif
 }
 END_CASE(JSOP_CALLPROP)
 
 BEGIN_CASE(JSOP_UNBRAND)
     JS_ASSERT(regs.sp - regs.fp->slots() >= 1);
-    if (!regs.sp[-1].toObject().unbrand(cx))
-        goto error;
+    regs.sp[-1].toObject().unbrand(cx);
 END_CASE(JSOP_UNBRAND)
 
 BEGIN_CASE(JSOP_SETGNAME)
 BEGIN_CASE(JSOP_SETNAME)
 BEGIN_CASE(JSOP_SETPROP)
 BEGIN_CASE(JSOP_SETMETHOD)
 {
     Value rval = regs.sp[-1];
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -3830,20 +3830,20 @@ js_InitClass(JSContext *cx, JSObject *ob
         (static_fs && !JS_DefineFunctions(cx, ctor, static_fs))) {
         goto bad;
     }
 
     /*
      * Pre-brand the prototype and constructor if they have built-in methods.
      * This avoids extra shape guard branch exits in the tracejitted code.
      */
-    if (fs && !proto->brand(cx))
-        goto bad;
-    if (ctor != proto && static_fs && !ctor->brand(cx))
-        goto bad;
+    if (fs)
+        proto->brand(cx);
+    if (ctor != proto && static_fs)
+        ctor->brand(cx);
 
     /*
      * Make sure proto's emptyShape is available to be shared by objects of
      * this class.  JSObject::emptyShape is a one-slot cache. If we omit this,
      * some other class could snap it up. (The risk is particularly great for
      * Object.prototype.)
      *
      * All callers of JSObject::initSharingEmptyShape depend on this.
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -332,26 +332,29 @@ struct JSObject : js::gc::Cell {
 
     inline js::Shape **nativeSearch(jsid id, bool adding = false);
     inline const js::Shape *nativeLookup(jsid id);
 
     inline bool nativeContains(jsid id);
     inline bool nativeContains(const js::Shape &shape);
 
     enum {
-        DELEGATE        = 0x01,
-        SYSTEM          = 0x02,
-        NOT_EXTENSIBLE  = 0x04,
-        BRANDED         = 0x08,
-        GENERIC         = 0x10,
-        METHOD_BARRIER  = 0x20,
-        INDEXED         = 0x40,
-        OWN_SHAPE       = 0x80,
-        BOUND_FUNCTION  = 0x100,
-        HAS_EQUALITY    = 0x200
+        DELEGATE                  =  0x01,
+        SYSTEM                    =  0x02,
+        NOT_EXTENSIBLE            =  0x04,
+        BRANDED                   =  0x08,
+        GENERIC                   =  0x10,
+        METHOD_BARRIER            =  0x20,
+        INDEXED                   =  0x40,
+        OWN_SHAPE                 =  0x80,
+        BOUND_FUNCTION            = 0x100,
+        HAS_EQUALITY              = 0x200,
+        METHOD_THRASH_COUNT_MASK  = 0xc00,
+        METHOD_THRASH_COUNT_SHIFT =    10,
+        METHOD_THRASH_COUNT_MAX   = METHOD_THRASH_COUNT_MASK >> METHOD_THRASH_COUNT_SHIFT
     };
 
     /*
      * Impose a sane upper bound, originally checked only for dense arrays, on
      * number of slots in an object.
      */
     enum {
         NSLOTS_BITS     = 29,
@@ -418,22 +421,36 @@ struct JSObject : js::gc::Cell {
 
     /*
      * A branded object contains plain old methods (function-valued properties
      * without magic getters and setters), and its shape evolves whenever a
      * function value changes.
      */
     bool branded()              { return !!(flags & BRANDED); }
 
+    /*
+     * NB: these return false on shape overflow but do not report any error.
+     * Callers who depend on shape guarantees should therefore bail off trace,
+     * e.g., on false returns.
+     */
     bool brand(JSContext *cx);
     bool unbrand(JSContext *cx);
 
     bool generic()              { return !!(flags & GENERIC); }
     void setGeneric()           { flags |= GENERIC; }
 
+    uintN getMethodThrashCount() const {
+        return (flags & METHOD_THRASH_COUNT_MASK) >> METHOD_THRASH_COUNT_SHIFT;
+    }
+
+    void setMethodThrashCount(uintN count) {
+        JS_ASSERT(count <= METHOD_THRASH_COUNT_MAX);
+        flags = (flags & ~METHOD_THRASH_COUNT_MASK) | (count << METHOD_THRASH_COUNT_SHIFT);
+    }
+
     bool hasSpecialEquality() const { return !!(flags & HAS_EQUALITY); }
     void assertSpecialEqualitySynced() const {
         JS_ASSERT(!!clasp->ext.equality == hasSpecialEquality());
     }
 
     /* Sets an object's HAS_EQUALITY flag based on its clasp. */
     inline void syncSpecialEquality();
 
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -100,18 +100,23 @@ JSObject::brand(JSContext *cx)
     flags |= BRANDED;
     return true;
 }
 
 inline bool
 JSObject::unbrand(JSContext *cx)
 {
     JS_ASSERT(isNative());
-    if (!branded())
-        setGeneric();
+    if (branded()) {
+        generateOwnShape(cx);
+        if (js_IsPropertyCacheDisabled(cx))  // check for rt->shapeGen overflow
+            return false;
+        flags &= ~BRANDED;
+    }
+    setGeneric();
     return true;
 }
 
 inline void
 JSObject::syncSpecialEquality()
 {
     if (clasp->ext.equality)
         flags |= JSObject::HAS_EQUALITY;
@@ -184,33 +189,31 @@ ChangesMethodValue(const js::Value &prev
     JSObject *prevObj;
     return prev.isObject() && (prevObj = &prev.toObject())->isFunction() &&
            (!v.isObject() || &v.toObject() != prevObj);
 }
 
 inline bool
 JSObject::methodWriteBarrier(JSContext *cx, const js::Shape &shape, const js::Value &v)
 {
-    if (flags & (BRANDED | METHOD_BARRIER)) {
-        if (shape.slot != SHAPE_INVALID_SLOT) {
-            const js::Value &prev = nativeGetSlot(shape.slot);
+    if (brandedOrHasMethodBarrier() && shape.slot != SHAPE_INVALID_SLOT) {
+        const js::Value &prev = nativeGetSlot(shape.slot);
 
-            if (ChangesMethodValue(prev, v)) {
-                JS_FUNCTION_METER(cx, mwritebarrier);
-                return methodShapeChange(cx, shape);
-            }
+        if (ChangesMethodValue(prev, v)) {
+            JS_FUNCTION_METER(cx, mwritebarrier);
+            return methodShapeChange(cx, shape);
         }
     }
     return true;
 }
 
 inline bool
 JSObject::methodWriteBarrier(JSContext *cx, uint32 slot, const js::Value &v)
 {
-    if (flags & (BRANDED | METHOD_BARRIER)) {
+    if (brandedOrHasMethodBarrier()) {
         const js::Value &prev = nativeGetSlot(slot);
 
         if (ChangesMethodValue(prev, v)) {
             JS_FUNCTION_METER(cx, mwslotbarrier);
             return methodShapeChange(cx, slot);
         }
     }
     return true;
--- a/js/src/jsscope.cpp
+++ b/js/src/jsscope.cpp
@@ -1380,16 +1380,28 @@ JSObject::methodShapeChange(JSContext *c
         if (!putProperty(cx, shape.id, NULL, shape.rawSetter, shape.slot,
                          shape.attrs,
                          shape.getFlags() & ~Shape::METHOD,
                          shape.shortid)) {
             return false;
         }
     }
 
+    if (branded()) {
+        uintN thrashCount = getMethodThrashCount();
+        if (thrashCount < JSObject::METHOD_THRASH_COUNT_MAX) {
+            ++thrashCount;
+            setMethodThrashCount(thrashCount);
+            if (thrashCount == JSObject::METHOD_THRASH_COUNT_MAX) {
+                unbrand(cx);
+                return true;
+            }
+        }
+    }
+
     generateOwnShape(cx);
     return true;
 }
 
 bool
 JSObject::methodShapeChange(JSContext *cx, uint32 slot)
 {
     if (!hasMethodBarrier()) {
--- a/js/src/methodjit/StubCalls.cpp
+++ b/js/src/methodjit/StubCalls.cpp
@@ -2569,18 +2569,18 @@ finally:
 
 void JS_FASTCALL
 stubs::Unbrand(VMFrame &f)
 {
     const Value &thisv = f.regs.sp[-1];
     if (!thisv.isObject())
         return;
     JSObject *obj = &thisv.toObject();
-    if (obj->isNative() && !obj->unbrand(f.cx))
-        THROW();
+    if (obj->isNative())
+        obj->unbrand(f.cx);
 }
 
 void JS_FASTCALL
 stubs::Pos(VMFrame &f)
 {
     if (!ValueToNumber(f.cx, &f.regs.sp[-1]))
         THROW();
 }