Bug 1481138 - Remove HashMap::lookupWithDefault(). r=luke draft
authorNicholas Nethercote <nnethercote@mozilla.com>
Mon, 06 Aug 2018 09:45:38 +1000
changeset 826905 9d96ceec60c7d9a25a3ca643fd52adbcff26cae8
parent 826904 cb701ed3ea0bfbd0c75283a80a7cd79d7352d5a9
child 826906 d09008e7d9d525043cd5399374f942fbabd5e2c7
push id118403
push usernnethercote@mozilla.com
push dateMon, 06 Aug 2018 07:02:00 +0000
reviewersluke
bugs1481138
milestone63.0a1
Bug 1481138 - Remove HashMap::lookupWithDefault(). r=luke Because it's quite strange, badly named, not that useful, and barely used. Also remove WeakMap::lookupWithDefault(), which is similar, but not used at all. MozReview-Commit-ID: IhIl4hQ73U1
js/src/gc/WeakMap.h
js/src/jsapi-tests/testHashTable.cpp
js/src/vm/Debugger.h
mfbt/HashTable.h
--- a/js/src/gc/WeakMap.h
+++ b/js/src/gc/WeakMap.h
@@ -141,23 +141,16 @@ class WeakMap : public HashMap<Key, Valu
 
     AddPtr lookupForAdd(const Lookup& l) const {
         AddPtr p = Base::lookupForAdd(l);
         if (p)
             exposeGCThingToActiveJS(p->value());
         return p;
     }
 
-    Ptr lookupWithDefault(const Key& k, const Value& defaultValue) {
-        Ptr p = Base::lookupWithDefault(k, defaultValue);
-        if (p)
-            exposeGCThingToActiveJS(p->value());
-        return p;
-    }
-
     // Resolve ambiguity with LinkedListElement<>::remove.
     using Base::remove;
 
     void markEntry(GCMarker* marker, gc::Cell* markedCell, JS::GCCellPtr origKey) override;
 
     void trace(JSTracer* trc) override;
 
   protected:
--- a/js/src/jsapi-tests/testHashTable.cpp
+++ b/js/src/jsapi-tests/testHashTable.cpp
@@ -345,56 +345,59 @@ BEGIN_TEST(testHashSetOfMoveOnlyType)
     CHECK(set.put(std::move(a))); // This shouldn't generate a compiler error.
 
     return true;
 }
 END_TEST(testHashSetOfMoveOnlyType)
 
 #if defined(DEBUG)
 
-// Add entries to a HashMap using lookupWithDefault until either we get an OOM,
-// or the table has been resized a few times.
+// Add entries to a HashMap until either we get an OOM, or the table has been
+// resized a few times.
 static bool
-LookupWithDefaultUntilResize() {
+GrowUntilResize()
+{
     IntMap m;
 
     if (!m.init())
         return false;
 
     // Add entries until we've resized the table four times.
     size_t lastCapacity = m.capacity();
     size_t resizes = 0;
     uint32_t key = 0;
     while (resizes < 4) {
-        if (!m.lookupWithDefault(key++, 0))
-            return false;
+        auto p = m.lookupForAdd(key);
+        if (!p && !m.add(p, key, 0))
+            return false;   // OOM'd while adding
 
         size_t capacity = m.capacity();
         if (capacity != lastCapacity) {
             resizes++;
             lastCapacity = capacity;
         }
+        key++;
     }
 
     return true;
 }
 
-BEGIN_TEST(testHashMapLookupWithDefaultOOM)
+BEGIN_TEST(testHashMapGrowOOM)
 {
     uint32_t timeToFail;
     for (timeToFail = 1; timeToFail < 1000; timeToFail++) {
         js::oom::SimulateOOMAfter(timeToFail, js::THREAD_TYPE_MAIN, false);
-        LookupWithDefaultUntilResize();
+        GrowUntilResize();
     }
 
     js::oom::ResetSimulatedOOM();
     return true;
 }
 
-END_TEST(testHashMapLookupWithDefaultOOM)
+END_TEST(testHashMapGrowOOM)
 #endif // defined(DEBUG)
 
 BEGIN_TEST(testHashTableMovableModIterator)
 {
     IntSet set;
     CHECK(set.init());
 
     // Exercise returning a hash table ModIterator object from a function.
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -220,19 +220,19 @@ class DebuggerWeakMap : private WeakMap<
             }
         }
 #ifdef DEBUG
         Base::assertEntriesNotAboutToBeFinalized();
 #endif
     }
 
     MOZ_MUST_USE bool incZoneCount(JS::Zone* zone) {
-        CountMap::Ptr p = zoneCounts.lookupWithDefault(zone, 0);
-        if (!p)
-            return false;
+        CountMap::AddPtr p = zoneCounts.lookupForAdd(zone);
+        if (!p && !zoneCounts.add(p, zone, 0))
+            return false;   // OOM'd while adding
         ++p->value();
         return true;
     }
 
     void decZoneCount(JS::Zone* zone) {
         CountMap::Ptr p = zoneCounts.lookup(zone);
         MOZ_ASSERT(p);
         MOZ_ASSERT(p->value() > 0);
--- a/mfbt/HashTable.h
+++ b/mfbt/HashTable.h
@@ -284,30 +284,16 @@ public:
   // Only call this to populate an empty map after reserving space with init().
   template<typename KeyInput, typename ValueInput>
   void putNewInfallible(KeyInput&& aKey, ValueInput&& aValue)
   {
     mImpl.putNewInfallible(
       aKey, std::forward<KeyInput>(aKey), std::forward<ValueInput>(aValue));
   }
 
-  // Add (aKey,aDefaultValue) if |aKey| is not found. Return a false-y Ptr on
-  // OOM.
-  Ptr lookupWithDefault(const Key& aKey, const Value& aDefaultValue)
-  {
-    AddPtr p = lookupForAdd(aKey);
-    if (p) {
-      return p;
-    }
-    bool ok = add(p, aKey, aDefaultValue);
-    MOZ_ASSERT_IF(!ok, !p); // p is left false-y on OOM.
-    (void)ok;
-    return p;
-  }
-
   // Like |lookup(l)|, but on miss, |p = lookupForAdd(l)| allows efficient
   // insertion of Key |k| (where |HashPolicy::match(k,l) == true|) using
   // |add(p,k,v)|. After |add(p,k,v)|, |p| points to the new key/value. E.g.:
   //
   //   using HM = HashMap<int,char>;
   //   HM h;
   //   HM::AddPtr p = h.lookupForAdd(3);
   //   if (!p) {