bz/brendan patch to hash function-local shapes in dictionary mode due to too many locals (610370, r=brendan/bz).

js/src/jsscope.cpp

@@ -441,6 +441,25 @@ PropertyTable::change(int log2Delta, JSContext *cx)
     return true;
 }
 
+bool
+PropertyTable::grow(JSContext *cx)
+{
+    JS_ASSERT(needsToGrow());
+
+    uint32 size = capacity();
+    int delta = removedCount < size >> 2;
+    if (!delta)
+        METER(compresses);
+    else
+        METER(grows);
+
+    if (!change(delta, cx) && entryCount + removedCount == size - 1) {
+        JS_ReportOutOfMemory(cx);
+        return false;
+    }
+    return true;
+}
+
 Shape *
 Shape::getChild(JSContext *cx, const js::Shape &child, Shape **listp)
 {
@@ -448,8 +467,47 @@ Shape::getChild(JSContext *cx, const js::Shape &child, Shape **listp)
     JS_ASSERT(!child.inDictionary());
 
     if (inDictionary()) {
-        if (newDictionaryShape(cx, child, listp))
-            return *listp;
+        Shape *oldShape = *listp;
+        PropertyTable *table = oldShape ? oldShape->table : NULL;
+
+        /*
+         * Attempt to grow table if needed before extending *listp, rather than
+         * risking OOM under table->grow after newDictionaryShape succeeds, and
+         * then have to fix up *listp.
+         */
+        if (table && table->needsToGrow() && !table->grow(cx))
+            return NULL;
+
+        if (newDictionaryShape(cx, child, listp)) {
+            Shape *newShape = *listp;
+
+            JS_ASSERT(oldShape == newShape->parent);
+            if (table) {
+                /* Add newShape to the property table. */
+                METER(searches);
+                Shape **spp = table->search(newShape->id, true);
+
+                /*
+                 * Beware duplicate formal parameters, allowed by ECMA-262 in
+                 * non-strict mode. Otherwise we know that JSFunction::addLocal
+                 * (our caller) won't pass an id already in the table to us. In
+                 * the case of duplicate formals, the last one wins, so while
+                 * we must not overcount entries, we must store newShape.
+                 */
+                if (!SHAPE_FETCH(spp))
+                    ++table->entryCount;
+                SHAPE_STORE_PRESERVING_COLLISION(spp, newShape);
+
+                /* Hand the table off from oldShape to newShape. */
+                oldShape->setTable(NULL);
+                newShape->setTable(table);
+            } else {
+                if (!newShape->table)
+                    newShape->maybeHash(cx);
+            }
+            return newShape;
+        }
+
         return NULL;
     }
 
@@ -751,18 +809,10 @@ JSObject::addPropertyInternal(JSContext *cx, jsid id,
             table = lastProp->table;
         }
     } else if ((table = lastProp->table) != NULL) {
-        /* Check whether we need to grow, if the load factor is >= .75. */
-        uint32 size = table->capacity();
-        if (table->entryCount + table->removedCount >= size - (size >> 2)) {
-            int delta = table->removedCount < size >> 2;
-            if (!delta)
-                METER(compresses);
-            else
-                METER(grows);
-            if (!table->change(delta, cx) && table->entryCount + table->removedCount == size - 1) {
-                JS_ReportOutOfMemory(cx);
+        if (table->needsToGrow()) {
+            if (!table->grow(cx))
                 return NULL;
-            }
+
             METER(searches);
             METER(changeSearches);
             spp = table->search(id, true);

js/src/jsscope.h

@@ -248,6 +248,19 @@ struct PropertyTable {
     /* By definition, hashShift = JS_DHASH_BITS - log2(capacity). */
     uint32 capacity() const { return JS_BIT(JS_DHASH_BITS - hashShift); }
 
+    /* Whether we need to grow.  We want to do this if the load factor is >= 0.75 */
+    bool needsToGrow() const {
+        uint32 size = capacity();
+        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(JSContext *cx);
+
     /*
      * NB: init and change are fallible but do not report OOM, so callers can
      * cope or ignore. They do however use JSRuntime's calloc method in order