Bug 1119288 part 4 - More ShapeTable cleanup. r=njn
authorJan de Mooij <jdemooij@mozilla.com>
Sat, 10 Jan 2015 14:51:03 +0100
changeset 239864 ab841f36d62f699d04853b7cc613bc27845a4760
parent 239863 2ea97247b91ac2930e23e1d55b864c54950d288f
child 239865 6cbf0b1ec001b3a26f1c83347b70cef858311adf
push id7472
push userraliiev@mozilla.com
push dateMon, 12 Jan 2015 20:36:27 +0000
treeherdermozilla-aurora@300ca104f8fb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnjn
bugs1119288
milestone37.0a1
Bug 1119288 part 4 - More ShapeTable cleanup. r=njn
js/src/vm/Shape.cpp
js/src/vm/Shape.h
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -44,33 +44,40 @@ ShapeTable::init(ExclusiveContext *cx, S
         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<Entry>(JS_BIT(sizeLog2));
+    size = JS_BIT(sizeLog2);
+    entries_ = cx->pod_calloc<Entry>(size);
     if (!entries_)
         return false;
 
+    MOZ_ASSERT(sizeLog2 <= HASH_BITS);
     hashShift_ = HASH_BITS - sizeLog2;
+
     for (Shape::Range<NoGC> r(lastProp); !r.empty(); r.popFront()) {
         Shape &shape = r.front();
         MOZ_ASSERT(cx->isThreadLocal(&shape));
         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 (!entry.shape())
             entry.setPreservingCollision(&shape);
     }
+
+    MOZ_ASSERT(capacity() == size);
+    MOZ_ASSERT(size >= MIN_SIZE);
+    MOZ_ASSERT(!needsToGrow());
     return true;
 }
 
 void
 Shape::removeFromDictionary(NativeObject *obj)
 {
     MOZ_ASSERT(inDictionary());
     MOZ_ASSERT(obj->inDictionaryMode());
@@ -164,49 +171,49 @@ Shape::hashify(ExclusiveContext *cx, Sha
     return true;
 }
 
 /*
  * Double hashing needs the second hash code to be relatively prime to table
  * size, so we simply make hash2 odd.
  */
 static HashNumber
-Hash1(HashNumber hash0, int shift)
+Hash1(HashNumber hash0, uint32_t shift)
 {
     return hash0 >> shift;
 }
 
 static HashNumber
-Hash2(HashNumber hash0, int log2, int shift)
+Hash2(HashNumber hash0, uint32_t log2, uint32_t shift)
 {
     return ((hash0 << log2) >> shift) | 1;
 }
 
 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_);
-    Entry *entry = entries_ + hash1;
+    Entry *entry = &getEntry(hash1);
 
     /* Miss: return space for a new entry. */
     if (entry->isFree())
         return *entry;
 
     /* Hit: return entry. */
     Shape *shape = entry->shape();
     if (shape && shape->propidRaw() == id)
         return *entry;
 
     /* Collision: double hash. */
-    int sizeLog2 = HASH_BITS - hashShift_;
+    uint32_t sizeLog2 = HASH_BITS - hashShift_;
     HashNumber hash2 = Hash2(hash0, sizeLog2, hashShift_);
     uint32_t sizeMask = JS_BITMASK(sizeLog2);
 
 #ifdef DEBUG
     bool collisionFlag = true;
 #endif
 
     /* Save the first removed entry pointer so we can recycle it if adding. */
@@ -217,20 +224,20 @@ ShapeTable::search(jsid id, bool adding)
         firstRemoved = nullptr;
         if (adding && !entry->hadCollision())
             entry->flagCollision();
 #ifdef DEBUG
         collisionFlag &= entry->hadCollision();
 #endif
     }
 
