Backed out changeset d1a4ee3d0c59 due to build fail, test fail, and perf regressions.
authorAndreas Gal <gal@mozilla.com>
Wed, 15 Apr 2009 14:20:52 -0700
changeset 24932 630261e7e9a73fd6901826a12678e6c07d510e4e
parent 24931 6f105eb31c26e07ac2e45b6d171d463ff5474123
child 24933 446304c18569f769faf9265b7749f65d36ce42a7
push id1267
push userrsayre@mozilla.com
push dateSun, 19 Apr 2009 02:47:24 +0000
milestone1.9.1b4pre
Backed out changeset d1a4ee3d0c59 due to build fail, test fail, and perf regressions.
js/src/jsarray.cpp
js/src/jsbuiltins.cpp
js/src/jsinterp.cpp
js/src/jsinterp.h
js/src/jsobj.cpp
js/src/jsobj.h
js/src/jsscope.cpp
js/src/jsscope.h
js/src/jstracer.cpp
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -788,19 +788,18 @@ array_getProperty(JSContext *cx, JSObjec
                                        &obj2, &prop) < 0)
             return JS_FALSE;
 
         if (prop) {
             if (OBJ_IS_NATIVE(obj2)) {
                 sprop = (JSScopeProperty *) prop;
                 if (!js_NativeGet(cx, obj, obj2, sprop, vp))
                     return JS_FALSE;
-            } else {
-                OBJ_DROP_PROPERTY(cx, obj2, prop);
             }
+            OBJ_DROP_PROPERTY(cx, obj2, prop);
         }
         return JS_TRUE;
     }
 
     *vp = obj->dslots[i];
     return JS_TRUE;
 }
 
@@ -1287,17 +1286,17 @@ js_MakeArraySlow(JSContext *cx, JSObject
 
         if (obj->dslots[i] == JSVAL_HOLE) {
             obj->dslots[i] = JSVAL_VOID;
             continue;
         }
 
         sprop = js_AddScopeProperty(cx, (JSScope *)map, id, NULL, NULL,
                                     i + JS_INITIAL_NSLOTS, JSPROP_ENUMERATE,
-                                    0, 0, NULL);
+                                    0, 0);
         if (!sprop)
             goto out_bad;
     }
 
     /*
      * Render our formerly-reserved count property GC-safe. If length fits in
      * a jsval, set our slow/sparse COUNT to the current length as a jsval, so
      * we can tell when only named properties have been added to a dense array
--- a/js/src/jsbuiltins.cpp
+++ b/js/src/jsbuiltins.cpp
@@ -269,18 +269,17 @@ js_AddProperty(JSContext* cx, JSObject* 
         ++scope->entryCount;
         scope->lastProp = sprop;
         JS_UNLOCK_SCOPE(cx, scope);
         return JS_TRUE;
     }
 
     sprop2 = js_AddScopeProperty(cx, scope, sprop->id,
                                  sprop->getter, sprop->setter, SPROP_INVALID_SLOT,
-                                 sprop->attrs, sprop->flags, sprop->shortid,
-                                 NULL);
+                                 sprop->attrs, sprop->flags, sprop->shortid);
     if (sprop2 == sprop) {
         JS_UNLOCK_SCOPE(cx, scope);
         return JS_TRUE;
     }
     slot = sprop2->slot;
 
   slot_changed:
     JS_UNLOCK_SCOPE(cx, scope);
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -106,50 +106,60 @@ js_GenerateShape(JSContext *cx, JSBool g
         JS_ASSERT(shape != 0);
         JS_ASSERT_IF(shape & SHAPE_OVERFLOW_BIT,
                      JS_PROPERTY_CACHE(cx).disabled);
     }
     return shape;
 }
 
 JS_REQUIRES_STACK void
-js_FillPropertyCache(JSContext *cx, JSObject *obj,
+js_FillPropertyCache(JSContext *cx, JSObject *obj, jsuword kshape,
                      uintN scopeIndex, uintN protoIndex,
                      JSObject *pobj, JSScopeProperty *sprop,
-                     JSBool cacheByPrevShape, JSPropCacheEntry **entryp)
+                     JSPropCacheEntry **entryp)
 {
     JSPropertyCache *cache;
     jsbytecode *pc;
     JSScope *scope;
-    jsuword kshape, khash;
     JSOp op;
     const JSCodeSpec *cs;
     jsuword vword;
     ptrdiff_t pcoff;
+    jsuword khash;
     JSAtom *atom;
     JSPropCacheEntry *entry;
 
-    JS_ASSERT(OBJ_SCOPE(pobj)->object == pobj);
-    JS_ASSERT(SCOPE_HAS_PROPERTY(OBJ_SCOPE(pobj), sprop));
     JS_ASSERT(!cx->runtime->gcRunning);
-
     cache = &JS_PROPERTY_CACHE(cx);
     pc = cx->fp->regs->pc;
     if (cache->disabled || (cx->fp->flags & JSFRAME_EVAL)) {
         PCMETER(cache->disfills++);
         *entryp = NULL;
         return;
     }
 
     /*
-     * Check for overdeep scope and prototype chain. Because resolve hooks
-     * that js_LookupPropertyWithFlags runs can change the prototype chain
-     * using JS_SetPrototype, we have to validate protoIndex if it is
-     * non-zero. If it is zero, then we know from the fact that obj == pobj
-     * that protoIndex is invariant.
+     * Check for fill from js_SetPropertyHelper where the setter removed sprop
+     * from pobj's scope (via unwatch or delete, e.g.).
+     */
+    scope = OBJ_SCOPE(pobj);
+    JS_ASSERT(scope->object == pobj);
+    if (!SCOPE_HAS_PROPERTY(scope, sprop)) {
+        PCMETER(cache->oddfills++);
+        *entryp = NULL;
+        return;
+    }
+
+    /*
+     * Check for overdeep scope and prototype chain. Because resolve, getter,
+     * and setter hooks can change the prototype chain using JS_SetPrototype
+     * after js_LookupPropertyWithFlags has returned the nominal protoIndex,
+     * we have to validate protoIndex if it is non-zero. If it is zero, then
+     * we know thanks to the SCOPE_HAS_PROPERTY test above, and from the fact
+     * that obj == pobj, that protoIndex is invariant.
      *
      * The scopeIndex can't be wrong. We require JS_SetParent calls to happen
      * before any running script might consult a parent-linked scope chain. If
      * this requirement is not satisfied, the fill in progress will never hit,
      * but vcap vs. scope shape tests ensure nothing malfunctions.
      */
     JS_ASSERT_IF(scopeIndex == 0 && protoIndex == 0, obj == pobj);
     if (protoIndex != 0) {
@@ -183,18 +193,17 @@ js_FillPropertyCache(JSContext *cx, JSOb
     }
 
     /*
      * Optimize the cached vword based on our parameters and the current pc's
      * opcode format flags.
      */
     op = js_GetOpcode(cx, cx->fp->script, pc);
     cs = &js_CodeSpec[op];
-    scope = OBJ_SCOPE(pobj);
-    kshape = 0;
+
     do {
         /*
          * Check for a prototype "plain old method" callee computation. What
          * is a plain old method? It's a function-valued property with stub
          * getter and setter, so get of a function is idempotent and set is
          * transparent.
          */
         if (cs->format & JOF_CALLOP) {
@@ -220,86 +229,58 @@ js_FillPropertyCache(JSContext *cx, JSOb
                         PCMETER(cache->brandfills++);
 #ifdef DEBUG_notme
                         fprintf(stderr,
                             "branding %p (%s) for funobj %p (%s), kshape %lu\n",
                             pobj, LOCKED_OBJ_GET_CLASS(pobj)->name,
                             JSVAL_TO_OBJECT(v),
                             JS_GetFunctionName(GET_FUNCTION_PRIVATE(cx,
                                                  JSVAL_TO_OBJECT(v))),
-                            OBJ_SHAPE(obj));
+                            kshape);
 #endif
                         SCOPE_MAKE_UNIQUE_SHAPE(cx, scope);
-                        if (cache->disabled) {
-                            /*
-                             * js_GenerateShape could not recover from shape's
-                             * overflow.
-                             */
-                            *entryp = NULL;
-                            return;
-                        }
                         SCOPE_SET_BRANDED(scope);
+                        if (OBJ_SCOPE(obj) == scope)
+                            kshape = scope->shape;
                     }
                     vword = JSVAL_OBJECT_TO_PCVAL(v);
                     break;
                 }
             }
         }
 
         /* If getting a value via a stub getter, we can cache the slot. */
         if (!(cs->format & (JOF_SET | JOF_INCDEC | JOF_FOR)) &&
             SPROP_HAS_STUB_GETTER(sprop) &&
             SPROP_HAS_VALID_SLOT(sprop, scope)) {
             /* Great, let's cache sprop's slot and use it on cache hit. */
             vword = SLOT_TO_PCVAL(sprop->slot);
         } else {
             /* Best we can do is to cache sprop (still a nice speedup). */
             vword = SPROP_TO_PCVAL(sprop);
-            if (cacheByPrevShape) {
-                /*
-                 * Our caller just called js_AddScopeProperty and added a new
-                 * property. We want to cache under the shape that was prior
-                 * the call to bias for the case when the mutator always adds
-                 * the same property. It allows to optimize periodic execution
-                 * of object initializers or explicit initialization sequences
-                 * like
-                 *
-                 *   obj = {}; obj.x = 1; obj.y = 2;
-                 *
-                 * We assume that on average the win from this optimization is
-                 * bigger that the cost of an extra mismatch per loop due to
-                 * the bias for the following case:
-                 *
-                 *   obj = {}; ... for (...) { ... obj.x = ... }
-                 *
-                 * Here on the first iteration JSOP_SETPROP fills the cache
-                 * with the shape of newly created object, not the shape after
-                 * obj.x is assigned. That mismatches obj's shape on the
-                 * second iteration. Note that on third and the following
-                 * iterations the cache will be hit since the shape no longer
-                 * mutates.
-                 */
-                JS_ASSERT(scope->object == obj);
-                JS_ASSERT(sprop == scope->lastProp);
-                if (sprop->parent) {
-                    kshape = sprop->parent->shape;
-                } else {
-                    JSObject *proto = STOBJ_GET_PROTO(obj);
-                    if (proto && OBJ_IS_NATIVE(proto))
-                        kshape = OBJ_SHAPE(proto);
-                }
-            }
         }
     } while (0);
 
