Bug 1821659 - Part 1: Allow fallible hashing to return the hash code …

…with maybeGetHash r=sfink

This is used where the hash table operation can fail early if we know there's
never been a hash code created for a lookup. If there has been a hash code
created, it's more efficient to return it though as otherwise we will always
query it straight away.

Therefore this renames |hasHash| to |maybeGetHash| and adds an output
parameter.

Differential Revision: https://phabricator.services.mozilla.com/D173122

js/public/RootingAPI.h

@@ -851,7 +851,7 @@ struct JS_PUBLIC_API MovableCellHasher {
   using Key = T;
   using Lookup = T;
 
-  static bool hasHash(const Lookup& l);
+  static bool maybeGetHash(const Lookup& l, mozilla::HashNumber* hashOut);
   static bool ensureHash(const Lookup& l);
   static HashNumber hash(const Lookup& l);
   static bool match(const Key& k, const Lookup& l);
@@ -864,8 +864,8 @@ struct JS_PUBLIC_API MovableCellHasher<JS::Heap<T>> {
   using Key = JS::Heap<T>;
   using Lookup = T;
 
-  static bool hasHash(const Lookup& l) {
-    return MovableCellHasher<T>::hasHash(l);
+  static bool maybeGetHash(const Lookup& l, HashNumber* hashOut) {
+    return MovableCellHasher<T>::maybeGetHash(l, hashOut);
   }
   static bool ensureHash(const Lookup& l) {
     return MovableCellHasher<T>::ensureHash(l);
@@ -885,8 +885,9 @@ namespace mozilla {
 template <typename T>
 struct FallibleHashMethods<js::MovableCellHasher<T>> {
   template <typename Lookup>
-  static bool hasHash(Lookup&& l) {
-    return js::MovableCellHasher<T>::hasHash(std::forward<Lookup>(l));
+  static bool maybeGetHash(Lookup&& l, HashNumber* hashOut) {
+    return js::MovableCellHasher<T>::maybeGetHash(std::forward<Lookup>(l),
+                                                  hashOut);
   }
   template <typename Lookup>
   static bool ensureHash(Lookup&& l) {

js/src/gc/Barrier.cpp

@@ -68,12 +68,14 @@ bool CurrentThreadIsIonCompiling() {
 #endif  // DEBUG
 
 template <typename T>
-/* static */ bool MovableCellHasher<T>::hasHash(const Lookup& l) {
+/* static */ bool MovableCellHasher<T>::maybeGetHash(const Lookup& l,
+                                                     HashNumber* hashOut) {
   if (!l) {
+    *hashOut = 0;
     return true;
   }
 
-  return l->zoneFromAnyThread()->hasUniqueId(l);
+  return l->zoneFromAnyThread()->maybeGetHashCode(l, hashOut);
 }
 
 template <typename T>

js/src/gc/Barrier.h

@@ -1133,8 +1133,8 @@ struct MovableCellHasher<PreBarriered<T>> {
   using Key = PreBarriered<T>;
   using Lookup = T;
 
-  static bool hasHash(const Lookup& l) {
-    return MovableCellHasher<T>::hasHash(l);
+  static bool maybeGetHash(const Lookup& l, HashNumber* hashOut) {
+    return MovableCellHasher<T>::maybeGetHash(l, hashOut);
   }
   static bool ensureHash(const Lookup& l) {
     return MovableCellHasher<T>::ensureHash(l);
@@ -1152,8 +1152,8 @@ struct MovableCellHasher<HeapPtr<T>> {
   using Key = HeapPtr<T>;
   using Lookup = T;
 
-  static bool hasHash(const Lookup& l) {
-    return MovableCellHasher<T>::hasHash(l);
+  static bool maybeGetHash(const Lookup& l, HashNumber* hashOut) {
+    return MovableCellHasher<T>::maybeGetHash(l, hashOut);
   }
   static bool ensureHash(const Lookup& l) {
     return MovableCellHasher<T>::ensureHash(l);
@@ -1171,8 +1171,8 @@ struct MovableCellHasher<WeakHeapPtr<T>> {
   using Key = WeakHeapPtr<T>;
   using Lookup = T;
 
-  static bool hasHash(const Lookup& l) {
-    return MovableCellHasher<T>::hasHash(l);
+  static bool maybeGetHash(const Lookup& l, HashNumber* hashOut) {
+    return MovableCellHasher<T>::maybeGetHash(l, hashOut);
   }
   static bool ensureHash(const Lookup& l) {
     return MovableCellHasher<T>::ensureHash(l);

js/src/gc/Zone-inl.h

@@ -24,6 +24,17 @@ inline bool JS::Zone::getHashCode(js::gc::Cell* cell, js::HashNumber* hashp) {
   return true;
 }
 
+inline bool JS::Zone::maybeGetHashCode(js::gc::Cell* cell,
+                                       js::HashNumber* hashOut) {
+  uint64_t uid;
+  if (!maybeGetUniqueId(cell, &uid)) {
+    return false;
+  }
+
+  *hashOut = UniqueIdToHash(uid);
+  return true;
+}
+
 inline bool JS::Zone::maybeGetUniqueId(js::gc::Cell* cell, uint64_t* uidp) {
   MOZ_ASSERT(uidp);
   MOZ_ASSERT(js::CurrentThreadCanAccessZone(this) ||

js/src/gc/Zone.h

@@ -541,6 +541,9 @@ class Zone : public js::ZoneAllocator, public js::gc::GraphNodeBase<JS::Zone> {
   // Gets an existing UID in |uidp| if one exists.
   [[nodiscard]] bool maybeGetUniqueId(js::gc::Cell* cell, uint64_t* uidp);
 
+  [[nodiscard]] bool maybeGetHashCode(js::gc::Cell* cell,
+                                      js::HashNumber* hashOut);
+
   // Puts an existing UID in |uidp|, or creates a new UID for this Cell and
   // puts that into |uidp|. Returns false on OOM.
   [[nodiscard]] bool getOrCreateUniqueId(js::gc::Cell* cell, uint64_t* uidp);

js/src/jsapi-tests/testGCWeakCache.cpp

@@ -147,8 +147,13 @@ struct MovableCellHasher<ObjectEntry> {
   using Key = ObjectEntry;
   using Lookup = JSObject*;
 
-  static bool hasHash(const Lookup& l) {
-    return MovableCellHasher<JSObject*>::hasHash(l);
+  static bool maybeGetHash(const Lookup& l, HashNumber* hashOut) {
+    if (!MovableCellHasher<JSObject*>::maybeGetHash(l, hashOut)) {
+      return false;
+    }
+    // Reduce hash code to single bit to generate hash collisions.
+    *hashOut &= 0x1;
+    return true;
   }
   static bool ensureHash(const Lookup& l) {
     return MovableCellHasher<JSObject*>::ensureHash(l);
@@ -193,14 +198,17 @@ struct MovableCellHasher<NumberAndObjectEntry> {
   using Key = NumberAndObjectEntry;
   using Lookup = NumberAndObjectLookup;
 
-  static bool hasHash(const Lookup& l) {
-    return MovableCellHasher<JSObject*>::hasHash(l.obj);
+  static bool maybeGetHash(const Lookup& l, HashNumber* hashOut) {
+    if (!MovableCellHasher<JSObject*>::maybeGetHash(l.obj, hashOut)) {
+      return false;
+    }
+    *hashOut ^= l.number;
+    return true;
   }
   static bool ensureHash(const Lookup& l) {
     return MovableCellHasher<JSObject*>::ensureHash(l.obj);
   }
   static HashNumber hash(const Lookup& l) {
-    // Reduce hash code to single bit to generate hash collisions.
     return MovableCellHasher<HeapPtr<JSObject*>>::hash(l.obj) ^ l.number;
   }
   static bool match(const Key& k, const Lookup& l) {

js/src/vm/SavedFrame.h

@@ -150,13 +150,16 @@ struct SavedFrame::HashPolicy {
   using SavedFramePtrHasher = MovableCellHasher<SavedFrame*>;
   using JSPrincipalsPtrHasher = PointerHasher<JSPrincipals*>;
 
-  static bool hasHash(const Lookup& l);
+  static bool maybeGetHash(const Lookup& l, HashNumber* hashOut);
   static bool ensureHash(const Lookup& l);
   static HashNumber hash(const Lookup& lookup);
   static bool match(SavedFrame* existing, const Lookup& lookup);
 
   using Key = WeakHeapPtr<SavedFrame*>;
   static void rekey(Key& key, const Key& newKey);
+
+ private:
+  static HashNumber calculateHash(const Lookup& lookup, HashNumber parentHash);
 };
 
 }  // namespace js
@@ -166,8 +169,9 @@ namespace mozilla {
 template <>
 struct FallibleHashMethods<js::SavedFrame::HashPolicy> {
   template <typename Lookup>
-  static bool hasHash(Lookup&& l) {
-    return js::SavedFrame::HashPolicy::hasHash(std::forward<Lookup>(l));
+  static bool maybeGetHash(Lookup&& l, HashNumber* hashOut) {
+    return js::SavedFrame::HashPolicy::maybeGetHash(std::forward<Lookup>(l),
+                                                    hashOut);
   }
   template <typename Lookup>
   static bool ensureHash(Lookup&& l) {

js/src/vm/SavedStacks.cpp

@@ -273,8 +273,14 @@ class MutableWrappedPtrOperations<SavedFrame::Lookup, Wrapper>
 };
 
 /* static */
-bool SavedFrame::HashPolicy::hasHash(const Lookup& l) {
-  return SavedFramePtrHasher::hasHash(l.parent);
+bool SavedFrame::HashPolicy::maybeGetHash(const Lookup& l,
+                                          HashNumber* hashOut) {
+  HashNumber parentHash;
+  if (!SavedFramePtrHasher::maybeGetHash(l.parent, &parentHash)) {
+    return false;
+  }
+  *hashOut = calculateHash(l, parentHash);
+  return true;
 }
 
 /* static */
@@ -284,13 +290,19 @@ bool SavedFrame::HashPolicy::ensureHash(const Lookup& l) {
 
 /* static */
 HashNumber SavedFrame::HashPolicy::hash(const Lookup& lookup) {
+  return calculateHash(lookup, SavedFramePtrHasher::hash(lookup.parent));
+}
+
+/* static */
+HashNumber SavedFrame::HashPolicy::calculateHash(const Lookup& lookup,
+                                                 HashNumber parentHash) {
   JS::AutoCheckCannotGC nogc;
   // Assume that we can take line mod 2^32 without losing anything of
   // interest.  If that assumption changes, we'll just need to start with 0
   // and add another overload of AddToHash with more arguments.
   return AddToHash(lookup.line, lookup.column, lookup.source,
                    lookup.functionDisplayName, lookup.asyncCause,
-                   lookup.mutedErrors, SavedFramePtrHasher::hash(lookup.parent),
+                   lookup.mutedErrors, parentHash,
                    JSPrincipalsPtrHasher::hash(lookup.principals));
 }
 

js/src/vm/TaggedProto.h

@@ -63,8 +63,13 @@ struct MovableCellHasher<TaggedProto> {
   using Key = TaggedProto;
   using Lookup = TaggedProto;
 
-  static bool hasHash(const Lookup& l) {
-    return !l.isObject() || MovableCellHasher<JSObject*>::hasHash(l.toObject());
+  static bool maybeGetHash(const Lookup& l, HashNumber* hashOut) {
+    if (!l.isObject()) {
+      *hashOut = hash(l);
+      return true;
+    }
+
+    return MovableCellHasher<JSObject*>::maybeGetHash(l.toObject(), hashOut);
   }
   static bool ensureHash(const Lookup& l) {
     return !l.isObject() ||

mfbt/HashTable.h

@@ -879,33 +879,42 @@ struct CStringHasher {
 // Fallible Hashing Interface
 //---------------------------------------------------------------------------
 
-// Most of the time generating a hash code is infallible so this class provides
-// default methods that always succeed.  Specialize this class for your own hash
-// policy to provide fallible hashing.
+// Most of the time generating a hash code is infallible, but sometimes it is
+// necessary to generate hash codes on demand in a way that can fail. Specialize
+// this class for your own hash policy to provide fallible hashing.
 //
 // This is used by MovableCellHasher to handle the fact that generating a unique
 // ID for cell pointer may fail due to OOM.
+//
+// The default implementations of these methods delegate to the usual HashPolicy
+// implementation and always succeed.
 template <typename HashPolicy>
 struct FallibleHashMethods {
-  // Return true if a hashcode is already available for its argument.  Once
-  // this returns true for a specific argument it must continue to do so.
+  // Return true if a hashcode is already available for its argument, and
+  // sets |aHashOut|. Once this succeeds for a specific argument it
+  // must continue to do so.
+  //
+  // Return false if a hashcode is not already available. This implies that any
+  // lookup must fail, as the hash code would have to have been successfully
+  // created on insertion.
   template <typename Lookup>
-  static bool hasHash(Lookup&& aLookup) {
+  static bool maybeGetHash(Lookup&& aLookup, HashNumber* aHashOut) {
+    *aHashOut = HashPolicy::hash(aLookup);
     return true;
   }
 
   // Fallible method to ensure a hashcode exists for its argument and create
-  // one if not.  Returns false on error, e.g. out of memory.
+  // one if not. Returns false on error, e.g. out of memory.
   template <typename Lookup>
   static bool ensureHash(Lookup&& aLookup) {
     return true;
   }
 };
 
 template <typename HashPolicy, typename Lookup>
-static bool HasHash(Lookup&& aLookup) {
-  return FallibleHashMethods<typename HashPolicy::Base>::hasHash(
-      std::forward<Lookup>(aLookup));
+static bool MaybeGetHash(Lookup&& aLookup, HashNumber* aHashOut) {
+  return FallibleHashMethods<typename HashPolicy::Base>::maybeGetHash(
+      std::forward<Lookup>(aLookup), aHashOut);
 }
 
 template <typename HashPolicy, typename Lookup>
@@ -1634,8 +1643,8 @@ class HashTable : private AllocPolicy {
 
   static bool isLiveHash(HashNumber aHash) { return Entry::isLiveHash(aHash); }
 
-  static HashNumber prepareHash(const Lookup& aLookup) {
-    HashNumber keyHash = ScrambleHashCode(HashPolicy::hash(aLookup));
+  static HashNumber prepareHash(HashNumber aInputHash) {
+    HashNumber keyHash = ScrambleHashCode(aInputHash);
 
     // Avoid reserved hash codes.
     if (!isLiveHash(keyHash)) {
@@ -1980,7 +1989,7 @@ class HashTable : private AllocPolicy {
   void putNewInfallibleInternal(const Lookup& aLookup, Args&&... aArgs) {
     MOZ_ASSERT(mTable);
 
-    HashNumber keyHash = prepareHash(aLookup);
+    HashNumber keyHash = prepareHash(HashPolicy::hash(aLookup));
     Slot slot = findNonLiveSlot(keyHash);
 
     if (slot.isRemoved()) {
@@ -2076,10 +2085,16 @@ class HashTable : private AllocPolicy {
   }
 
   MOZ_ALWAYS_INLINE Ptr readonlyThreadsafeLookup(const Lookup& aLookup) const {
-    if (empty() || !HasHash<HashPolicy>(aLookup)) {
+    if (empty()) {
       return Ptr();
     }
-    HashNumber keyHash = prepareHash(aLookup);
+
+    HashNumber inputHash;
+    if (!MaybeGetHash<HashPolicy>(aLookup, &inputHash)) {
+      return Ptr();
+    }
+
+    HashNumber keyHash = prepareHash(inputHash);
     return Ptr(lookup<ForNonAdd>(aLookup, keyHash), *this);
   }
 
@@ -2094,7 +2109,7 @@ class HashTable : private AllocPolicy {
       return AddPtr();
     }
 
-    HashNumber keyHash = prepareHash(aLookup);
+    HashNumber keyHash = prepareHash(HashPolicy::hash(aLookup));
 
     if (!mTable) {
       return AddPtr(*this, keyHash);
@@ -2209,7 +2224,7 @@ class HashTable : private AllocPolicy {
     if (mTable) {
       ReentrancyGuard g(*this);
       // Check that aLookup has not been destroyed.
-      MOZ_ASSERT(prepareHash(aLookup) == aPtr.mKeyHash);
+      MOZ_ASSERT(prepareHash(HashPolicy::hash(aLookup)) == aPtr.mKeyHash);
       aPtr.mSlot = lookup<ForAdd>(aLookup, aPtr.mKeyHash);
       if (aPtr.found()) {
         return true;