bug 487204 - avoiding extra locks for js_Native(Get|Set). r=brendan
authorIgor Bukanov <igor@mir2.org>
Wed, 15 Apr 2009 21:13:27 +0200
changeset 24931 6f105eb31c26e07ac2e45b6d171d463ff5474123
parent 24930 88343c2bfbbafecc7a194b6ee6671279963eab51
child 24932 630261e7e9a73fd6901826a12678e6c07d510e4e
push id1267
push userrsayre@mozilla.com
push dateSun, 19 Apr 2009 02:47:24 +0000
reviewersbrendan
bugs487204
milestone1.9.1b4pre
bug 487204 - avoiding extra locks for js_Native(Get|Set). r=brendan
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,18 +788,19 @@ 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;
 }
 
@@ -1286,17 +1287,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);
+                                    0, 0, NULL);
         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,17 +269,18 @@ 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);
+                                 sprop->attrs, sprop->flags, sprop->shortid,
+                                 NULL);
     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,60 +106,50 @@ 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, jsuword kshape,
+js_FillPropertyCache(JSContext *cx, JSObject *obj,
                      uintN scopeIndex, uintN protoIndex,
                      JSObject *pobj, JSScopeProperty *sprop,
-                     JSPropCacheEntry **entryp)
+                     JSBool cacheByPrevShape, 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 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.
+     * 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.
      *
      * 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) {
@@ -193,17 +183,18 @@ 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) {
@@ -229,58 +220,86 @@ 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))),
-                            kshape);
+                            OBJ_SHAPE(obj));
 #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);
 
-    /*
-     * 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;
-
+    if (kshape == 0)
+        kshape = OBJ_SHAPE(obj);
     khash = PROPERTY_CACHE_HASH_PC(pc, kshape);
     if (obj == pobj) {
-        JS_ASSERT(kshape != 0 || scope->shape != 0);
+        JS_ASSERT(kshape != 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);
@@ -465,17 +484,16 @@ 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);
@@ -2615,16 +2633,26 @@ 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;
@@ -3011,20 +3039,17 @@ 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) {
-#ifdef JS_TRACER
-                if (TRACE_RECORDER(cx))
-                    js_AbortRecording(cx, "interrupt handler");
-#endif
+                ABORT_RECORDING(cx, "interrupt handler");
                 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;
@@ -3554,31 +3579,35 @@ 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,sprop,vp)                                           \
+#define NATIVE_SET(cx,obj,entry,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.
  *
@@ -4428,16 +4457,17 @@ 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;
@@ -4511,26 +4541,27 @@ 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);
             }
@@ -4631,19 +4662,17 @@ 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, sprop, &rval);
-                                    JS_UNLOCK_SCOPE(cx, scope);
-                                    TRACE_2(SetPropHit, entry, sprop);
+                                    NATIVE_SET(cx, obj, entry, sprop, &rval);
                                     break;
                                 }
                             } else {
                                 scope = js_GetMutableScope(cx, obj);
                                 if (!scope) {
                                     JS_UNLOCK_OBJ(cx, obj);
                                     goto error;
                                 }
@@ -4700,17 +4729,18 @@ js_Interpret(JSContext *cx)
                                     JSScopeProperty *sprop2 =
                                         js_AddScopeProperty(cx, scope,
                                                             sprop->id,
                                                             sprop->getter,
                                                             sprop->setter,
                                                             slot,
                                                             sprop->attrs,
                                                             sprop->flags,
-                                                            sprop->shortid);
+                                                            sprop->shortid,
+                                                            NULL);
                                     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 &&
@@ -4758,23 +4788,20 @@ 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, sprop, &rval);
+                            NATIVE_SET(cx, obj, entry, sprop, &rval);
+                            break;
                         }
                         JS_UNLOCK_OBJ(cx, obj2);
-                        if (sprop) {
-                            TRACE_2(SetPropHit, entry, sprop);
-                            break;
-                        }
                     }
                 }
 
                 if (!atom)
                     LOAD_ATOM(0);
                 id = ATOM_TO_JSID(atom);
                 if (entry) {
                     if (!js_SetPropertyHelper(cx, obj, id, op == JSOP_SETNAME,
@@ -4784,20 +4811,17 @@ 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;
                 }
-#ifdef JS_TRACER
-                if (!entry && TRACE_RECORDER(cx))
-                    js_AbortRecording(cx, "SetPropUncached");
-#endif
+                ABORT_RECORDING(cx, "SetPropUncached");
             } 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)) {
@@ -5307,17 +5331,16 @@ 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)
@@ -5787,20 +5810,17 @@ 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.
                  */
