Bug 697646 - Don't create tiny property tables. r=bhackett.

js/src/jsscope.h

@@ -207,6 +207,8 @@
  * were non-null and minimal-length.  Until a scope is searched
  * MAX_LINEAR_SEARCHES times, we use linear search from obj->lastProp to find a
  * given id, and save on the time and space overhead of creating a hash table.
+ * Also, we don't create tables for property tree Shapes that have shape
+ * lineages smaller than MIN_ENTRIES.
  */
 
 #define SHAPE_INVALID_SLOT              0xffffffff
@@ -215,12 +217,11 @@ namespace js {
 
 /*
  * Shapes use multiplicative hashing, _a la_ jsdhash.[ch], but specialized to
- * minimize footprint.  But if a Shape lineage has been searched fewer than
- * MAX_LINEAR_SEARCHES times, we use linear search and avoid allocating
- * scope->table.
+ * minimize footprint.
  */
 struct PropertyTable {
     static const uint32 MAX_LINEAR_SEARCHES = 7;
+    static const uint32 MIN_ENTRIES         = 7;
     static const uint32 MIN_SIZE_LOG2       = 4;
     static const uint32 MIN_SIZE            = JS_BIT(MIN_SIZE_LOG2);
 
@@ -625,6 +626,18 @@ struct Shape : public js::gc::Cell
         return count;
     }
 
+    bool isBigEnoughForAPropertyTable() const {
+        JS_ASSERT(!hasTable());
+        const js::Shape *shape = this;
+        uint32 count = 0;
+        for (js::Shape::Range r = shape->all(); !r.empty(); r.popFront()) {
+            ++count;
+            if (count >= PropertyTable::MIN_ENTRIES)
+                return true;
+        }
+        return false;
+    }
+
 #ifdef DEBUG
     void dump(JSContext *cx, FILE *fp) const;
     void dumpSubtree(JSContext *cx, int level, FILE *fp) const;
@@ -704,6 +717,17 @@ js_GenerateShape(JSContext *cx);
 
 namespace js {
 
+/*
+ * The search succeeds if it finds a Shape with the given id.  There are two
+ * success cases:
+ * - If the Shape is the last in its shape lineage, we return |startp|, which
+ *   is &obj->lastProp or something similar.
+ * - Otherwise, we return &shape->parent, where |shape| is the successor to the
+ *   found Shape.
+ *
+ * There is one failure case:  we return &emptyShape->parent, where
+ * |emptyShape| is the EmptyShape at the start of the shape lineage.
+ */
 JS_ALWAYS_INLINE js::Shape **
 Shape::search(JSContext *cx, js::Shape **startp, jsid id, bool adding)
 {
@@ -712,9 +736,12 @@ Shape::search(JSContext *cx, js::Shape **startp, jsid id, bool adding)
         return start->getTable()->search(id, adding);
 
     if (start->numLinearSearches == PropertyTable::MAX_LINEAR_SEARCHES) {
-        if (start->hashify(cx))
+        if (start->isBigEnoughForAPropertyTable() && start->hashify(cx))
             return start->getTable()->search(id, adding);
-        /* OOM!  Don't increment numLinearSearches, to keep hasTable() false. */
+        /* 
+         * No table built -- there weren't enough entries, or OOM occurred.
+         * Don't increment numLinearSearches, to keep hasTable() false.
+         */
         JS_ASSERT(!start->hasTable());
     } else {
         JS_ASSERT(start->numLinearSearches < PropertyTable::MAX_LINEAR_SEARCHES);
@@ -725,9 +752,9 @@ Shape::search(JSContext *cx, js::Shape **startp, jsid id, bool adding)
      * Not enough searches done so far to justify hashing: search linearly
      * from *startp.
      *
-     * We don't use a Range here, or stop at null parent (the empty shape
-     * at the end), to avoid an extra load per iteration just to save a
-     * load and id test at the end (when missing).
+     * We don't use a Range here, or stop at null parent (the empty shape at
+     * the end).  This avoids an extra load per iteration at the cost (if the
+     * search fails) of an extra load and id test at the end.
      */
     js::Shape **spp;
     for (spp = startp; js::Shape *shape = *spp; spp = &shape->parent) {