-    for (;;) {
+    while (true) {
         hash1 -= hash2;
         hash1 &= sizeMask;
-        entry = entries_ + hash1;
+        entry = &getEntry(hash1);
 
         if (entry->isFree())
             return (adding && firstRemoved) ? *firstRemoved : *entry;
 
         shape = entry->shape();
         if (shape && shape->propidRaw() == id) {
             MOZ_ASSERT(collisionFlag);
             return *entry;
@@ -250,73 +257,77 @@ 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_;
-    uint32_t size = JS_BIT(log2);
+    uint32_t size = capacity();
     for (HashNumber i = 0; i < size; i++) {
-        Entry &entry = entries_[i];
+        Entry &entry = getEntry(i);
         Shape *shape = entry.shape();
         if (shape && IsForwarded(shape))
             entry.setPreservingCollision(Forwarded(shape));
     }
 }
 #endif
 
 bool
 ShapeTable::change(int log2Delta, ExclusiveContext *cx)
 {
     MOZ_ASSERT(entries_);
+    MOZ_ASSERT(-1 <= log2Delta && log2Delta <= 1);
 
     /*
      * 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);
-    Entry *newTable = cx->pod_calloc<Entry>(newsize);
+    uint32_t oldLog2 = HASH_BITS - hashShift_;
+    uint32_t newLog2 = oldLog2 + log2Delta;
+    uint32_t oldSize = JS_BIT(oldLog2);
+    uint32_t newSize = JS_BIT(newLog2);
+    Entry *newTable = cx->pod_calloc<Entry>(newSize);
     if (!newTable)
         return false;
 
     /* Now that we have newTable allocated, update members. */
-    hashShift_ = HASH_BITS - newlog2;
+    MOZ_ASSERT(newLog2 <= HASH_BITS);
+    hashShift_ = HASH_BITS - newLog2;
     removedCount_ = 0;
     Entry *oldTable = entries_;
     entries_ = newTable;
 
     /* Copy only live entries, leaving removed and free ones behind. */
-    for (Entry *oldEntry = oldTable; oldsize != 0; oldEntry++) {
-        Shape *shape = oldEntry->shape();
-        MOZ_ASSERT(cx->isThreadLocal(shape));
-        if (shape) {
+    for (Entry *oldEntry = oldTable; oldSize != 0; oldEntry++) {
+        if (Shape *shape = oldEntry->shape()) {
+            MOZ_ASSERT(cx->isThreadLocal(shape));
             Entry &entry = search(shape->propid(), true);
             MOZ_ASSERT(entry.isFree());
             entry.setShape(shape);
         }
-        oldsize--;
+        oldSize--;
     }
 
+    MOZ_ASSERT(capacity() == newSize);
+
     /* Finally, free the old entries storage. */
     js_free(oldTable);
     return true;
 }
 
 bool
 ShapeTable::grow(ExclusiveContext *cx)
 {
     MOZ_ASSERT(needsToGrow());
 
     uint32_t size = capacity();
-    int delta = removedCount_ < size >> 2;
+    int delta = removedCount_ < (size >> 2);
+
+    MOZ_ASSERT(entryCount_ + removedCount_ <= size - 1);
 
     if (!change(delta, cx) && entryCount_ + removedCount_ == size - 1) {
         js_ReportOutOfMemory(cx);
         return false;
     }
     return true;
 }
 
@@ -990,18 +1001,18 @@ 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 (entry->hadCollision()) {
             entry->setRemoved();
+            table.decEntryCount();
             table.incRemovedCount();
-            table.decEntryCount();
         } else {
             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
--- a/js/src/vm/Shape.h
+++ b/js/src/vm/Shape.h
@@ -174,17 +174,17 @@ class ShapeTable {
   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);
 
-    int             hashShift_;         /* multiplicative hash shift */
+    uint32_t        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
                                            freelist in owning dictionary-mode
                                            object */
 
@@ -228,25 +228,31 @@ class ShapeTable {
     Entry &search(jsid id, bool adding);
 
 #ifdef JSGC_COMPACTING
     /* Update entries whose shapes have been moved */
     void fixupAfterMovingGC();
 #endif
 
   private:
+    Entry &getEntry(uint32_t i) const {
+        MOZ_ASSERT(i < capacity());
+        return entries_[i];
+    }
     void decEntryCount() {
         MOZ_ASSERT(entryCount_ > 0);
         entryCount_--;
     }
     void incEntryCount() {
         entryCount_++;
+        MOZ_ASSERT(entryCount_ + removedCount_ <= capacity());
     }
     void incRemovedCount() {
         removedCount_++;
+        MOZ_ASSERT(entryCount_ + removedCount_ <= capacity());
     }
 
     /* 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();