Bug 1119288 part 3 - Add a ShapeTable::Entry class and use it instead of raw Shape** pointers. r=njn
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 09 Jan 2015 14:31:59 +0100
changeset 248834 61fdafc3b45c035674d2f55be85adc0328c74bf6
parent 248833 82be31c150f52f109346ee6a8e962e1f7d5af651
child 248835 936382a9e919c911ec18daf5452129b7a361a00b
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs1119288
milestone37.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1119288 part 3 - Add a ShapeTable::Entry class and use it instead of raw Shape** pointers. r=njn
js/src/vm/NativeObject.cpp
js/src/vm/NativeObject.h
js/src/vm/ScopeObject.cpp
js/src/vm/Shape-inl.h
js/src/vm/Shape.cpp
js/src/vm/Shape.h
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -179,18 +179,18 @@ js::NativeObject::checkShapeConsistency(
              fslot = getSlot(fslot).toPrivateUint32())
         {
             MOZ_ASSERT(fslot < slotSpan());
         }
 
         for (int n = throttle; --n >= 0 && shape->parent; shape = shape->parent) {
             MOZ_ASSERT_IF(lastProperty() != shape, !shape->hasTable());
 
-            Shape **spp = table.search(shape->propid(), false);
-            MOZ_ASSERT(SHAPE_FETCH(spp) == shape);
+            ShapeTable::Entry &entry = table.search(shape->propid(), false);
+            MOZ_ASSERT(entry.shape() == shape);
         }
 
         shape = lastProperty();
         for (int n = throttle; --n >= 0 && shape; shape = shape->parent) {
             MOZ_ASSERT_IF(shape->slot() != SHAPE_INVALID_SLOT, shape->slot() < slotSpan());
             if (!prev) {
                 MOZ_ASSERT(lastProperty() == shape);
                 MOZ_ASSERT(shape->listp == &shape_);
@@ -200,18 +200,18 @@ js::NativeObject::checkShapeConsistency(
             prev = shape;
         }
     } else {
         for (int n = throttle; --n >= 0 && shape->parent; shape = shape->parent) {
             if (shape->hasTable()) {
                 ShapeTable &table = shape->table();
                 MOZ_ASSERT(shape->parent);
                 for (Shape::Range<NoGC> r(shape); !r.empty(); r.popFront()) {
-                    Shape **spp = table.search(r.front().propid(), false);
-                    MOZ_ASSERT(SHAPE_FETCH(spp) == &r.front());
+                    ShapeTable::Entry &entry = table.search(r.front().propid(), false);
+                    MOZ_ASSERT(entry.shape() == &r.front());
                 }
             }
             if (prev) {
                 MOZ_ASSERT(prev->maybeSlot() >= shape->maybeSlot());
                 shape->kids.checkConsistency(prev);
             }
             prev = shape;
         }
@@ -277,18 +277,18 @@ js::NativeObject::slotInRange(uint32_t s
  * up the stack.
  */
 MOZ_NEVER_INLINE
 #endif
 Shape *
 js::NativeObject::lookup(ExclusiveContext *cx, jsid id)
 {
     MOZ_ASSERT(isNative());
-    Shape **spp;
-    return Shape::search(cx, lastProperty(), id, &spp);
+    ShapeTable::Entry *entry;
+    return Shape::search(cx, lastProperty(), id, &entry);
 }
 
 Shape *
 js::NativeObject::lookupPure(jsid id)
 {
     MOZ_ASSERT(isNative());
     return Shape::searchNoHashify(lastProperty(), id);
 }
--- a/js/src/vm/NativeObject.h
+++ b/js/src/vm/NativeObject.h
@@ -684,17 +684,17 @@ class NativeObject : public JSObject
      *
      * Notes:
      * 1. getter and setter must be normalized based on flags (see jsscope.cpp).
      * 2. Checks for non-extensibility must be done by callers.
      */
     static Shape *
     addPropertyInternal(ExclusiveContext *cx, HandleNativeObject obj, HandleId id,
                         JSPropertyOp getter, JSStrictPropertyOp setter,
-                        uint32_t slot, unsigned attrs, unsigned flags, Shape **spp,
+                        uint32_t slot, unsigned attrs, unsigned flags, ShapeTable::Entry *entry,
                         bool allowDictionary);
 
     void fillInAfterSwap(JSContext *cx, const Vector<Value> &values, void *priv);
 
   public:
     // Return true if this object has been converted from shared-immutable
     // prototype-rooted shape storage to dictionary-shapes in a doubly-linked
     // list.
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -695,18 +695,18 @@ StaticBlockObject::addVar(ExclusiveConte
                           bool constant, unsigned index, bool *redeclared)
 {
     MOZ_ASSERT(JSID_IS_ATOM(id));
     MOZ_ASSERT(index < LOCAL_INDEX_LIMIT);
 
     *redeclared = false;
 
     /* Inline NativeObject::addProperty in order to trap the redefinition case. */
-    Shape **spp;
-    if (Shape::search(cx, block->lastProperty(), id, &spp, true)) {
+    ShapeTable::Entry *entry;
+    if (Shape::search(cx, block->lastProperty(), id, &entry, true)) {
         *redeclared = true;
         return nullptr;
     }
 
     /*
      * Don't convert this object to dictionary mode so that we can clone the
      * block's shape later.
      */
@@ -714,17 +714,17 @@ StaticBlockObject::addVar(ExclusiveConte
     uint32_t readonly = constant ? JSPROP_READONLY : 0;
     uint32_t propFlags = readonly | JSPROP_ENUMERATE | JSPROP_PERMANENT;
     return NativeObject::addPropertyInternal(cx, block, id,
                                              /* getter = */ nullptr,
                                              /* setter = */ nullptr,
                                              slot,
                                              propFlags,
                                              /* attrs = */ 0,
-                                             spp,
+                                             entry,
                                              /* allowDictionary = */ false);
 }
 
 const Class BlockObject::class_ = {
     "Block",
     JSCLASS_IMPLEMENTS_BARRIERS |
     JSCLASS_HAS_RESERVED_SLOTS(BlockObject::RESERVED_SLOTS) |
     JSCLASS_IS_ANONYMOUS
--- a/js/src/vm/Shape-inl.h
+++ b/js/src/vm/Shape-inl.h
@@ -46,17 +46,17 @@ Shape::get(JSContext* cx, HandleObject r
 
     RootedId id(cx, propid());
     return CallJSPropertyOp(cx, getterOp(), receiver, id, vp);
 }
 
 inline Shape *
 Shape::search(ExclusiveContext *cx, jsid id)
 {
-    Shape **_;
+    ShapeTable::Entry *_;
     return search(cx, this, id, &_);
 }
 
 inline bool
 Shape::set(JSContext* cx, HandleObject obj, HandleObject receiver, bool strict,
            MutableHandleValue vp)
 {
     MOZ_ASSERT_IF(hasDefaultSetter(), hasGetterValue());
@@ -82,35 +82,35 @@ Shape::set(JSContext* cx, HandleObject o
         RootedObject nobj(cx, &obj->as<DynamicWithObject>().object());
         return CallJSPropertyOpSetter(cx, setterOp(), nobj, id, strict, vp);
     }
 
     return CallJSPropertyOpSetter(cx, setterOp(), obj, id, strict, vp);
 }
 
 /* static */ inline Shape *
-Shape::search(ExclusiveContext *cx, Shape *start, jsid id, Shape ***pspp, bool adding)
+Shape::search(ExclusiveContext *cx, Shape *start, jsid id, ShapeTable::Entry **pentry, bool adding)
 {
     if (start->inDictionary()) {
-        *pspp = start->table().search(id, adding);
-        return SHAPE_FETCH(*pspp);
+        *pentry = &start->table().search(id, adding);
+        return (*pentry)->shape();
     }
 
-    *pspp = nullptr;
+    *pentry = nullptr;
 
     if (start->hasTable()) {
-        Shape **spp = start->table().search(id, adding);
-        return SHAPE_FETCH(spp);
+        ShapeTable::Entry &entry = start->table().search(id, adding);
+        return entry.shape();
     }
 
     if (start->numLinearSearches() == LINEAR_SEARCHES_MAX) {
         if (start->isBigEnoughForAShapeTable()) {
             if (Shape::hashify(cx, start)) {
-                Shape **spp = start->table().search(id, adding);
-                return SHAPE_FETCH(spp);
+                ShapeTable::Entry &entry = start->table().search(id, adding);
+                return entry.shape();
             } else {
                 cx->recoverFromOutOfMemory();
             }
         }
         /*
          * No table built -- there weren't enough entries, or OOM occurred.
          * Don't increment numLinearSearches, to keep hasTable() false.
          */
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -28,46 +28,48 @@
 using namespace js;
 using namespace js::gc;
 
 using mozilla::CeilingLog2Size;
 using mozilla::DebugOnly;
 using mozilla::PodZero;
 using mozilla::RotateLeft;
 
+Shape *const ShapeTable::Entry::SHAPE_REMOVED = (Shape *)ShapeTable::Entry::SHAPE_COLLISION;
+
 bool
 ShapeTable::init(ExclusiveContext *cx, Shape *lastProp)
 {
     uint32_t sizeLog2 = CeilingLog2Size(entryCount_);
     uint32_t size = JS_BIT(sizeLog2);
     if (entryCount_ >= size - (size >> 2))
         sizeLog2++;
     if (sizeLog2 < MIN_SIZE_LOG2)
         sizeLog2 = MIN_SIZE_LOG2;
 
     /*
      * Use rt->calloc for memory accounting and overpressure handling
      * without OOM reporting. See ShapeTable::change.
      */
-    entries_ = cx->pod_calloc<Shape *>(JS_BIT(sizeLog2));
+    entries_ = cx->pod_calloc<Entry>(JS_BIT(sizeLog2));
     if (!entries_)
         return false;
 
     hashShift_ = HASH_BITS - sizeLog2;
     for (Shape::Range<NoGC> r(lastProp); !r.empty(); r.popFront()) {
         Shape &shape = r.front();
         MOZ_ASSERT(cx->isThreadLocal(&shape));
-        Shape **spp = search(shape.propid(), true);
+        Entry &entry = search(shape.propid(), true);
 
         /*
          * Beware duplicate args and arg vs. var conflicts: the youngest shape
          * (nearest to lastProp) must win. See bug 600067.
          */
-        if (!SHAPE_FETCH(spp))
-            SHAPE_STORE_PRESERVING_COLLISION(spp, &shape);
+        if (!entry.shape())
+            entry.setPreservingCollision(&shape);
     }
     return true;
 }
 
 void
 Shape::removeFromDictionary(NativeObject *obj)
 {
     MOZ_ASSERT(inDictionary());
@@ -173,133 +175,132 @@ Hash1(HashNumber hash0, int shift)
 }
 
 static HashNumber
 Hash2(HashNumber hash0, int log2, int shift)
 {
     return ((hash0 << log2) >> shift) | 1;
 }
 
-Shape **
+ShapeTable::Entry &
 ShapeTable::search(jsid id, bool adding)
 {
     MOZ_ASSERT(entries_);
     MOZ_ASSERT(!JSID_IS_EMPTY(id));
 
     /* Compute the primary hash address. */
     HashNumber hash0 = HashId(id);
     HashNumber hash1 = Hash1(hash0, hashShift_);
-    Shape **spp = entries_ + hash1;
+    Entry *entry = entries_ + hash1;
 
     /* Miss: return space for a new entry. */
-    Shape *stored = *spp;
-    if (SHAPE_IS_FREE(stored))
-        return spp;
+    if (entry->isFree())
+        return *entry;
 
     /* Hit: return entry. */
-    Shape *shape = SHAPE_CLEAR_COLLISION(stored);
+    Shape *shape = entry->shape();
     if (shape && shape->propidRaw() == id)
-        return spp;
+        return *entry;
 
     /* Collision: double hash. */
     int sizeLog2 = HASH_BITS - hashShift_;
     HashNumber hash2 = Hash2(hash0, sizeLog2, hashShift_);
     uint32_t sizeMask = JS_BITMASK(sizeLog2);
 
 #ifdef DEBUG
-    uintptr_t collision_flag = SHAPE_COLLISION;
+    bool collisionFlag = true;
 #endif
 
     /* Save the first removed entry pointer so we can recycle it if adding. */
-    Shape **firstRemoved;
-    if (SHAPE_IS_REMOVED(stored)) {
-        firstRemoved = spp;
+    Entry *firstRemoved;
+    if (entry->isRemoved()) {
+        firstRemoved = entry;
     } else {
         firstRemoved = nullptr;
-        if (adding && !SHAPE_HAD_COLLISION(stored))
-            SHAPE_FLAG_COLLISION(spp, shape);
+        if (adding && !entry->hadCollision())
+            entry->flagCollision();
 #ifdef DEBUG
-        collision_flag &= uintptr_t(*spp) & SHAPE_COLLISION;
+        collisionFlag &= entry->hadCollision();
 #endif
     }
 
     for (;;) {
         hash1 -= hash2;
         hash1 &= sizeMask;
-        spp = entries_ + hash1;
+        entry = entries_ + hash1;
+
+        if (entry->isFree())
+            return (adding && firstRemoved) ? *firstRemoved : *entry;
 
-        stored = *spp;
-        if (SHAPE_IS_FREE(stored))
-            return (adding && firstRemoved) ? firstRemoved : spp;
-
-        shape = SHAPE_CLEAR_COLLISION(stored);
+        shape = entry->shape();
         if (shape && shape->propidRaw() == id) {
-            MOZ_ASSERT(collision_flag);
-            return spp;
+            MOZ_ASSERT(collisionFlag);
+            return *entry;
         }
 
-        if (SHAPE_IS_REMOVED(stored)) {
+        if (entry->isRemoved()) {
             if (!firstRemoved)
-                firstRemoved = spp;
+                firstRemoved = entry;
         } else {
-            if (adding && !SHAPE_HAD_COLLISION(stored))
-                SHAPE_FLAG_COLLISION(spp, shape);
+            if (adding && !entry->hadCollision())
+                entry->flagCollision();
 #ifdef DEBUG
-            collision_flag &= uintptr_t(*spp) & SHAPE_COLLISION;
+            collisionFlag &= entry->hadCollision();
 #endif
         }
     }
 
     MOZ_CRASH("Shape::search failed to find an expected entry.");
 }
 
 #ifdef JSGC_COMPACTING
 void
 ShapeTable::fixupAfterMovingGC()
 {
     int log2 = HASH_BITS - hashShift_;
     uint32_t size = JS_BIT(log2);
     for (HashNumber i = 0; i < size; i++) {
-        Shape *shape = SHAPE_FETCH(&entries_[i]);
+        Entry &entry = entries_[i];
+        Shape *shape = entry.shape();
         if (shape && IsForwarded(shape))
-            SHAPE_STORE_PRESERVING_COLLISION(&entries_[i], Forwarded(shape));
+            entry.setPreservingCollision(Forwarded(shape));
     }
 }
 #endif
 
 bool
 ShapeTable::change(int log2Delta, ExclusiveContext *cx)
 {
     MOZ_ASSERT(entries_);
 
     /*
      * Grow, shrink, or compress by changing this->entries_.
      */
     int oldlog2 = HASH_BITS - hashShift_;
     int newlog2 = oldlog2 + log2Delta;
     uint32_t oldsize = JS_BIT(oldlog2);
     uint32_t newsize = JS_BIT(newlog2);
-    Shape **newTable = cx->pod_calloc<Shape *>(newsize);
+    Entry *newTable = cx->pod_calloc<Entry>(newsize);
     if (!newTable)
         return false;
 
     /* Now that we have newTable allocated, update members. */
     hashShift_ = HASH_BITS - newlog2;
     removedCount_ = 0;
-    Shape **oldTable = entries_;
+    Entry *oldTable = entries_;
     entries_ = newTable;
 
     /* Copy only live entries, leaving removed and free ones behind. */
-    for (Shape **oldspp = oldTable; oldsize != 0; oldspp++) {
-        Shape *shape = SHAPE_FETCH(oldspp);
+    for (Entry *oldEntry = oldTable; oldsize != 0; oldEntry++) {
+        Shape *shape = oldEntry->shape();
         MOZ_ASSERT(cx->isThreadLocal(shape));
         if (shape) {
-            Shape **spp = search(shape->propid(), true);
-            MOZ_ASSERT(SHAPE_IS_FREE(*spp));
-            *spp = shape;
+            Entry &entry = search(shape->propid(), true);
+            MOZ_ASSERT(entry.isFree());
+            entry.setShape(shape);
         }
         oldsize--;
     }
 
     /* Finally, free the old entries storage. */
     js_free(oldTable);
     return true;
 }
@@ -497,21 +498,21 @@ NativeObject::addProperty(ExclusiveConte
     if (!JSObject::isExtensible(cx, obj, &extensible))
         return nullptr;
     if (!extensible) {
         if (cx->isJSContext())
             obj->reportNotExtensible(cx->asJSContext());
         return nullptr;
     }
 
-    Shape **spp = nullptr;
+    ShapeTable::Entry *entry = nullptr;
     if (obj->inDictionaryMode())
-        spp = obj->lastProperty()->table().search(id, true);
+        entry = &obj->lastProperty()->table().search(id, true);
 
-    return addPropertyInternal(cx, obj, id, getter, setter, slot, attrs, flags, spp,
+    return addPropertyInternal(cx, obj, id, getter, setter, slot, attrs, flags, entry,
                                allowDictionary);
 }
 
 static bool
 ShouldConvertToDictionary(JSObject *obj)
 {
     /*
      * Use a lower limit if this object is likely a hashmap (SETELEM was used
@@ -522,17 +523,17 @@ ShouldConvertToDictionary(JSObject *obj)
     return obj->lastProperty()->entryCount() >= PropertyTree::MAX_HEIGHT;
 }
 
 /* static */ Shape *
 NativeObject::addPropertyInternal(ExclusiveContext *cx,
                                   HandleNativeObject obj, HandleId id,
                                   PropertyOp getter, StrictPropertyOp setter,
                                   uint32_t slot, unsigned attrs,
-                                  unsigned flags, Shape **spp,
+                                  unsigned flags, ShapeTable::Entry *entry,
                                   bool allowDictionary)
 {
     MOZ_ASSERT(cx->isThreadLocal(obj));
     MOZ_ASSERT_IF(!allowDictionary, !obj->inDictionaryMode());
     MOZ_ASSERT(getter != JS_PropertyStub);
     MOZ_ASSERT(setter != JS_StrictPropertyStub);
 
     AutoRooterGetterSetter gsRoot(cx, attrs, &getter, &setter);
@@ -550,29 +551,29 @@ NativeObject::addPropertyInternal(Exclus
             (slot == obj->lastProperty()->maybeSlot() + 1);
         MOZ_ASSERT_IF(!allowDictionary, stableSlot);
         if (allowDictionary &&
             (!stableSlot || ShouldConvertToDictionary(obj)))
         {
             if (!obj->toDictionaryMode(cx))
                 return nullptr;
             table = &obj->lastProperty()->table();
-            spp = table->search(id, true);
+            entry = &table->search(id, true);
         }
     } else {
         table = &obj->lastProperty()->table();
         if (table->needsToGrow()) {
             if (!table->grow(cx))
                 return nullptr;
-            spp = table->search(id, true);
-            MOZ_ASSERT(!SHAPE_FETCH(spp));
+            entry = &table->search(id, true);
+            MOZ_ASSERT(!entry->shape());
         }
     }
 
-    MOZ_ASSERT(!!table == !!spp);
+    MOZ_ASSERT(!!table == !!entry);
 
     /* Find or create a property tree node labeled by our arguments. */
     RootedShape shape(cx);
     {
         RootedShape last(cx, obj->lastProperty());
 
         uint32_t index;
         bool indexed = js_IdIsIndex(id, &index);
@@ -593,17 +594,17 @@ NativeObject::addPropertyInternal(Exclus
         shape = getChildProperty(cx, obj, last, child);
     }
 
     if (shape) {
         MOZ_ASSERT(shape == obj->lastProperty());
 
         if (table) {
             /* Store the tree node pointer in the table entry for id. */
-            SHAPE_STORE_PRESERVING_COLLISION(spp, static_cast<Shape *>(shape));
+            entry->setPreservingCollision(shape);
             table->incEntryCount();
 
             /* Pass the table along to the new last property, namely shape. */
             MOZ_ASSERT(&shape->parent->table() == table);
             shape->parent->handoffTableTo(shape);
         }
 
         obj->checkShapeConsistency();
@@ -721,18 +722,18 @@ NativeObject::putProperty(ExclusiveConte
      *
      * Note that we can only try to claim an entry in a table that is thread
      * local. An object may be thread local *without* its shape being thread
      * local. The only thread local objects that *also* have thread local
      * shapes are dictionaries that were allocated/converted thread
      * locally. Only for those objects we can try to claim an entry in its
      * shape table.
      */
-    Shape **spp;
-    RootedShape shape(cx, Shape::search(cx, obj->lastProperty(), id, &spp, true));
+    ShapeTable::Entry *entry;
+    RootedShape shape(cx, Shape::search(cx, obj->lastProperty(), id, &entry, true));
     if (!shape) {
         /*
          * You can't add properties to a non-extensible object, but you can change
          * attributes of properties in such objects.
          */
         bool extensible;
 
         if (!JSObject::isExtensible(cx, obj, &extensible))
@@ -740,21 +741,21 @@ NativeObject::putProperty(ExclusiveConte
 
         if (!extensible) {
             if (cx->isJSContext())
                 obj->reportNotExtensible(cx->asJSContext());
             return nullptr;
         }
 
         return addPropertyInternal(cx, obj, id, getter, setter, slot, attrs, flags,
-                                   spp, true);
+                                   entry, true);
     }
 
-    /* Property exists: search must have returned a valid *spp. */
-    MOZ_ASSERT_IF(spp, !SHAPE_IS_REMOVED(*spp));
+    /* Property exists: search must have returned a valid entry. */
+    MOZ_ASSERT_IF(entry, !entry->isRemoved());
 
     if (!CheckCanChangeAttrs(cx, obj, shape, &attrs))
         return nullptr;
 
     /*
      * If the caller wants to allocate a slot, but doesn't care which slot,
      * copy the existing shape's slot into slot so we can match shape, if all
      * other members match.
@@ -786,18 +787,18 @@ NativeObject::putProperty(ExclusiveConte
     /*
      * Overwriting a non-last property requires switching to dictionary mode.
      * The shape tree is shared immutable, and we can't removeProperty and then
      * addPropertyInternal because a failure under add would lose data.
      */
     if (shape != obj->lastProperty() && !obj->inDictionaryMode()) {
         if (!obj->toDictionaryMode(cx))
             return nullptr;
-        spp = obj->lastProperty()->table().search(shape->propid(), false);
-        shape = SHAPE_FETCH(spp);
+        entry = &obj->lastProperty()->table().search(shape->propid(), false);
+        shape = entry->shape();
     }
 
     MOZ_ASSERT_IF(shape->hasSlot() && !(attrs & JSPROP_SHARED), shape->slot() == slot);
 
     if (obj->inDictionaryMode()) {
         /*
          * Updating some property in a dictionary-mode object. Create a new
          * shape for the existing property, and also generate a new shape for
@@ -926,30 +927,30 @@ NativeObject::changeProperty(ExclusiveCo
 }
 
 bool
 NativeObject::removeProperty(ExclusiveContext *cx, jsid id_)
 {
     RootedId id(cx, id_);
     RootedNativeObject self(cx, this);
 
-    Shape **spp;
-    RootedShape shape(cx, Shape::search(cx, lastProperty(), id, &spp));
+    ShapeTable::Entry *entry;
+    RootedShape shape(cx, Shape::search(cx, lastProperty(), id, &entry));
     if (!shape)
         return true;
 
     /*
      * If shape is not the last property added, or the last property cannot
      * be removed, switch to dictionary mode.
      */
     if (!self->inDictionaryMode() && (shape != self->lastProperty() || !self->canRemoveLastProperty())) {
         if (!self->toDictionaryMode(cx))
             return false;
-        spp = self->lastProperty()->table().search(shape->propid(), false);
-        shape = SHAPE_FETCH(spp);
+        entry = &self->lastProperty()->table().search(shape->propid(), false);
+        shape = entry->shape();
     }
 
     /*
      * If in dictionary mode, get a new shape for the last property after the
      * removal. We need a fresh shape for all dictionary deletions, even of
      * the last property. Otherwise, a shape could replay and caches might
      * return deleted DictionaryShapes! See bug 595365. Do this before changing
      * the object or table, so the remaining removal is infallible.
@@ -987,22 +988,22 @@ NativeObject::removeProperty(ExclusiveCo
     /*
      * A dictionary-mode object owns mutable, unique shapes on a non-circular
      * doubly linked list, hashed by lastProperty()->table. So we can edit the
      * list and hash in place.
      */
     if (self->inDictionaryMode()) {
         ShapeTable &table = self->lastProperty()->table();
 
-        if (SHAPE_HAD_COLLISION(*spp)) {
-            *spp = SHAPE_REMOVED;
+        if (entry->hadCollision()) {
+            entry->setRemoved();
             table.incRemovedCount();
             table.decEntryCount();
         } else {
-            *spp = nullptr;
+            entry->setFree();
             table.decEntryCount();
 
 #ifdef DEBUG
             /*
              * Check the consistency of the table but limit the number of
              * checks not to alter significantly the complexity of the
              * delete in debug builds, see bug 534493.
              */
@@ -1121,35 +1122,35 @@ NativeObject::replaceWithNewEquivalentSh
         if (!newShape)
             return nullptr;
         new (newShape) Shape(oldRoot->base()->unowned(), 0);
         self = selfRoot;
         oldShape = oldRoot;
     }
 
     ShapeTable &table = self->lastProperty()->table();
-    Shape **spp = oldShape->isEmptyShape()
-                  ? nullptr
-                  : table.search(oldShape->propidRef(), false);
+    ShapeTable::Entry *entry = oldShape->isEmptyShape()
+                               ? nullptr
+                               : &table.search(oldShape->propidRef(), false);
 
     /*
      * Splice the new shape into the same position as the old shape, preserving
      * enumeration order (see bug 601399).
      */
     StackShape nshape(oldShape);
     newShape->initDictionaryShape(nshape, self->numFixedSlots(), oldShape->listp);
 
     MOZ_ASSERT(newShape->parent == oldShape);
     oldShape->removeFromDictionary(self);
 
     if (newShape == self->lastProperty())
         oldShape->handoffTableTo(newShape);
 
-    if (spp)
-        SHAPE_STORE_PRESERVING_COLLISION(spp, newShape);
+    if (entry)
+        entry->setPreservingCollision(newShape);
     return newShape;
 }
 
 bool
 NativeObject::shadowingShapeChange(ExclusiveContext *cx, const Shape &shape)
 {
     return generateOwnShape(cx);
 }
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -127,16 +127,55 @@ static const uint32_t SHAPE_MAXIMUM_SLOT
  * Shapes use multiplicative hashing, but specialized to
  * minimize footprint.
  */
 class ShapeTable {
   public:
     friend class NativeObject;
     static const uint32_t MIN_ENTRIES   = 11;
 
+    class Entry {
+        // js::Shape pointer tag bit indicating a collision.
+        static const uintptr_t SHAPE_COLLISION = 1;
+        static Shape *const SHAPE_REMOVED; // = SHAPE_COLLISION
+
+        Shape *shape_;
+
+        Entry() MOZ_DELETE;
+        Entry(const Entry&) MOZ_DELETE;
+        Entry& operator=(const Entry&) MOZ_DELETE;
+
+      public:
+        bool isFree() const { return shape_ == nullptr; }
+        bool isRemoved() const { return shape_ == SHAPE_REMOVED; }
+        bool hadCollision() const { return uintptr_t(shape_) & SHAPE_COLLISION; }
+
+        void setFree() { shape_ = nullptr; }
+        void setRemoved() { shape_ = SHAPE_REMOVED; }
+
+        Shape *shape() const {
+            return reinterpret_cast<Shape*>(uintptr_t(shape_) & ~SHAPE_COLLISION);
+        }
+
+        void setShape(Shape *shape) {
+            MOZ_ASSERT(isFree());
+            MOZ_ASSERT(shape);
+            MOZ_ASSERT(shape != SHAPE_REMOVED);
+            shape_ = shape;
+            MOZ_ASSERT(!hadCollision());
+        }
+
+        void flagCollision() {
+            shape_ = reinterpret_cast<Shape*>(uintptr_t(shape_) | SHAPE_COLLISION);
+        }
+        void setPreservingCollision(Shape *shape) {
+            shape_ = reinterpret_cast<Shape*>(uintptr_t(shape) | uintptr_t(hadCollision()));
+        }
+    };
+
   private:
     static const uint32_t HASH_BITS     = mozilla::tl::BitSize<HashNumber>::value;
 
     // This value is low because it's common for a ShapeTable to be created
     // with an entryCount of zero.
     static const uint32_t MIN_SIZE_LOG2 = 2;
     static const uint32_t MIN_SIZE      = JS_BIT(MIN_SIZE_LOG2);
 
@@ -144,17 +183,17 @@ class ShapeTable {
 
     uint32_t        entryCount_;        /* number of entries in table */
     uint32_t        removedCount_;      /* removed entry sentinels in table */
 
     uint32_t        freeList_;          /* SHAPE_INVALID_SLOT or head of slot
                                            freelist in owning dictionary-mode
                                            object */
 
-    js::Shape       **entries_;         /* table of ptrs to shared tree nodes */
+    Entry           *entries_;          /* table of ptrs to shared tree nodes */
 
   public:
     explicit ShapeTable(uint32_t nentries)
       : hashShift_(HASH_BITS - MIN_SIZE_LOG2),
         entryCount_(nentries),
         removedCount_(0),
         freeList_(SHAPE_INVALID_SLOT),
         entries_(nullptr)
@@ -181,17 +220,17 @@ class ShapeTable {
 
     /*
      * NB: init and change are fallible but do not report OOM, so callers can
      * cope or ignore. They do however use the context's calloc method in
      * order to update the malloc counter on success.
      */
     bool init(ExclusiveContext *cx, Shape *lastProp);
     bool change(int log2Delta, ExclusiveContext *cx);
-    Shape **search(jsid id, bool adding);
+    Entry &search(jsid id, bool adding);
 
 #ifdef JSGC_COMPACTING
     /* Update entries whose shapes have been moved */
     void fixupAfterMovingGC();
 #endif
 
   private:
     void decEntryCount() {
@@ -634,17 +673,17 @@ class Shape : public gc::TenuredCell
                                    to many-kids data structure */
         HeapPtrShape *listp;    /* dictionary list starting at shape_
                                    has a double-indirect back pointer,
                                    either to the next shape's parent if not
                                    last, else to obj->shape_ */
     };
 
     static inline Shape *search(ExclusiveContext *cx, Shape *start, jsid id,
-                                Shape ***pspp, bool adding = false);
+                                ShapeTable::Entry **pentry, bool adding = false);
     static inline Shape *searchNoHashify(Shape *start, jsid id);
 
     void removeFromDictionary(NativeObject *obj);
     void insertIntoDictionary(HeapPtrShape *dictp);
 
     inline void initDictionaryShape(const StackShape &child, uint32_t nfixed, HeapPtrShape *dictp);
 
     /* Replace the base shape of the last shape in a non-dictionary lineage with base. */
@@ -1292,69 +1331,16 @@ struct StackShape
         hash = mozilla::RotateLeft(hash, 4) ^ uintptr_t(rawSetter);
         return hash;
     }
 
     // For RootedGeneric<StackShape*>
     void trace(JSTracer *trc);
 };
 
-} /* namespace js */
-
-/* js::Shape pointer tag bit indicating a collision. */
-#define SHAPE_COLLISION                 (uintptr_t(1))
-#define SHAPE_REMOVED                   ((js::Shape *) SHAPE_COLLISION)
-
-/* Functions to get and set shape pointer values and collision flags. */
-
-inline bool
-SHAPE_IS_FREE(js::Shape *shape)
-{
-    return shape == nullptr;
-}
-
-inline bool
-SHAPE_IS_REMOVED(js::Shape *shape)
-{
-    return shape == SHAPE_REMOVED;
-}
-
-inline void
-SHAPE_FLAG_COLLISION(js::Shape **spp, js::Shape *shape)
-{
-    *spp = reinterpret_cast<js::Shape*>(uintptr_t(shape) | SHAPE_COLLISION);
-}
-
-inline bool
-SHAPE_HAD_COLLISION(js::Shape *shape)
-{
-    return uintptr_t(shape) & SHAPE_COLLISION;
-}
-
-inline js::Shape *
-SHAPE_CLEAR_COLLISION(js::Shape *shape)
-{
-    return reinterpret_cast<js::Shape*>(uintptr_t(shape) & ~SHAPE_COLLISION);
-}
-
-inline js::Shape *
-SHAPE_FETCH(js::Shape **spp)
-{
-    return SHAPE_CLEAR_COLLISION(*spp);
-}
-
-inline void
-SHAPE_STORE_PRESERVING_COLLISION(js::Shape **spp, js::Shape *shape)
-{
-    *spp = reinterpret_cast<js::Shape*>(uintptr_t(shape) |
-                                        uintptr_t(SHAPE_HAD_COLLISION(*spp)));
-}
-
-namespace js {
-
 inline
 Shape::Shape(const StackShape &other, uint32_t nfixed)
   : base_(other.base),
     propid_(other.propid),
     slotInfo(other.maybeSlot() | (nfixed << FIXED_SLOTS_SHIFT)),
     attrs(other.attrs),
     flags(other.flags),
     parent(nullptr)
@@ -1478,18 +1464,18 @@ Shape::markChildren(JSTracer *trc)
 inline Shape *
 Shape::searchNoHashify(Shape *start, jsid id)
 {
     /*
      * If we have a table, search in the shape table, else do a linear
      * search. We never hashify into a table in parallel.
      */
     if (start->hasTable()) {
-        Shape **spp = start->table().search(id, false);
-        return SHAPE_FETCH(spp);
+        ShapeTable::Entry &entry = start->table().search(id, false);
+        return entry.shape();
     }
 
     return start->searchLinear(id);
 }
 
 inline bool
 Shape::matches(const StackShape &other) const
 {