Bug 1119288 part 2 - Turn ShapeTable into a class with private members. r=njn
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 09 Jan 2015 14:31:56 +0100
changeset 248833 82be31c150f52f109346ee6a8e962e1f7d5af651
parent 248832 49f2b5bf30c579adc5a2f1bc14bc276ef2eea3b8
child 248834 61fdafc3b45c035674d2f55be85adc0328c74bf6
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 2 - Turn ShapeTable into a class with private members. r=njn
js/src/vm/NativeObject.cpp
js/src/vm/Shape.cpp
js/src/vm/Shape.h
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -169,18 +169,20 @@ js::NativeObject::checkShapeConsistency(
 
     Shape *shape = lastProperty();
     Shape *prev = nullptr;
 
     if (inDictionaryMode()) {
         MOZ_ASSERT(shape->hasTable());
 
         ShapeTable &table = shape->table();
-        for (uint32_t fslot = table.freelist; fslot != SHAPE_INVALID_SLOT;
-             fslot = getSlot(fslot).toPrivateUint32()) {
+        for (uint32_t fslot = table.freeList();
+             fslot != SHAPE_INVALID_SLOT;
+             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);
@@ -948,28 +950,28 @@ NativeObject::allocSlot(ExclusiveContext
     MOZ_ASSERT(slot >= JSSLOT_FREE(obj->getClass()));
 
     /*
      * If this object is in dictionary mode, try to pull a free slot from the
      * shape table's slot-number freelist.
      */
     if (obj->inDictionaryMode()) {
         ShapeTable &table = obj->lastProperty()->table();
-        uint32_t last = table.freelist;
+        uint32_t last = table.freeList();
         if (last != SHAPE_INVALID_SLOT) {
 #ifdef DEBUG
             MOZ_ASSERT(last < slot);
             uint32_t next = obj->getSlot(last).toPrivateUint32();
             MOZ_ASSERT_IF(next != SHAPE_INVALID_SLOT, next < slot);
 #endif
 
             *slotp = last;
 
             const Value &vref = obj->getSlot(last);
-            table.freelist = vref.toPrivateUint32();
+            table.setFreeList(vref.toPrivateUint32());
             obj->setSlot(last, UndefinedValue());
             return true;
         }
     }
 
     if (slot >= SHAPE_MAXIMUM_SLOT) {
         js_ReportOutOfMemory(cx);
         return false;
@@ -984,29 +986,30 @@ NativeObject::allocSlot(ExclusiveContext
 }
 
 void
 NativeObject::freeSlot(uint32_t slot)
 {
     MOZ_ASSERT(slot < slotSpan());
 
     if (inDictionaryMode()) {
-        uint32_t &last = lastProperty()->table().freelist;
+        ShapeTable &table = lastProperty()->table();
+        uint32_t last = table.freeList();
 
         /* Can't afford to check the whole freelist, but let's check the head. */
         MOZ_ASSERT_IF(last != SHAPE_INVALID_SLOT, last < slotSpan() && last != slot);
 
         /*
          * Place all freed slots other than reserved slots (bug 595230) on the
          * dictionary's free list.
          */
         if (JSSLOT_FREE(getClass()) <= slot) {
             MOZ_ASSERT_IF(last != SHAPE_INVALID_SLOT, last < slotSpan());
             setSlot(slot, PrivateUint32Value(last));
-            last = slot;
+            table.setFreeList(slot);
             return;
         }
     }
     setSlot(slot, UndefinedValue());
 }
 
 Shape *
 NativeObject::addDataProperty(ExclusiveContext *cx, jsid idArg, uint32_t slot, unsigned attrs)
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -31,32 +31,32 @@ using namespace js::gc;
 using mozilla::CeilingLog2Size;
 using mozilla::DebugOnly;
 using mozilla::PodZero;
 using mozilla::RotateLeft;
 
 bool
 ShapeTable::init(ExclusiveContext *cx, Shape *lastProp)
 {
-    uint32_t sizeLog2 = CeilingLog2Size(entryCount);
+    uint32_t sizeLog2 = CeilingLog2Size(entryCount_);
     uint32_t size = JS_BIT(sizeLog2);
-    if (entryCount >= size - (size >> 2))
+    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));
-    if (!entries)
+    entries_ = cx->pod_calloc<Shape *>(JS_BIT(sizeLog2));
+    if (!entries_)
         return false;
 
-    hashShift = HASH_BITS - sizeLog2;
+    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);
 
         /*
          * Beware duplicate args and arg vs. var conflicts: the youngest shape
          * (nearest to lastProp) must win. See bug 600067.
@@ -176,37 +176,37 @@ static HashNumber
 Hash2(HashNumber hash0, int log2, int shift)
 {
     return ((hash0 << log2) >> shift) | 1;
 }
 
 Shape **
 ShapeTable::search(jsid id, bool adding)
 {
-    MOZ_ASSERT(entries);
+    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;
+    HashNumber hash1 = Hash1(hash0, hashShift_);
+    Shape **spp = entries_ + hash1;
 
     /* Miss: return space for a new entry. */
     Shape *stored = *spp;
     if (SHAPE_IS_FREE(stored))
         return spp;
 
     /* Hit: return entry. */
     Shape *shape = SHAPE_CLEAR_COLLISION(stored);
     if (shape && shape->propidRaw() == id)
         return spp;
 
     /* Collision: double hash. */
-    int sizeLog2 = HASH_BITS - hashShift;
-    HashNumber hash2 = Hash2(hash0, sizeLog2, hashShift);
+    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;
 #endif
 
     /* Save the first removed entry pointer so we can recycle it if adding. */
     Shape **firstRemoved;
@@ -219,17 +219,17 @@ ShapeTable::search(jsid id, bool adding)
 #ifdef DEBUG
         collision_flag &= uintptr_t(*spp) & SHAPE_COLLISION;
 #endif
     }
 
     for (;;) {
         hash1 -= hash2;
         hash1 &= sizeMask;
-        spp = entries + hash1;
+        spp = entries_ + hash1;
 
         stored = *spp;
         if (SHAPE_IS_FREE(stored))
             return (adding && firstRemoved) ? firstRemoved : spp;
 
         shape = SHAPE_CLEAR_COLLISION(stored);
         if (shape && shape->propidRaw() == id) {
             MOZ_ASSERT(collision_flag);
@@ -250,47 +250,47 @@ ShapeTable::search(jsid id, bool adding)
 
     MOZ_CRASH("Shape::search failed to find an expected entry.");
 }
 
 #ifdef JSGC_COMPACTING
 void
 ShapeTable::fixupAfterMovingGC()
 {
-    int log2 = HASH_BITS - hashShift;
+    int log2 = HASH_BITS - hashShift_;
     uint32_t size = JS_BIT(log2);
     for (HashNumber i = 0; i < size; i++) {
-        Shape *shape = SHAPE_FETCH(&entries[i]);
+        Shape *shape = SHAPE_FETCH(&entries_[i]);
         if (shape && IsForwarded(shape))
-            SHAPE_STORE_PRESERVING_COLLISION(&entries[i], Forwarded(shape));
+            SHAPE_STORE_PRESERVING_COLLISION(&entries_[i], Forwarded(shape));
     }
 }
 #endif
 
 bool
 ShapeTable::change(int log2Delta, ExclusiveContext *cx)
 {
-    MOZ_ASSERT(entries);
+    MOZ_ASSERT(entries_);
 
     /*
-     * Grow, shrink, or compress by changing this->entries.
+     * Grow, shrink, or compress by changing this->entries_.
      */
-    int oldlog2 = HASH_BITS - hashShift;
+    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);
     if (!newTable)
         return false;
 
     /* Now that we have newTable allocated, update members. */
-    hashShift = HASH_BITS - newlog2;
-    removedCount = 0;
-    Shape **oldTable = entries;
-    entries = newTable;
+    hashShift_ = HASH_BITS - newlog2;
+    removedCount_ = 0;
+    Shape **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);
         MOZ_ASSERT(cx->isThreadLocal(shape));
         if (shape) {
             Shape **spp = search(shape->propid(), true);
             MOZ_ASSERT(SHAPE_IS_FREE(*spp));
@@ -305,19 +305,19 @@ ShapeTable::change(int log2Delta, Exclus
 }
 
 bool
 ShapeTable::grow(ExclusiveContext *cx)
 {
     MOZ_ASSERT(needsToGrow());
 
     uint32_t size = capacity();
-    int delta = removedCount < size >> 2;
+    int delta = removedCount_ < size >> 2;
 
-    if (!change(delta, cx) && entryCount + removedCount == size - 1) {
+    if (!change(delta, cx) && entryCount_ + removedCount_ == size - 1) {
         js_ReportOutOfMemory(cx);
         return false;
     }
     return true;
 }
 
 /* static */ Shape *
 Shape::replaceLastProperty(ExclusiveContext *cx, StackBaseShape &base,
@@ -594,17 +594,17 @@ NativeObject::addPropertyInternal(Exclus
     }
 
     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));
-            ++table->entryCount;
+            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();
         return shape;
@@ -989,21 +989,21 @@ NativeObject::removeProperty(ExclusiveCo
      * 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;
-            ++table.removedCount;
-            --table.entryCount;
+            table.incRemovedCount();
+            table.decEntryCount();
         } else {
             *spp = nullptr;
-            --table.entryCount;
+            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.
              */
             Shape *aprop = self->lastProperty();
@@ -1021,17 +1021,17 @@ NativeObject::removeProperty(ExclusiveCo
             oldLastProp->handoffTableTo(self->lastProperty());
         }
 
         /* Generate a new shape for the object, infallibly. */
         JS_ALWAYS_TRUE(self->generateOwnShape(cx, spare));
 
         /* Consider shrinking table if its load factor is <= .25. */
         uint32_t size = table.capacity();
-        if (size > ShapeTable::MIN_SIZE && table.entryCount <= size >> 2)
+        if (size > ShapeTable::MIN_SIZE && table.entryCount() <= size >> 2)
             (void) table.change(-1, cx);
     } else {
         /*
          * Non-dictionary-mode shape tables are shared immutables, so all we
          * need do is retract the last property and we'll either get or else
          * lazily make via a later hashify the exact table for the new property
          * lineage.
          */
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -122,84 +122,109 @@ typedef JSPropertyDescriptor PropertyDes
 /* Limit on the number of slotful properties in an object. */
 static const uint32_t SHAPE_INVALID_SLOT = JS_BIT(24) - 1;
 static const uint32_t SHAPE_MAXIMUM_SLOT = JS_BIT(24) - 2;
 
 /*
  * Shapes use multiplicative hashing, but specialized to
  * minimize footprint.
  */
-struct ShapeTable {
+class ShapeTable {
+  public:
+    friend class NativeObject;
+    static const uint32_t MIN_ENTRIES   = 11;
+
+  private:
     static const uint32_t HASH_BITS     = mozilla::tl::BitSize<HashNumber>::value;
-    static const uint32_t MIN_ENTRIES   = 11;
 
     // 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);
 
-    int             hashShift;          /* multiplicative hash shift */
+    int             hashShift_;         /* multiplicative hash shift */
 
-    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
+    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 */
 
+    js::Shape       **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)
+      : hashShift_(HASH_BITS - MIN_SIZE_LOG2),
+        entryCount_(nentries),
+        removedCount_(0),
+        freeList_(SHAPE_INVALID_SLOT),
+        entries_(nullptr)
     {
         /* NB: entries is set by init, which must be called. */
     }
 
     ~ShapeTable() {
-        js_free(entries);
+        js_free(entries_);
     }
 
-    /* By definition, hashShift = HASH_BITS - log2(capacity). */
-    uint32_t capacity() const { return JS_BIT(HASH_BITS - hashShift); }
+    uint32_t entryCount() const { return entryCount_; }
+
+    uint32_t freeList() const { return freeList_; }
+    void setFreeList(uint32_t slot) { freeList_ = slot; }
 
     /*
      * This counts the ShapeTable object itself (which must be
      * heap-allocated) and its |entries| array.
      */
     size_t sizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const {
-        return mallocSizeOf(this) + mallocSizeOf(entries);
+        return mallocSizeOf(this) + mallocSizeOf(entries_);
     }
 
+    /*
+     * 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);
+
+#ifdef JSGC_COMPACTING
+    /* Update entries whose shapes have been moved */
+    void fixupAfterMovingGC();
+#endif
+
+  private:
+    void decEntryCount() {
+        MOZ_ASSERT(entryCount_ > 0);
+        entryCount_--;
+    }
+    void incEntryCount() {
+        entryCount_++;
+    }
+    void incRemovedCount() {
+        removedCount_++;
+    }
+
+    /* By definition, hashShift = HASH_BITS - log2(capacity). */
+    uint32_t capacity() const { return JS_BIT(HASH_BITS - hashShift_); }
+
     /* Whether we need to grow.  We want to do this if the load factor is >= 0.75 */
     bool needsToGrow() const {
         uint32_t size = capacity();
-        return entryCount + removedCount >= size - (size >> 2);
+        return entryCount_ + removedCount_ >= size - (size >> 2);
     }
 
     /*
      * Try to grow the table.  On failure, reports out of memory on cx
      * and returns false.  This will make any extant pointers into the
      * table invalid.  Don't call this unless needsToGrow() is true.
      */
     bool grow(ExclusiveContext *cx);
-
-    /*
-     * 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);
-
-#ifdef JSGC_COMPACTING
-    /* Update entries whose shapes have been moved */
-    void            fixupAfterMovingGC();
-#endif
 };
 
 /*
  * Reuse the API-only JSPROP_INDEX attribute to mean shadowability.
  */
 #define JSPROP_SHADOWABLE       JSPROP_INDEX
 
 /*
@@ -951,17 +976,17 @@ class Shape : public gc::TenuredCell
      */
     bool shadowable() const {
         MOZ_ASSERT_IF(isDataDescriptor(), writable());
         return hasSlot() || (attrs & JSPROP_SHADOWABLE);
     }
 
     uint32_t entryCount() {
         if (hasTable())
-            return table().entryCount;
+            return table().entryCount();
         uint32_t count = 0;
         for (Shape::Range<NoGC> r(this); !r.empty(); r.popFront())
             ++count;
         return count;
     }
 
     bool isBigEnoughForAShapeTable() {
         MOZ_ASSERT(!hasTable());