-#ifdef JS_TRACER
-                if (TRACE_RECORDER(cx))
-                    js_AbortRecording(cx, "SETGVAR with NULL slot");
-#endif
+                ABORT_RECORDING(cx, "SETGVAR with NULL slot");
                 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);
@@ -6095,20 +6115,17 @@ 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) {
-#ifdef JS_TRACER
-                    if (TRACE_RECORDER(cx))
-                        js_AbortRecording(cx, "DEFLOCALFUN for closure");
-#endif
+                    ABORT_RECORDING(cx, "DEFLOCALFUN for closure");
                     obj = js_CloneFunctionObject(cx, fun, parent);
                     if (!obj)
                         goto error;
                 }
             }
 
             slot = GET_SLOTNO(regs.pc);
             TRACE_2(DefLocalFunSetSlot, slot, obj);
@@ -6392,17 +6409,18 @@ 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);
+                                                sprop->flags, sprop->shortid,
+                                                NULL);
                         if (!sprop2) {
                             js_FreeSlot(cx, obj, slot);
                             JS_UNLOCK_SCOPE(cx, scope);
                             goto error;
                         }
                         JS_ASSERT(sprop2 == sprop);
                     } else {
                         scope->shape = sprop->shape;
@@ -7139,25 +7157,22 @@ 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.
      */
-    if (TRACE_RECORDER(cx))
-        js_AbortRecording(cx, "error or exception while recording");
-#endif
+    ABORT_RECORDING(cx, "error or exception while recording");
 
     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,17 +268,16 @@ 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 */
@@ -339,20 +338,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, jsuword kshape,
+js_FillPropertyCache(JSContext *cx, JSObject *obj,
                      uintN scopeIndex, uintN protoIndex,
                      JSObject *pobj, JSScopeProperty *sprop,
-                     JSPropCacheEntry **entryp);
+                     JSBool cacheByPrevShape, 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);
+                                    flags, shortid, NULL);
     }
     JS_UNLOCK_OBJ(cx, obj);
     return sprop;
 }
 
 JSScopeProperty *
 js_ChangeNativePropertyAttrs(JSContext *cx, JSObject *obj,
                              JSScopeProperty *sprop, uintN attrs, uintN mask,
@@ -3707,22 +3707,21 @@ 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)) {
@@ -3780,42 +3779,45 @@ 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);
+                                    shortid, &cacheByPrevShape);
         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, shape, 0, 0, obj, sprop, entryp);
-        else
+        if (!(attrs & JSPROP_SHARED)) {
+            js_FillPropertyCache(cx, obj, 0, 0, obj, sprop,
+                                 cacheByPrevShape, entryp);
+        } else {
             PCMETER(JS_PROPERTY_CACHE(cx).nofills++);
+        }
     }
     if (propp)
         *propp = (JSProperty *) sprop;
     else
         JS_UNLOCK_OBJ(cx, obj);
     return JS_TRUE;
 
 bad:
@@ -4004,25 +4006,24 @@ out:
     return protoIndex;
 }
 
 int
 js_FindPropertyHelper(JSContext *cx, jsid id, JSObject **objp,
                       JSObject **pobjp, JSProperty **propp,
                       JSPropCacheEntry **entryp)
 {
-    JSObject *obj, *pobj, *lastobj;
-    uint32 shape;
+    JSObject *scopeChain, *obj, *pobj, *lastobj;
     int scopeIndex, protoIndex;
     JSProperty *prop;
     JSScopeProperty *sprop;
 
     JS_ASSERT_IF(entryp, !JS_ON_TRACE(cx));
-    obj = js_GetTopStackFrame(cx)->scopeChain;
-    shape = OBJ_SHAPE(obj);
+    scopeChain = js_GetTopStackFrame(cx)->scopeChain;
+    obj = scopeChain;
     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 {
@@ -4030,18 +4031,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, cx->fp->scopeChain, shape,
-                                         scopeIndex, protoIndex, pobj, sprop,
+                    js_FillPropertyCache(cx, scopeChain, scopeIndex,
+                                         protoIndex, pobj, sprop, false,
                                          entryp);
                 } else {
                     PCMETER(JS_PROPERTY_CACHE(cx).nofills++);
                     *entryp = NULL;
                 }
             }
             SCOPE_DEPTH_ACCUM(&rt->scopeSearchDepthStats, scopeIndex);
             *objp = obj;
