Bug 1591132 - Make it easy to switch on and off these assertions in different build configurations. r=froydnj

Put them behind a MOZ_HASH_TABLE_CHECKS_ENABLED define, which right now is only
defined in DEBUG builds, preserving behavior.

MakeImmutable becomes an empty inline function when disabled, which should be
zero-cost.

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

--HG--
extra : moz-landing-system : lando

Author: emilio

dom/indexedDB/ActorsParent.cpp

@@ -14471,13 +14471,9 @@ void VersionChangeTransaction::UpdateMetadata(nsresult aResult) {
           indexIter.Remove();
         }
       }
-#ifdef DEBUG
       metadata->mIndexes.MarkImmutable();
-#endif
     }
-#ifdef DEBUG
     info->mMetadata->mObjectStores.MarkImmutable();
-#endif
   } else {
     // Replace metadata pointers for all live databases.
     info->mMetadata = oldMetadata.forget();
@@ -18611,10 +18607,7 @@ nsresult DatabaseOperationBase::GetUniqueIndexTableForObjectStore(
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
-#ifdef DEBUG
   aMaybeUniqueIndexTable.ref().MarkImmutable();
-#endif
-
   return NS_OK;
 }
 
@@ -24190,11 +24183,9 @@ bool ObjectStoreAddOrPutRequestOp::Init(TransactionBase* aTransaction) {
     mUniqueIndexTable.emplace();
   }
 
-#ifdef DEBUG
   if (mUniqueIndexTable.isSome()) {
     mUniqueIndexTable.ref().MarkImmutable();
   }
-#endif
 
   const nsTArray<FileAddInfo>& fileAddInfos = mParams.fileAddInfos();
 

toolkit/components/telemetry/core/TelemetryOrigin.cpp

@@ -319,10 +319,8 @@ void TelemetryOrigin::InitializeGlobalState() {
 
   // This map shouldn't change at runtime, so make debug builds complain
   // if it tries.
-#ifdef DEBUG
   gOriginToIndexMap->MarkImmutable();
   gHashToIndexMap->MarkImmutable();
-#endif  // DEBUG
 
   gInitDone = true;
 }

xpcom/ds/PLDHashTable.cpp

@@ -21,7 +21,7 @@
 
 using namespace mozilla;
 
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
 
 class AutoReadOp {
   Checker& mChk;
@@ -222,7 +222,7 @@ PLDHashTable& PLDHashTable::operator=(PLDHashTable&& aOther) {
   mEntryCount = std::move(aOther.mEntryCount);
   mRemovedCount = std::move(aOther.mRemovedCount);
   mEntryStore.Set(aOther.mEntryStore.Get(), &mGeneration);
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
   mChecker = std::move(aOther.mChecker);
 #endif
 
@@ -231,7 +231,7 @@ PLDHashTable& PLDHashTable::operator=(PLDHashTable&& aOther) {
   // Clear up |aOther| so its destruction will be a no-op and it reports being
   // empty.
   {
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
     AutoDestructorOp op(mChecker);
 #endif
     aOther.mEntryCount = 0;
@@ -285,7 +285,7 @@ auto PLDHashTable::SlotForIndex(uint32_t aIndex) const -> Slot {
 }
 
 PLDHashTable::~PLDHashTable() {
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
   AutoDestructorOp op(mChecker);
 #endif
 
@@ -495,7 +495,7 @@ PLDHashTable::ComputeKeyHash(const void* aKey) const {
 }
 
 PLDHashEntryHdr* PLDHashTable::Search(const void* aKey) const {
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
   AutoReadOp op(mChecker);
 #endif
 
@@ -511,7 +511,7 @@ PLDHashEntryHdr* PLDHashTable::Search(const void* aKey) const {
 
 PLDHashEntryHdr* PLDHashTable::Add(const void* aKey,
                                    const mozilla::fallible_t&) {
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
   AutoWriteOp op(mChecker);
 #endif
 
@@ -593,7 +593,7 @@ PLDHashEntryHdr* PLDHashTable::Add(const void* aKey) {
 }
 
 void PLDHashTable::Remove(const void* aKey) {
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
   AutoWriteOp op(mChecker);
 #endif
 
@@ -614,7 +614,7 @@ void PLDHashTable::Remove(const void* aKey) {
 }
 
 void PLDHashTable::RemoveEntry(PLDHashEntryHdr* aEntry) {
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
   AutoWriteOp op(mChecker);
 #endif
 
@@ -669,7 +669,7 @@ void PLDHashTable::ShrinkIfAppropriate() {
 
 size_t PLDHashTable::ShallowSizeOfExcludingThis(
     MallocSizeOf aMallocSizeOf) const {
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
   AutoReadOp op(mChecker);
 #endif
 
@@ -705,7 +705,7 @@ PLDHashTable::Iterator::Iterator(PLDHashTable* aTable)
       mNextsLimit(mTable->EntryCount()),
       mHaveRemoved(false),
       mEntrySize(aTable->mEntrySize) {
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
   mTable->mChecker.StartReadOp();
 #endif
 
@@ -733,7 +733,7 @@ PLDHashTable::Iterator::Iterator(PLDHashTable* aTable, EndIteratorTag aTag)
       mNextsLimit(mTable->EntryCount()),
       mHaveRemoved(false),
       mEntrySize(aTable->mEntrySize) {
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
   mTable->mChecker.StartReadOp();
 #endif
 
@@ -750,7 +750,7 @@ PLDHashTable::Iterator::Iterator(const Iterator& aOther)
   // TODO: Is this necessary?
   MOZ_ASSERT(!mHaveRemoved);
 
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
   mTable->mChecker.StartReadOp();
 #endif
 }
@@ -760,7 +760,7 @@ PLDHashTable::Iterator::~Iterator() {
     if (mHaveRemoved) {
       mTable->ShrinkIfAppropriate();
     }
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
     mTable->mChecker.EndReadOp();
 #endif
   }
@@ -820,7 +820,3 @@ void PLDHashTable::Iterator::Remove() {
   mTable->RawRemove(mCurrent);
   mHaveRemoved = true;
 }
-
-#ifdef DEBUG
-void PLDHashTable::MarkImmutable() { mChecker.SetNonWritable(); }
-#endif

xpcom/ds/PLDHashTable.h

@@ -23,6 +23,10 @@
 using PLDHashNumber = mozilla::HashNumber;
 static const uint32_t kPLDHashNumberBits = mozilla::kHashNumberBits;
 
+#ifdef DEBUG
+#define MOZ_HASH_TABLE_CHECKS_ENABLED 1
+#endif
+
 class PLDHashTable;
 struct PLDHashTableOps;
 
@@ -52,7 +56,7 @@ struct PLDHashEntryHdr {
   friend class PLDHashTable;
 };
 
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
 
 // This class does three kinds of checking:
 //
@@ -378,7 +382,7 @@ class PLDHashTable {
   uint32_t mEntryCount;               // Number of entries in table.
   uint32_t mRemovedCount;             // Removed entry sentinels in table.
 
-#ifdef DEBUG
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
   mutable Checker mChecker;
 #endif
 
@@ -515,12 +519,14 @@ class PLDHashTable {
   // Like ShallowSizeOfExcludingThis(), but includes sizeof(*this).
   size_t ShallowSizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
-#ifdef DEBUG
   // Mark a table as immutable for the remainder of its lifetime. This
   // changes the implementation from asserting one set of invariants to
   // asserting a different set.
-  void MarkImmutable();
+  void MarkImmutable() {
+#ifdef MOZ_HASH_TABLE_CHECKS_ENABLED
+    mChecker.SetNonWritable();
 #endif
+  }
 
   // If you use PLDHashEntryStub or a subclass of it as your entry struct, and
   // if your entries move via memcpy and clear via memset(0), you can use these

xpcom/ds/nsBaseHashtable.h

@@ -511,9 +511,7 @@ class nsBaseHashtable
     nsTHashtable<EntryType>::SwapElements(aOther);
   }
 
-#ifdef DEBUG
   using nsTHashtable<EntryType>::MarkImmutable;
-#endif
 };
 
 //

xpcom/ds/nsStaticNameTable.cpp

@@ -134,9 +134,7 @@ nsStaticCaseInsensitiveNameTable::nsStaticCaseInsensitiveNameTable(
 
     entry->mIndex = index;
   }
-#ifdef DEBUG
   mNameTable.MarkImmutable();
-#endif
 }
 
 nsStaticCaseInsensitiveNameTable::~nsStaticCaseInsensitiveNameTable() {

xpcom/ds/nsTHashtable.h

@@ -309,15 +309,13 @@ class MOZ_NEEDS_NO_VTABLE_TYPE nsTHashtable {
     mozilla::Swap(this->mTable, aOther.mTable);
   }
 
-#ifdef DEBUG
   /**
    * Mark the table as constant after initialization.
    *
    * This will prevent assertions when a read-only hash is accessed on multiple
    * threads without synchronization.
    */
   void MarkImmutable() { mTable.MarkImmutable(); }
-#endif
 
  protected:
   PLDHashTable mTable;
@@ -515,10 +513,7 @@ class nsTHashtable<nsPtrHashKey<T>>
 
   using Base::ShallowSizeOfExcludingThis;
   using Base::ShallowSizeOfIncludingThis;
-
-#ifdef DEBUG
   using Base::MarkImmutable;
-#endif
 
   /* Wrapper functions */
   EntryType* GetEntry(T* aKey) const {