-    if (kshape == 0)
-        kshape = OBJ_SHAPE(obj);
+    /*
+     * Our caller preserved the scope shape prior to the js_GetPropertyHelper
+     * or similar call out of the interpreter. We want to cache under that
+     * shape if op is overtly mutating, to bias for the case where the mutator
+     * udpates shape predictably.
+     *
+     * Note that an apparently non-mutating op such as JSOP_NAME may still
+     * mutate the base object via, e.g., lazy standard class initialization,
+     * but that is a one-time event and we'll have to miss the old shape and
+     * re-fill under the new one.
+     */
+    if (!(cs->format & (JOF_SET | JOF_INCDEC)) && obj == pobj)
+        kshape = scope->shape;
+
     khash = PROPERTY_CACHE_HASH_PC(pc, kshape);
     if (obj == pobj) {
-        JS_ASSERT(kshape != 0);
+        JS_ASSERT(kshape != 0 || scope->shape != 0);
         JS_ASSERT(scopeIndex == 0 && protoIndex == 0);
         JS_ASSERT(OBJ_SCOPE(obj)->object == obj);
     } else {
         if (op == JSOP_LENGTH) {
             atom = cx->runtime->atomState.lengthAtom;
         } else {
             pcoff = (JOF_TYPE(cs->format) == JOF_SLOTATOM) ? 2 : 0;
             GET_ATOM_FROM_BYTECODE(cx->fp->script, pc, pcoff, atom);
@@ -484,16 +465,17 @@ js_PurgePropertyCache(JSContext *cx, JSP
 #endif
         fprintf(fp, "GC %u\n", cx->runtime->gcNumber);
 
 # define P(mem) fprintf(fp, "%11s %10lu\n", #mem, (unsigned long)cache->mem)
         P(fills);
         P(nofills);
         P(rofills);
         P(disfills);
+        P(oddfills);
         P(modfills);
         P(brandfills);
         P(noprotos);
         P(longchains);
         P(recycles);
         P(pcrecycles);
         P(tests);
         P(pchits);
@@ -2633,26 +2615,16 @@ JS_STATIC_ASSERT(JSOP_SETNAME_LENGTH == 
 JS_STATIC_ASSERT(JSOP_IFNE_LENGTH == JSOP_IFEQ_LENGTH);
 JS_STATIC_ASSERT(JSOP_IFNE == JSOP_IFEQ + 1);
 
 /* For the fastest case inder JSOP_INCNAME, etc. */
 JS_STATIC_ASSERT(JSOP_INCNAME_LENGTH == JSOP_DECNAME_LENGTH);
 JS_STATIC_ASSERT(JSOP_INCNAME_LENGTH == JSOP_NAMEINC_LENGTH);
 JS_STATIC_ASSERT(JSOP_INCNAME_LENGTH == JSOP_NAMEDEC_LENGTH);
 
-#ifdef JS_TRACER
-# define ABORT_RECORDING(cx, reason)                                          \
-    JS_BEGIN_MACRO                                                            \
-        if (TRACE_RECORDER(cx))                                               \
-            js_AbortRecording(cx, reason);                                    \
-    JS_END_MACRO
-#else
-# define ABORT_RECORDING(cx, reason)    ((void 0))
-#endif
-
 JS_REQUIRES_STACK JSBool
 js_Interpret(JSContext *cx)
 {
     JSRuntime *rt;
     JSStackFrame *fp;
     JSScript *script;
     uintN inlineCallCount;
     JSAtom **atoms;
@@ -3039,17 +3011,20 @@ js_Interpret(JSContext *cx)
         switch (switchOp) {
           case -1:
             JS_ASSERT(switchMask == -1);
 #endif /* !JS_THREADED_INTERP */
           {
             bool moreInterrupts = false;
             JSTrapHandler handler = cx->debugHooks->interruptHandler;
             if (handler) {
-                ABORT_RECORDING(cx, "interrupt handler");
+#ifdef JS_TRACER
+                if (TRACE_RECORDER(cx))
+                    js_AbortRecording(cx, "interrupt handler");
+#endif
                 switch (handler(cx, script, regs.pc, &rval,
                                 cx->debugHooks->interruptHandlerData)) {
                   case JSTRAP_ERROR:
                     goto error;
                   case JSTRAP_CONTINUE:
                     break;
                   case JSTRAP_RETURN:
                     fp->rval = rval;
@@ -3579,35 +3554,31 @@ js_Interpret(JSContext *cx)
     JS_BEGIN_MACRO                                                            \
         if (SPROP_HAS_STUB_GETTER(sprop)) {                                   \
             /* Fast path for Object instance properties. */                   \
             JS_ASSERT((sprop)->slot != SPROP_INVALID_SLOT ||                  \
                       !SPROP_HAS_STUB_SETTER(sprop));                         \
             *vp = ((sprop)->slot != SPROP_INVALID_SLOT)                       \
                   ? LOCKED_OBJ_GET_SLOT(pobj, (sprop)->slot)                  \
                   : JSVAL_VOID;                                               \
-            JS_UNLOCK_OBJ(cx, pobj);                                          \
         } else {                                                              \
             if (!js_NativeGet(cx, obj, pobj, sprop, vp))                      \
                 goto error;                                                   \
         }                                                                     \
     JS_END_MACRO
 
-#define NATIVE_SET(cx,obj,entry,sprop,vp)                                     \
+#define NATIVE_SET(cx,obj,sprop,vp)                                           \
     JS_BEGIN_MACRO                                                            \
         if (SPROP_HAS_STUB_SETTER(sprop) &&                                   \
             (sprop)->slot != SPROP_INVALID_SLOT) {                            \
             /* Fast path for, e.g., Object instance properties. */            \
             LOCKED_OBJ_WRITE_BARRIER(cx, obj, (sprop)->slot, *vp);            \
-            JS_UNLOCK_OBJ(cx, obj);                                           \
-            TRACE_2(SetPropHit, entry, sprop);                                \
         } else {                                                              \
             if (!js_NativeSet(cx, obj, sprop, vp))                            \
                 goto error;                                                   \
-            ABORT_RECORDING(cx, "non-stub setter");                           \
         }                                                                     \
     JS_END_MACRO
 
 /*
  * Skip the JSOP_POP typically found after a JSOP_SET* opcode, where oplen is
  * the constant length of the SET opcode sequence, and spdec is the constant
  * by which to decrease the stack pointer to pop all of the SET op's operands.
  *
@@ -4457,17 +4428,16 @@ js_Interpret(JSContext *cx)
                         } else if (PCVAL_IS_SLOT(entry->vword)) {
                             slot = PCVAL_TO_SLOT(entry->vword);
                             JS_ASSERT(slot < obj2->map->freeslot);
                             rval = LOCKED_OBJ_GET_SLOT(obj2, slot);
                         } else {
                             JS_ASSERT(PCVAL_IS_SPROP(entry->vword));
                             sprop = PCVAL_TO_SPROP(entry->vword);
                             NATIVE_GET(cx, obj, obj2, sprop, &rval);
-                            break;
                         }
                         JS_UNLOCK_OBJ(cx, obj2);
                         break;
                     }
                 } else {
                     entry = NULL;
                     if (i < 0)
                         atom = rt->atomState.lengthAtom;
@@ -4541,27 +4511,26 @@ js_Interpret(JSContext *cx)
 
             aobj = js_GetProtoIfDenseArray(cx, obj);
             if (JS_LIKELY(aobj->map->ops->getProperty == js_GetProperty)) {
                 PROPERTY_CACHE_TEST(cx, regs.pc, aobj, obj2, entry, atom);
                 if (!atom) {
                     ASSERT_VALID_PROPERTY_CACHE_HIT(0, aobj, obj2, entry);
                     if (PCVAL_IS_OBJECT(entry->vword)) {
                         rval = PCVAL_OBJECT_TO_JSVAL(entry->vword);
-                        JS_UNLOCK_OBJ(cx, obj2);
                     } else if (PCVAL_IS_SLOT(entry->vword)) {
                         slot = PCVAL_TO_SLOT(entry->vword);
                         JS_ASSERT(slot < obj2->map->freeslot);
                         rval = LOCKED_OBJ_GET_SLOT(obj2, slot);
-                        JS_UNLOCK_OBJ(cx, obj2);
                     } else {
                         JS_ASSERT(PCVAL_IS_SPROP(entry->vword));
                         sprop = PCVAL_TO_SPROP(entry->vword);
                         NATIVE_GET(cx, obj, obj2, sprop, &rval);
                     }
+                    JS_UNLOCK_OBJ(cx, obj2);
                     STORE_OPND(-1, rval);
                     PUSH_OPND(lval);
                     goto end_callprop;
                 }
             } else {
                 entry = NULL;
                 LOAD_ATOM(0);
             }
@@ -4662,17 +4631,19 @@ js_Interpret(JSContext *cx)
                                  * Fastest path: the cached sprop is already
                                  * in scope. Just NATIVE_SET and break to get
                                  * out of the do-while(0).
                                  */
                                 if (sprop == scope->lastProp ||
                                     SCOPE_HAS_PROPERTY(scope, sprop)) {
                                     PCMETER(cache->pchits++);
                                     PCMETER(cache->setpchits++);
-                                    NATIVE_SET(cx, obj, entry, sprop, &rval);
+                                    NATIVE_SET(cx, obj, sprop, &rval);
+                                    JS_UNLOCK_SCOPE(cx, scope);
+                                    TRACE_2(SetPropHit, entry, sprop);
                                     break;
                                 }
                             } else {
                                 scope = js_GetMutableScope(cx, obj);
                                 if (!scope) {
                                     JS_UNLOCK_OBJ(cx, obj);
                                     goto error;
                                 }
@@ -4729,18 +4700,17 @@ js_Interpret(JSContext *cx)
                                     JSScopeProperty *sprop2 =
                                         js_AddScopeProperty(cx, scope,
                                                             sprop->id,
                                                             sprop->getter,
                                                             sprop->setter,
                                                             slot,
                                                             sprop->attrs,
                                                             sprop->flags,
-                                                            sprop->shortid,
-                                                            NULL);
+                                                            sprop->shortid);
                                     if (!sprop2) {
                                         js_FreeSlot(cx, obj, slot);
                                         JS_UNLOCK_SCOPE(cx, scope);
                                         goto error;
                                     }
                                     if (sprop2 != sprop) {
                                         PCMETER(cache->slotchanges++);
                                         JS_ASSERT(slot != sprop->slot &&
@@ -4788,20 +4758,23 @@ js_Interpret(JSContext *cx)
                     } else {
                         ASSERT_VALID_PROPERTY_CACHE_HIT(0, obj, obj2, entry);
                         sprop = NULL;
                         if (obj == obj2) {
                             JS_ASSERT(PCVAL_IS_SPROP(entry->vword));
                             sprop = PCVAL_TO_SPROP(entry->vword);
                             JS_ASSERT(!(sprop->attrs & JSPROP_READONLY));
                             JS_ASSERT(!SCOPE_IS_SEALED(OBJ_SCOPE(obj2)));
-                            NATIVE_SET(cx, obj, entry, sprop, &rval);
+                            NATIVE_SET(cx, obj, sprop, &rval);
+                        }
+                        JS_UNLOCK_OBJ(cx, obj2);
+                        if (sprop) {
+                            TRACE_2(SetPropHit, entry, sprop);
                             break;
                         }
-                        JS_UNLOCK_OBJ(cx, obj2);
                     }
                 }
 
                 if (!atom)
                     LOAD_ATOM(0);
                 id = ATOM_TO_JSID(atom);
                 if (entry) {
                     if (!js_SetPropertyHelper(cx, obj, id, op == JSOP_SETNAME,
@@ -4811,17 +4784,20 @@ js_Interpret(JSContext *cx)
 #ifdef JS_TRACER
                     if (entry)
                         TRACE_1(SetPropMiss, entry);
 #endif
                 } else {
                     if (!OBJ_SET_PROPERTY(cx, obj, id, &rval))
                         goto error;
                 }
-                ABORT_RECORDING(cx, "SetPropUncached");
+#ifdef JS_TRACER
+                if (!entry && TRACE_RECORDER(cx))
+                    js_AbortRecording(cx, "SetPropUncached");
+#endif
             } while (0);
           END_SET_CASE_STORE_RVAL(JSOP_SETPROP, 2);
 
           BEGIN_CASE(JSOP_GETELEM)
             /* Open-coded ELEMENT_OP optimized for strings and dense arrays. */
             lval = FETCH_OPND(-2);
             rval = FETCH_OPND(-1);
             if (JSVAL_IS_STRING(lval) && JSVAL_IS_INT(rval)) {
@@ -5331,16 +5307,17 @@ js_Interpret(JSContext *cx)
                 OBJ_DROP_PROPERTY(cx, obj2, prop);
                 if (!OBJ_GET_PROPERTY(cx, obj, id, &rval))
                     goto error;
                 entry = NULL;
             } else {
                 sprop = (JSScopeProperty *)prop;
           do_native_get:
                 NATIVE_GET(cx, obj, obj2, sprop, &rval);
+                OBJ_DROP_PROPERTY(cx, obj2, (JSProperty *) sprop);
             }
 
           do_push_rval:
             PUSH_OPND(rval);
             if (op == JSOP_CALLNAME)
                 PUSH_OPND(OBJECT_TO_JSVAL(obj));
           }
           END_CASE(JSOP_NAME)
@@ -5810,17 +5787,20 @@ js_Interpret(JSContext *cx)
             obj = fp->varobj;
             lval = fp->slots[slot];
             if (JSVAL_IS_NULL(lval)) {
                 /*
                  * Inline-clone and deoptimize JSOP_SETNAME code here because
                  * JSOP_SETGVAR has arity 1: [rval], not arity 2: [obj, rval]
                  * as JSOP_SETNAME does, where [obj] is due to JSOP_BINDNAME.
                  */
-                ABORT_RECORDING(cx, "SETGVAR with NULL slot");
+#ifdef JS_TRACER
+                if (TRACE_RECORDER(cx))
+                    js_AbortRecording(cx, "SETGVAR with NULL slot");
+#endif
                 LOAD_ATOM(0);
                 id = ATOM_TO_JSID(atom);
                 if (!OBJ_SET_PROPERTY(cx, obj, id, &rval))
                     goto error;
             } else {
                 slot = JSVAL_TO_INT(lval);
                 JS_LOCK_OBJ(cx, obj);
                 LOCKED_OBJ_WRITE_BARRIER(cx, obj, slot, rval);
@@ -6115,17 +6095,20 @@ js_Interpret(JSContext *cx)
                 if (!obj)
                     goto error;
             } else {
                 parent = js_GetScopeChain(cx, fp);
                 if (!parent)
                     goto error;
 
                 if (OBJ_GET_PARENT(cx, obj) != parent) {
-                    ABORT_RECORDING(cx, "DEFLOCALFUN for closure");
+#ifdef JS_TRACER
+                    if (TRACE_RECORDER(cx))
+                        js_AbortRecording(cx, "DEFLOCALFUN for closure");
+#endif
                     obj = js_CloneFunctionObject(cx, fun, parent);
                     if (!obj)
                         goto error;
                 }
             }
 
             slot = GET_SLOTNO(regs.pc);
             TRACE_2(DefLocalFunSetSlot, slot, obj);
@@ -6409,18 +6392,17 @@ js_Interpret(JSContext *cx)
 
                     JS_ASSERT(!scope->lastProp ||
                               scope->shape == scope->lastProp->shape);
                     if (scope->table) {
                         JSScopeProperty *sprop2 =
                             js_AddScopeProperty(cx, scope, sprop->id,
                                                 sprop->getter, sprop->setter,
                                                 slot, sprop->attrs,
-                                                sprop->flags, sprop->shortid,
-                                                NULL);
+                                                sprop->flags, sprop->shortid);
                         if (!sprop2) {
                             js_FreeSlot(cx, obj, slot);
                             JS_UNLOCK_SCOPE(cx, scope);
                             goto error;
                         }
                         JS_ASSERT(sprop2 == sprop);
                     } else {
                         scope->shape = sprop->shape;
@@ -7157,22 +7139,25 @@ js_Interpret(JSContext *cx)
         // Handle other exceptions as if they came from the imacro-calling pc.
         regs.pc = fp->imacpc;
         fp->imacpc = NULL;
         atoms = script->atomMap.vector;
     }
 
     JS_ASSERT((size_t)((fp->imacpc ? fp->imacpc : regs.pc) - script->code) < script->length);
 
+#ifdef JS_TRACER
     /*
      * This abort could be weakened to permit tracing through exceptions that
      * are thrown and caught within a loop, with the co-operation of the tracer.
      * For now just bail on any sign of trouble.
      */
-    ABORT_RECORDING(cx, "error or exception while recording");
+    if (TRACE_RECORDER(cx))
+        js_AbortRecording(cx, "error or exception while recording");
+#endif
 
     if (!cx->throwing) {
         /* This is an error, not a catchable exception, quit the frame ASAP. */
         ok = JS_FALSE;
     } else {
         JSTrapHandler handler;
         JSTryNote *tn, *tnlimit;
         uint32 offset;
--- a/js/src/jsinterp.h
+++ b/js/src/jsinterp.h
@@ -268,16 +268,17 @@ typedef struct JSPropertyCache {
     JSPropCacheEntry    table[PROPERTY_CACHE_SIZE];
     JSBool              empty;
     jsrefcount          disabled;       /* signed for anti-underflow asserts */
 #ifdef JS_PROPERTY_CACHE_METERING
     uint32              fills;          /* number of cache entry fills */
     uint32              nofills;        /* couldn't fill (e.g. default get) */
     uint32              rofills;        /* set on read-only prop can't fill */
     uint32              disfills;       /* fill attempts on disabled cache */
+    uint32              oddfills;       /* fill attempt after setter deleted */
     uint32              modfills;       /* fill that rehashed to a new entry */
     uint32              brandfills;     /* scope brandings to type structural
                                            method fills */
     uint32              noprotos;       /* resolve-returned non-proto pobj */
     uint32              longchains;     /* overlong scope and/or proto chain */
     uint32              recycles;       /* cache entries recycled by fills */
     uint32              pcrecycles;     /* pc-keyed entries recycled by atom-
                                            keyed fills */
@@ -338,20 +339,20 @@ typedef struct JSPropertyCache {
 #define SPROP_TO_PCVAL(sprop)   PCVAL_SETTAG(sprop, PCVAL_SPROP)
 
 /*
  * Fill property cache entry for key cx->fp->pc, optimized value word computed
  * from obj and sprop, and entry capability forged from 24-bit OBJ_SHAPE(obj),
  * 4-bit scopeIndex, and 4-bit protoIndex.
  */
 extern JS_REQUIRES_STACK void
-js_FillPropertyCache(JSContext *cx, JSObject *obj,
+js_FillPropertyCache(JSContext *cx, JSObject *obj, jsuword kshape,
                      uintN scopeIndex, uintN protoIndex,
                      JSObject *pobj, JSScopeProperty *sprop,
-                     JSBool cacheByPrevShape, JSPropCacheEntry **entryp);
+                     JSPropCacheEntry **entryp);
 
 /*
  * Property cache lookup macros. PROPERTY_CACHE_TEST is designed to inline the
  * fast path in js_Interpret, so it makes "just-so" restrictions on parameters,
  * e.g. pobj and obj should not be the same variable, since for JOF_PROP-mode
  * opcodes, obj must not be changed because of a cache miss.
  *
  * On return from PROPERTY_CACHE_TEST, if atom is null then obj points to the
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -3641,17 +3641,17 @@ js_AddNativeProperty(JSContext *cx, JSOb
     JS_LOCK_OBJ(cx, obj);
     scope = js_GetMutableScope(cx, obj);
     if (!scope) {
         sprop = NULL;
     } else {
         /* Convert string indices to integers if appropriate. */
         CHECK_FOR_STRING_INDEX(id);
         sprop = js_AddScopeProperty(cx, scope, id, getter, setter, slot, attrs,
-                                    flags, shortid, NULL);
+                                    flags, shortid);
     }
     JS_UNLOCK_OBJ(cx, obj);
     return sprop;
 }
 
 JSScopeProperty *
 js_ChangeNativePropertyAttrs(JSContext *cx, JSObject *obj,
                              JSScopeProperty *sprop, uintN attrs, uintN mask,
@@ -3707,21 +3707,22 @@ JSBool
 js_DefineNativeProperty(JSContext *cx, JSObject *obj, jsid id, jsval value,
                         JSPropertyOp getter, JSPropertyOp setter, uintN attrs,
                         uintN flags, intN shortid, JSProperty **propp,
                         JSPropCacheEntry** entryp /* = NULL */)
 {
     JSClass *clasp;
     JSScope *scope;
     JSScopeProperty *sprop;
-    JSBool cacheByPrevShape;
 
     /* Convert string indices to integers if appropriate. */
     CHECK_FOR_STRING_INDEX(id);
 
+    uint32 shape = OBJ_SHAPE(obj);
+
 #if JS_HAS_GETTER_SETTER
     /*
      * If defining a getter or setter, we must check for its counterpart and
      * update the attributes and property ops.  A getter or setter is really
      * only half of a property.
      */
     sprop = NULL;
     if (attrs & (JSPROP_GETTER | JSPROP_SETTER)) {
@@ -3779,45 +3780,42 @@ js_DefineNativeProperty(JSContext *cx, J
     if (!setter)
         setter = clasp->setProperty;
 
     /* Get obj's own scope if it has one, or create a new one for obj. */
     scope = js_GetMutableScope(cx, obj);
     if (!scope)
         goto bad;
 
-    cacheByPrevShape = false;
     if (!sprop) {
         /* Add a new property, or replace an existing one of the same id. */
         if (clasp->flags & JSCLASS_SHARE_ALL_PROPERTIES)
             attrs |= JSPROP_SHARED;
         sprop = js_AddScopeProperty(cx, scope, id, getter, setter,
                                     SPROP_INVALID_SLOT, attrs, flags,
-                                    shortid, &cacheByPrevShape);
+                                    shortid);
         if (!sprop)
             goto bad;
     }
 
     /* Store value before calling addProperty, in case the latter GC's. */
     if (SPROP_HAS_VALID_SLOT(sprop, scope))
         LOCKED_OBJ_WRITE_BARRIER(cx, obj, sprop->slot, value);
 
     /* XXXbe called with lock held */
     ADD_PROPERTY_HELPER(cx, clasp, obj, scope, sprop, &value,
                         js_RemoveScopeProperty(cx, scope, id);
                         goto bad);
 
     if (entryp) {
         JS_ASSERT_NOT_ON_TRACE(cx);
-        if (!(attrs & JSPROP_SHARED)) {
-            js_FillPropertyCache(cx, obj, 0, 0, obj, sprop,
-                                 cacheByPrevShape, entryp);
-        } else {
+        if (!(attrs & JSPROP_SHARED))
+            js_FillPropertyCache(cx, obj, shape, 0, 0, obj, sprop, entryp);
+        else
             PCMETER(JS_PROPERTY_CACHE(cx).nofills++);
-        }
     }
     if (propp)
         *propp = (JSProperty *) sprop;
     else
         JS_UNLOCK_OBJ(cx, obj);
     return JS_TRUE;
 
 bad:
@@ -4006,24 +4004,25 @@ out:
     return protoIndex;
 }
 
 int
 js_FindPropertyHelper(JSContext *cx, jsid id, JSObject **objp,
                       JSObject **pobjp, JSProperty **propp,
                       JSPropCacheEntry **entryp)
 {
-    JSObject *scopeChain, *obj, *pobj, *lastobj;
+    JSObject *obj, *pobj, *lastobj;
+    uint32 shape;
     int scopeIndex, protoIndex;
     JSProperty *prop;
     JSScopeProperty *sprop;
 
     JS_ASSERT_IF(entryp, !JS_ON_TRACE(cx));
-    scopeChain = js_GetTopStackFrame(cx)->scopeChain;
-    obj = scopeChain;
+    obj = js_GetTopStackFrame(cx)->scopeChain;
+    shape = OBJ_SHAPE(obj);
     for (scopeIndex = 0; ; scopeIndex++) {
         if (obj->map->ops->lookupProperty == js_LookupProperty) {
             protoIndex =
                 js_LookupPropertyWithFlags(cx, obj, id, cx->resolveFlags,
                                            &pobj, &prop);
             if (protoIndex < 0)
                 return -1;
         } else {
@@ -4031,18 +4030,18 @@ js_FindPropertyHelper(JSContext *cx, jsi
                 return -1;
             protoIndex = -1;
         }
 
         if (prop) {
             if (entryp) {
                 if (protoIndex >= 0 && OBJ_IS_NATIVE(pobj)) {
                     sprop = (JSScopeProperty *) prop;
-                    js_FillPropertyCache(cx, scopeChain, scopeIndex,
-                                         protoIndex, pobj, sprop, false,
+                    js_FillPropertyCache(cx, cx->fp->scopeChain, shape,
+                                         scopeIndex, protoIndex, pobj, sprop,
                                          entryp);
                 } else {
                     PCMETER(JS_PROPERTY_CACHE(cx).nofills++);
                     *entryp = NULL;
                 }
             }
             SCOPE_DEPTH_ACCUM(&rt->scopeSearchDepthStats, scopeIndex);
             *objp = obj;
@@ -4114,18 +4113,19 @@ js_FindIdentifierBase(JSContext *cx, JSO
         int protoIndex = js_LookupPropertyWithFlags(cx, obj, id,
                                                     cx->resolveFlags,
                                                     &pobj, &prop);
         if (protoIndex < 0)
             return NULL;
         if (prop) {
             JS_ASSERT(OBJ_IS_NATIVE(pobj));
             JS_ASSERT(OBJ_GET_CLASS(cx, pobj) == OBJ_GET_CLASS(cx, obj));
-            js_FillPropertyCache(cx, scopeChain, scopeIndex, protoIndex, pobj,
-                                 (JSScopeProperty *) prop, false, &entry);
+            js_FillPropertyCache(cx, scopeChain, OBJ_SHAPE(scopeChain),
+                                 scopeIndex, protoIndex, pobj,
+                                 (JSScopeProperty *) prop, &entry);
             JS_UNLOCK_OBJ(cx, pobj);
             return obj;
         }
 
         /* Call and other cacheable objects always have a parent. */
         obj = OBJ_GET_PARENT(cx, obj);
         if (!OBJ_GET_PARENT(cx, obj))
             return obj;
@@ -4154,141 +4154,130 @@ js_FindIdentifierBase(JSContext *cx, JSO
     } while (OBJ_GET_PARENT(cx, obj));
     return obj;
 }
 
 JSBool
 js_NativeGet(JSContext *cx, JSObject *obj, JSObject *pobj,
              JSScopeProperty *sprop, jsval *vp)
 {
+    js_LeaveTraceIfGlobalObject(cx, pobj);
+
+    JSScope *scope;
+    uint32 slot;
+    int32 sample;
+    JSTempValueRooter tvr;
+    JSBool ok;
+
     JS_ASSERT(OBJ_IS_NATIVE(pobj));
     JS_ASSERT(JS_IS_OBJ_LOCKED(cx, pobj));
-    JS_ASSERT(OBJ_SCOPE(pobj)->object == pobj);
-
-    js_LeaveTraceIfGlobalObject(cx, pobj);
-
-    JSScope *scope = OBJ_SCOPE(pobj);
-    uint32 slot = sprop->slot;
-    if (slot == SPROP_INVALID_SLOT) {
-        JS_UNLOCK_SCOPE(cx, scope);
-        *vp = JSVAL_VOID;
-        return SPROP_HAS_STUB_GETTER(sprop) || js_GetSprop(cx, sprop, obj, vp);
+    scope = OBJ_SCOPE(pobj);
+    JS_ASSERT(scope->object == pobj);
+
+    slot = sprop->slot;
+    *vp = (slot != SPROP_INVALID_SLOT)
+          ? LOCKED_OBJ_GET_SLOT(pobj, slot)
+          : JSVAL_VOID;
+    if (SPROP_HAS_STUB_GETTER(sprop))
+        return JS_TRUE;
+
+    sample = cx->runtime->propertyRemovals;
+    JS_UNLOCK_SCOPE(cx, scope);
+    JS_PUSH_TEMP_ROOT_SPROP(cx, sprop, &tvr);
+    ok = js_GetSprop(cx, sprop, obj, vp);
+    JS_POP_TEMP_ROOT(cx, &tvr);
+    if (!ok)
+        return JS_FALSE;
+
+    JS_LOCK_SCOPE(cx, scope);
+    JS_ASSERT(scope->object == pobj);
+    if (SLOT_IN_SCOPE(slot, scope) &&
+        (JS_LIKELY(cx->runtime->propertyRemovals == sample) ||
+         SCOPE_GET_PROPERTY(scope, sprop->id) == sprop)) {
+        LOCKED_OBJ_SET_SLOT(pobj, slot, *vp);
     }
 
-    *vp = LOCKED_OBJ_GET_SLOT(pobj, slot);
-    if (!SPROP_HAS_STUB_GETTER(sprop)) {
-        /*
-         * For API compatibility, we must set the slot with the getter's
-         * result, so we have to make sure that it is still valid after we run
-         * the getter. FIXME: we should reconsider this, bug 488458.
-         */
-        int32 sample = cx->runtime->propertyRemovals;
-        JSTempValueRooter tvr, tvr2;
-        JSBool ok;
-
-        JS_UNLOCK_SCOPE(cx, scope);
-        JS_PUSH_TEMP_ROOT_SPROP(cx, sprop, &tvr);
-        JS_PUSH_TEMP_ROOT_OBJECT(cx, pobj, &tvr2);
-        ok = js_GetSprop(cx, sprop, obj, vp);
-        JS_POP_TEMP_ROOT(cx, &tvr2);
-        JS_POP_TEMP_ROOT(cx, &tvr);
-        if (!ok)
-            return JS_FALSE;
-
-        JS_LOCK_SCOPE(cx, scope);
-        JS_ASSERT(scope->object == pobj);
-        if (SLOT_IN_SCOPE(slot, scope) &&
-            (JS_LIKELY(cx->runtime->propertyRemovals == sample) ||
-             SCOPE_GET_PROPERTY(scope, sprop->id) == sprop)) {
-            LOCKED_OBJ_SET_SLOT(pobj, slot, *vp);
-        }
-    }
-    JS_UNLOCK_SCOPE(cx, scope);
-
     return JS_TRUE;
 }
 
 JSBool
 js_NativeSet(JSContext *cx, JSObject *obj, JSScopeProperty *sprop, jsval *vp)
 {
+    js_LeaveTraceIfGlobalObject(cx, obj);
+
+    JSScope *scope;
+    uint32 slot;
+    int32 sample;
+    JSTempValueRooter tvr;
+    JSBool ok;
+
     JS_ASSERT(OBJ_IS_NATIVE(obj));
     JS_ASSERT(JS_IS_OBJ_LOCKED(cx, obj));
-    JS_ASSERT(OBJ_SCOPE(obj)->object == obj);
-
-    js_LeaveTraceIfGlobalObject(cx, obj);
-
-    JSScope *scope = OBJ_SCOPE(obj);
-    uint32 slot = sprop->slot;
-    if (slot == SPROP_INVALID_SLOT) {
-        JS_UNLOCK_SCOPE(cx, scope);
-
+    scope = OBJ_SCOPE(obj);
+    JS_ASSERT(scope->object == obj);
+
+    slot = sprop->slot;
+    if (slot != SPROP_INVALID_SLOT) {
+        OBJ_CHECK_SLOT(obj, slot);
+
+        /* If sprop has a stub setter, keep scope locked and just store *vp. */
+        if (SPROP_HAS_STUB_SETTER(sprop))
+            goto set_slot;
+    } else {
         /*
          * Allow API consumers to create shared properties with stub setters.
          * Such properties lack value storage, so setting them is like writing
          * to /dev/null.
          *
          * But we can't short-circuit if there's a scripted getter or setter
          * since we might need to throw. In that case, we let SPROP_SET
          * decide whether to throw an exception. See bug 478047.
          */
         if (!(sprop->attrs & JSPROP_GETTER) && SPROP_HAS_STUB_SETTER(sprop)) {
             JS_ASSERT(!(sprop->attrs & JSPROP_SETTER));
             return JS_TRUE;
         }
-        return js_SetSprop(cx, sprop, obj, vp);
     }
 
-    OBJ_CHECK_SLOT(obj, slot);
-
-    if (SPROP_HAS_STUB_SETTER(sprop)) {
+    sample = cx->runtime->propertyRemovals;
+    JS_UNLOCK_SCOPE(cx, scope);
+    JS_PUSH_TEMP_ROOT_SPROP(cx, sprop, &tvr);
+    ok = js_SetSprop(cx, sprop, obj, vp);
+    JS_POP_TEMP_ROOT(cx, &tvr);
+    if (!ok)
+        return JS_FALSE;
+
+    JS_LOCK_SCOPE(cx, scope);
+    JS_ASSERT(scope->object == obj);
+    if (SLOT_IN_SCOPE(slot, scope) &&
+        (JS_LIKELY(cx->runtime->propertyRemovals == sample) ||
+         SCOPE_GET_PROPERTY(scope, sprop->id) == sprop)) {
+  set_slot:
         LOCKED_OBJ_WRITE_BARRIER(cx, obj, slot, *vp);
-    } else {
-        /*
-         * For API compatibility we must set the slot with setter's result so
-         * we must make sure that it is still valid after we run the setter.
-         * FIXME: we should reconsider this, bug 488458.
-         */
-        int32 sample = cx->runtime->propertyRemovals;
-        JSTempValueRooter tvr;
-        JSBool ok;
-
-        JS_UNLOCK_SCOPE(cx, scope);
-        JS_PUSH_TEMP_ROOT_SPROP(cx, sprop, &tvr);
-        ok = js_SetSprop(cx, sprop, obj, vp);
-        JS_POP_TEMP_ROOT(cx, &tvr);
-        if (!ok)
-            return JS_FALSE;
-
-        JS_LOCK_SCOPE(cx, scope);
-        JS_ASSERT(scope->object == obj);
-        if (SLOT_IN_SCOPE(slot, scope) &&
-            (JS_LIKELY(cx->runtime->propertyRemovals == sample) ||
-             SCOPE_GET_PROPERTY(scope, sprop->id) == sprop)) {
-            LOCKED_OBJ_WRITE_BARRIER(cx, obj, slot, *vp);
-        }
     }
-    JS_UNLOCK_SCOPE(cx, scope);
 
     return JS_TRUE;
 }
 
 JSBool
 js_GetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, jsval *vp,
                      JSPropCacheEntry **entryp)
 {
     JSObject *aobj, *obj2;
+    uint32 shape;
     int protoIndex;
     JSProperty *prop;
     JSScopeProperty *sprop;
 
     JS_ASSERT_IF(entryp, !JS_ON_TRACE(cx));
     /* Convert string indices to integers if appropriate. */
     CHECK_FOR_STRING_INDEX(id);
 
     aobj = js_GetProtoIfDenseArray(cx, obj);
+    shape = OBJ_SHAPE(aobj);
     protoIndex = js_LookupPropertyWithFlags(cx, aobj, id, cx->resolveFlags,
                                             &obj2, &prop);
     if (protoIndex < 0)
         return JS_FALSE;
     if (!prop) {
         *vp = JSVAL_VOID;
 
         if (!OBJ_GET_CLASS(cx, obj)->getProperty(cx, obj, ID_TO_VALUE(id), vp))
@@ -4352,24 +4341,25 @@ js_GetPropertyHelper(JSContext *cx, JSOb
     }
 
     if (!OBJ_IS_NATIVE(obj2)) {
         OBJ_DROP_PROPERTY(cx, obj2, prop);
         return OBJ_GET_PROPERTY(cx, obj2, id, vp);
     }
 
     sprop = (JSScopeProperty *) prop;
+    if (!js_NativeGet(cx, obj, obj2, sprop, vp))
+        return JS_FALSE;
+
     if (entryp) {
         JS_ASSERT_NOT_ON_TRACE(cx);
-        js_FillPropertyCache(cx, aobj, 0, protoIndex, obj2, sprop, false,
-                             entryp);
+        js_FillPropertyCache(cx, aobj, shape, 0, protoIndex, obj2, sprop, entryp);
     }
-
-    /* The following unlocks the object. */
-    return js_NativeGet(cx, obj, obj2, sprop, vp);
+    JS_UNLOCK_OBJ(cx, obj2);
+    return JS_TRUE;
 }
 
 JSBool
 js_GetProperty(JSContext *cx, JSObject *obj, jsid id, jsval *vp)
 {
     return js_GetPropertyHelper(cx, obj, id, vp, NULL);
 }
 
@@ -4388,39 +4378,40 @@ js_GetMethod(JSContext *cx, JSObject *ob
 #endif
     return OBJ_GET_PROPERTY(cx, obj, id, vp);
 }
 
 JSBool
 js_SetPropertyHelper(JSContext *cx, JSObject *obj, jsid id,
                      JSBool unqualified, jsval *vp, JSPropCacheEntry **entryp)
 {
+    uint32 shape;
     int protoIndex;
     JSObject *pobj;
     JSProperty *prop;
     JSScopeProperty *sprop;
     JSScope *scope;
     uintN attrs, flags;
     intN shortid;
     JSClass *clasp;
     JSPropertyOp getter, setter;
-    JSBool cacheByPrevShape;
 
     /* Convert string indices to integers if appropriate. */
     CHECK_FOR_STRING_INDEX(id);
 
     /*
      * We peek at OBJ_SCOPE(obj) without locking obj. Any race means a failure
      * to seal before sharing, which is inherently ambiguous.
      */
     if (SCOPE_IS_SEALED(OBJ_SCOPE(obj)) && OBJ_SCOPE(obj)->object == obj) {
         flags = JSREPORT_ERROR;
         goto read_only_error;
     }
 
+    shape = OBJ_SHAPE(obj);
     protoIndex = js_LookupPropertyWithFlags(cx, obj, id, cx->resolveFlags,
                                             &pobj, &prop);
     if (protoIndex < 0)
         return JS_FALSE;
     if (prop) {
         if (!OBJ_IS_NATIVE(pobj)) {
             OBJ_DROP_PROPERTY(cx, pobj, prop);
             prop = NULL;
@@ -4551,17 +4542,16 @@ js_SetPropertyHelper(JSContext *cx, JSOb
             sprop = NULL;
         }
 #ifdef __GNUC__ /* suppress bogus gcc warnings */
     } else {
         scope = NULL;
 #endif
     }
 
-    cacheByPrevShape = false;
     if (!sprop) {
         /*
          * Purge the property cache of now-shadowed id in obj's scope chain.
          * Do this early, before locking obj to avoid nesting locks.
          */
         js_PurgeScopeChain(cx, obj, id);
 
         /* Find or make a property descriptor with the right heritage. */
@@ -4569,18 +4559,17 @@ js_SetPropertyHelper(JSContext *cx, JSOb
         scope = js_GetMutableScope(cx, obj);
         if (!scope) {
             JS_UNLOCK_OBJ(cx, obj);
             return JS_FALSE;
         }
         if (clasp->flags & JSCLASS_SHARE_ALL_PROPERTIES)
             attrs |= JSPROP_SHARED;
         sprop = js_AddScopeProperty(cx, scope, id, getter, setter,
-                                    SPROP_INVALID_SLOT, attrs, flags, shortid,
-                                    &cacheByPrevShape);
+                                    SPROP_INVALID_SLOT, attrs, flags, shortid);
         if (!sprop) {
             JS_UNLOCK_SCOPE(cx, scope);
             return JS_FALSE;
         }
 
         /*
          * Initialize the new property value (passed to setter) to undefined.
          * Note that we store before calling addProperty, to match the order
@@ -4591,28 +4580,28 @@ js_SetPropertyHelper(JSContext *cx, JSOb
 
         /* XXXbe called with obj locked */
         ADD_PROPERTY_HELPER(cx, clasp, obj, scope, sprop, vp,
                             js_RemoveScopeProperty(cx, scope, id);
                             JS_UNLOCK_SCOPE(cx, scope);
                             return JS_FALSE);
     }
 
+    if (!js_NativeSet(cx, obj, sprop, vp))
+        return JS_FALSE;
+
     if (entryp) {
         JS_ASSERT_NOT_ON_TRACE(cx);
-        if (!(attrs & JSPROP_SHARED)) {
-            js_FillPropertyCache(cx, obj, 0, 0, obj, sprop, cacheByPrevShape,
-                                 entryp);
-        } else {
+        if (!(attrs & JSPROP_SHARED))
+            js_FillPropertyCache(cx, obj, shape, 0, 0, obj, sprop, entryp);
+        else
             PCMETER(JS_PROPERTY_CACHE(cx).nofills++);
-        }
     }
-
-    /* The following unlocks the object. */
-    return js_NativeSet(cx, obj, sprop, vp);
+    JS_UNLOCK_SCOPE(cx, scope);
+    return JS_TRUE;
 
   read_only_error:
     return js_ReportValueErrorFlags(cx, flags, JSMSG_READ_ONLY,
                                     JSDVG_IGNORE_STACK, ID_TO_VALUE(id), NULL,
                                     NULL, NULL);
 }
 
 JSBool
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -683,29 +683,25 @@ js_FindProperty(JSContext *cx, jsid id, 
 extern JS_REQUIRES_STACK JSObject *
 js_FindIdentifierBase(JSContext *cx, JSObject *scopeChain, jsid id,
                       JSPropCacheEntry *entry);
 
 extern JSObject *
 js_FindVariableScope(JSContext *cx, JSFunction **funp);
 
 /*
- * This function must be called with pobj's scope locked. On return that scope
- * is always unlocked. After this function returns, the caller must not access
- * pobj or sprop unless it knows that sprop's getter is a stub.
+ * NB: js_NativeGet and js_NativeSet are called with the scope containing sprop
+ * (pobj's scope for Get, obj's for Set) locked, and on successful return, that
+ * scope is again locked.  But on failure, both functions return false with the
+ * scope containing sprop unlocked.
  */
 extern JSBool
 js_NativeGet(JSContext *cx, JSObject *obj, JSObject *pobj,
              JSScopeProperty *sprop, jsval *vp);
 
-/*
- * This function must be called with obj's scope locked. On return that scope
- * is always unlocked. After this function returns, the caller must not access
- * sprop unless it knows that sprop's setter is a stub.
- */
 extern JSBool
 js_NativeSet(JSContext *cx, JSObject *obj, JSScopeProperty *sprop, jsval *vp);
 
 extern JSBool
 js_GetPropertyHelper(JSContext *cx, JSObject *obj, jsid id, jsval *vp,
                      JSPropCacheEntry **entryp);
 
 extern JSBool
--- a/js/src/jsscope.cpp
+++ b/js/src/jsscope.cpp
@@ -986,26 +986,24 @@ ReportReadOnlyScope(JSContext *cx, JSSco
     if (!bytes)
         return;
     JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_READ_ONLY, bytes);
 }
 
 JSScopeProperty *
 js_AddScopeProperty(JSContext *cx, JSScope *scope, jsid id,
                     JSPropertyOp getter, JSPropertyOp setter, uint32 slot,
-                    uintN attrs, uintN flags, intN shortid,
-                    JSBool *cacheByPrevShape)
+                    uintN attrs, uintN flags, intN shortid)
 {
     JSScopeProperty **spp, *sprop, *overwriting, **spvec, **spp2, child;
     uint32 size, splen, i;
     int change;
     JSTempValueRooter tvr;
 
     JS_ASSERT(JS_IS_SCOPE_LOCKED(cx, scope));
-    JS_ASSERT_IF(cacheByPrevShape, !*cacheByPrevShape);
     CHECK_ANCESTOR_LINE(scope, JS_TRUE);
 
     /*
      * You can't add properties to a sealed scope.  But note well that you can
      * change property attributes in a sealed scope, even though that replaces
      * a JSScopeProperty * in the scope's hash table -- but no id is added, so
      * the scope remains sealed.
      */
@@ -1269,27 +1267,18 @@ js_AddScopeProperty(JSContext *cx, JSSco
         sprop = GetPropertyTreeChild(cx, scope->lastProp, &child);
         if (!sprop)
             goto fail_overwrite;
 
         /*
          * The scope's shape defaults to its last property's shape, but may
          * be regenerated later as the scope diverges (from the property cache
          * point of view) from the structural type associated with sprop.
-         *
-         * The following is an open-coded version of SCOPE_EXTEND_SHAPE to
-         * set *cacheByPrevShape.
          */
-        if (!scope->lastProp || scope->shape == scope->lastProp->shape) {
-            scope->shape = sprop->shape;
-            if (cacheByPrevShape)
-                *cacheByPrevShape = true;
-        } else {
-            scope->shape = js_GenerateShape(cx, false, sprop);
-        }
+        SCOPE_EXTEND_SHAPE(cx, scope, sprop);
 
         /* Store the tree node pointer in the table entry for id. */
         if (scope->table)
             SPROP_STORE_PRESERVING_COLLISION(spp, sprop);
         scope->entryCount++;
         scope->lastProp = sprop;
         CHECK_ANCESTOR_LINE(scope, JS_FALSE);
 #ifdef DEBUG
@@ -1415,18 +1404,17 @@ js_ChangeScopePropertyAttrs(JSContext *c
         /*
          * Let js_AddScopeProperty handle this |overwriting| case, including
          * the conservation of sprop->slot (if it's valid).  We must not call
          * js_RemoveScopeProperty here, it will free a valid sprop->slot and
          * js_AddScopeProperty won't re-allocate it.
          */
         newsprop = js_AddScopeProperty(cx, scope, child.id,
                                        child.getter, child.setter, child.slot,
-                                       child.attrs, child.flags, child.shortid,
-                                       NULL);
+                                       child.attrs, child.flags, child.shortid);
     }
 
     if (newsprop) {
         if (scope->shape == sprop->shape)
             scope->shape = newsprop->shape;
         else
             SCOPE_MAKE_UNIQUE_SHAPE(cx, scope);
     }
--- a/js/src/jsscope.h
+++ b/js/src/jsscope.h
@@ -414,26 +414,20 @@ extern JS_FRIEND_API(JSScopeProperty **)
 js_SearchScope(JSScope *scope, jsid id, JSBool adding);
 
 #define SCOPE_GET_PROPERTY(scope, id)                                         \
     SPROP_FETCH(js_SearchScope(scope, id, JS_FALSE))
 
 #define SCOPE_HAS_PROPERTY(scope, sprop)                                      \
     (SCOPE_GET_PROPERTY(scope, (sprop)->id) == (sprop))
 
-/*
- * If cacheByPrevShape is not null, *cacheByPrevShape must be false on
- * entrance. On exit it will point to true if this call added the property
- * predictably and js_FillPropertyCache can optimize for that.
- */
 extern JSScopeProperty *
 js_AddScopeProperty(JSContext *cx, JSScope *scope, jsid id,
                     JSPropertyOp getter, JSPropertyOp setter, uint32 slot,
-                    uintN attrs, uintN flags, intN shortid,
-                    JSBool *cacheByPrevShape);
+                    uintN attrs, uintN flags, intN shortid);
 
 extern JSScopeProperty *
 js_ChangeScopePropertyAttrs(JSContext *cx, JSScope *scope,
                             JSScopeProperty *sprop, uintN attrs, uintN mask,
                             JSPropertyOp getter, JSPropertyOp setter);
 
 extern JSBool
 js_RemoveScopeProperty(JSContext *cx, JSScope *scope, jsid id);
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -5957,18 +5957,18 @@ TraceRecorder::test_property_cache(JSObj
                 ABORT_TRACE("failed to lookup property");
 
             if (prop) {
                 if (!OBJ_IS_NATIVE(obj2)) {
                     OBJ_DROP_PROPERTY(cx, obj2, prop);
                     ABORT_TRACE("property found on non-native object");
                 }
 
-                js_FillPropertyCache(cx, aobj, 0, protoIndex, obj2,
-                                     (JSScopeProperty*) prop, false, &entry);
+                js_FillPropertyCache(cx, aobj, OBJ_SHAPE(aobj), 0, protoIndex, obj2,
+                                     (JSScopeProperty*) prop, &entry);
             }
         }
 
         if (!prop) {
             // Propagate obj from js_FindPropertyHelper to record_JSOP_BINDNAME
             // via our obj2 out-parameter. If we are recording JSOP_SETNAME and
             // the global it's assigning does not yet exist, create it.
             obj2 = obj;