@@ -4113,19 +4114,18 @@ 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, OBJ_SHAPE(scopeChain),
-                                 scopeIndex, protoIndex, pobj,
-                                 (JSScopeProperty *) prop, &entry);
+            js_FillPropertyCache(cx, scopeChain, scopeIndex, protoIndex, pobj,
+                                 (JSScopeProperty *) prop, false, &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,130 +4154,141 @@ 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));
-    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_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);
+    }
+
+    *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);
-    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);
-    }
 
     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));
-    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 {
+    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);
+
         /*
          * 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;
         }
-    }
-
-    sample = cx->runtime->propertyRemovals;
+        return js_SetSprop(cx, sprop, obj, vp);
+    }
+
+    OBJ_CHECK_SLOT(obj, slot);
+
+    if (SPROP_HAS_STUB_SETTER(sprop)) {
+        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);
-    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);
-    }
 
     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))
@@ -4341,25 +4352,24 @@ 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, shape, 0, protoIndex, obj2, sprop, entryp);
-    }
-    JS_UNLOCK_OBJ(cx, obj2);
-    return JS_TRUE;
+        js_FillPropertyCache(cx, aobj, 0, protoIndex, obj2, sprop, false,
+                             entryp);
+    }
+
+    /* The following unlocks the object. */
+    return js_NativeGet(cx, obj, obj2, sprop, vp);
 }
 
 JSBool
 js_GetProperty(JSContext *cx, JSObject *obj, jsid id, jsval *vp)
 {
     return js_GetPropertyHelper(cx, obj, id, vp, NULL);
 }
 
@@ -4378,40 +4388,39 @@ 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;
@@ -4542,16 +4551,17 @@ 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. */
@@ -4559,17 +4569,18 @@ 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);
+                                    SPROP_INVALID_SLOT, attrs, flags, shortid,
+                                    &cacheByPrevShape);
         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
@@ -4580,28 +4591,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, shape, 0, 0, obj, sprop, entryp);
-        else
+        if (!(attrs & JSPROP_SHARED)) {
+            js_FillPropertyCache(cx, obj, 0, 0, obj, sprop, cacheByPrevShape,
+                                 entryp);
+        } else {
             PCMETER(JS_PROPERTY_CACHE(cx).nofills++);
-    }
-    JS_UNLOCK_SCOPE(cx, scope);
-    return JS_TRUE;
+        }
+    }
+
+    /* The following unlocks the object. */
+    return js_NativeSet(cx, obj, sprop, vp);
 
   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,25 +683,29 @@ 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);
 
 /*
- * 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.
+ * 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.
  */
 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,24 +986,26 @@ 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)
+                    uintN attrs, uintN flags, intN shortid,
+                    JSBool *cacheByPrevShape)
 {
     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.
      */
@@ -1267,18 +1269,27 @@ 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.
          */
-        SCOPE_EXTEND_SHAPE(cx, scope, sprop);
+        if (!scope->lastProp || scope->shape == scope->lastProp->shape) {
+            scope->shape = sprop->shape;
+            if (cacheByPrevShape)
+                *cacheByPrevShape = true;
+        } else {
+            scope->shape = js_GenerateShape(cx, false, 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
@@ -1404,17 +1415,18 @@ 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);
+                                       child.attrs, child.flags, child.shortid,
+                                       NULL);
     }
 
     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,20 +414,26 @@ 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);
+                    uintN attrs, uintN flags, intN shortid,
+                    JSBool *cacheByPrevShape);
 
 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, OBJ_SHAPE(aobj), 0, protoIndex, obj2,
-                                     (JSScopeProperty*) prop, &entry);
+                js_FillPropertyCache(cx, aobj, 0, protoIndex, obj2,
+                                     (JSScopeProperty*) prop, false, &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;