Bug 580633 - Do less copying when adding elements to HashMap (r=bz)
authorLuke Wagner <lw@mozilla.com>
Wed, 21 Jul 2010 19:01:33 -0700
changeset 48524 116265442418c6d7fa7d4bea9feec4379fdf3ca3
parent 48523 471b8cd1dae6daf5defac0796b33b885c0682f35
child 48525 0b8353af430044f79def54a1e6e4d50e3fe50217
push id14748
push userrsayre@mozilla.com
push dateSun, 01 Aug 2010 00:33:23 +0000
treeherdermozilla-central@f0df797bb2a9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs580633
milestone2.0b2pre
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 580633 - Do less copying when adding elements to HashMap (r=bz)
js/src/jshashtable.h
--- a/js/src/jshashtable.h
+++ b/js/src/jshashtable.h
@@ -588,17 +588,22 @@ class HashTable : AllocPolicy
         Entry &entry = lookup(l, keyHash, sCollisionBit);
 #ifdef DEBUG
         return AddPtr(entry, keyHash, mutationCount);
 #else
         return AddPtr(entry, keyHash);
 #endif
     }
 
-    bool add(AddPtr &p, const T &t)
+    /*
+     * There is an important contract between the caller and callee for this
+     * function: if add() returns true, the caller must assign the T value
+     * which produced p before using the hashtable again.
+     */
+    bool add(AddPtr &p, T **pentry)
     {
         ReentrancyGuard g(*this);
         JS_ASSERT(mutationCount == p.mutationCount);
         JS_ASSERT(table);
         JS_ASSERT(!p.found());
         JS_ASSERT(!(p.keyHash & sCollisionBit));
 
         /*
@@ -625,17 +630,17 @@ class HashTable : AllocPolicy
                 if (!changeTableSize(deltaLog2))
                     return false;
 
                 /* Preserve the validity of |p.entry|. */
                 p.entry = &findFreeEntry(p.keyHash);
             }
         }
 
-        p.entry->t = t;
+        *pentry = &p.entry->t;
         p.entry->setLive(p.keyHash);
         entryCount++;
 #ifdef DEBUG
         mutationCount++;
 #endif
         return true;
     }
 
@@ -837,18 +842,37 @@ class HashMap
      *      if (!h.relookupOrAdd(p, 3, 'a'))
      *        return false;
      *    }
      *    const HM::Entry &e = *p;
      *    assert(p->key == 3);
      *    char val = p->value;
      */
     typedef typename Impl::AddPtr AddPtr;
-    AddPtr lookupForAdd(const Lookup &l) const        { return impl.lookupForAdd(l); }
-    bool add(AddPtr &p, const Key &k, const Value &v) { return impl.add(p,Entry(k,v)); }
+    AddPtr lookupForAdd(const Lookup &l) const {
+        return impl.lookupForAdd(l);
+    }
+
+    bool add(AddPtr &p, const Key &k, const Value &v) {
+        Entry *pentry;
+        if (!impl.add(p, &pentry))
+            return false;
+        const_cast<Key &>(pentry->key) = k;
+        pentry->value = v;
+        return true;
+    }
+
+    bool add(AddPtr &p, const Key &k) {
+        Entry *pentry;
+        if (!impl.add(p, &pentry))
+            return false;
+        const_cast<Key &>(pentry->key) = k;
+        return true;
+    }
+
     bool relookupOrAdd(AddPtr &p, const Key &k, const Value &v) {
         return impl.relookupOrAdd(p, k, Entry(k, v));
     }
 
     /*
      * |all()| returns a Range containing |count()| elements. E.g.:
      *
      *   typedef HashMap<int,char> HM;
@@ -984,18 +1008,27 @@ class HashSet
      *     if (!h.add(p, 3))
      *       return false;
      *   }
      *   assert(*p == 3);   // p acts like a pointer to int
      *
      * Also see the definition of AddPtr in HashTable above.
      */
     typedef typename Impl::AddPtr AddPtr;
-    AddPtr lookupForAdd(const Lookup &l) const        { return impl.lookupForAdd(l); }
-    bool add(AddPtr &p, const T &t)                   { return impl.add(p,t); }
+    AddPtr lookupForAdd(const Lookup &l) const {
+        return impl.lookupForAdd(l);
+    }
+
+    bool add(AddPtr &p, const T &t) {
+        const T *pentry;
+        if (!impl.add(p, &pentry))
+            return false;
+        const_cast<T &>(*pentry) = t;
+        return true;
+    }
 
     /*
      * |all()| returns a Range containing |count()| elements:
      *
      *   typedef HashSet<int> HS;
      *   HS h;
      *   for (HS::Range r = h.all(); !r.empty(); r.popFront())
      *     int i = r.front();