Bug 1050035 (part 1, attempt 2) - Lazily allocate PLDHashTable::mEntryStore. r=froydnj.

This makes zero-element hash tables, which are common, smaller, and also avoids
unnecessary malloc/free pairs.

I did some measurements during some basic browsing of a few sites. I found that
35% of all live tables were empty with a few tabs open. And cumulatively, for
the whole session, 45% of tables never had an element added to them.

xpcom/glue/pldhash.cpp

@@ -250,11 +250,8 @@ PLDHashTable::Init(const PLDHashTableOps* aOps,
     return false;  // overflowed
   }
 
-  mEntryStore = (char*)malloc(nbytes);
-  if (!mEntryStore) {
-    return false;
-  }
-  memset(mEntryStore, 0, nbytes);
+  mEntryStore = nullptr;
+
   METER(memset(&mStats, 0, sizeof(mStats)));
 
   // Set this only once we reach a point where we know we can't fail.
@@ -371,6 +368,7 @@ template <PLDHashTable::SearchReason Reason>
 PLDHashEntryHdr* PL_DHASH_FASTCALL
 PLDHashTable::SearchTable(const void* aKey, PLDHashNumber aKeyHash)
 {
+  MOZ_ASSERT(mEntryStore);
   METER(mStats.mSearches++);
   NS_ASSERTION(!(aKeyHash & COLLISION_FLAG),
                "!(aKeyHash & COLLISION_FLAG)");
@@ -451,6 +449,7 @@ PLDHashEntryHdr* PL_DHASH_FASTCALL
 PLDHashTable::FindFreeEntry(PLDHashNumber aKeyHash)
 {
   METER(mStats.mSearches++);
+  MOZ_ASSERT(mEntryStore);
   NS_ASSERTION(!(aKeyHash & COLLISION_FLAG),
                "!(aKeyHash & COLLISION_FLAG)");
 
@@ -492,6 +491,8 @@ PLDHashTable::FindFreeEntry(PLDHashNumber aKeyHash)
 bool
 PLDHashTable::ChangeTable(int aDeltaLog2)
 {
+  MOZ_ASSERT(mEntryStore);
+
   /* Look, but don't touch, until we succeed in getting new entry store. */
   int oldLog2 = PL_DHASH_BITS - mHashShift;
   int newLog2 = oldLog2 + aDeltaLog2;
@@ -550,6 +551,8 @@ PLDHashTable::ChangeTable(int aDeltaLog2)
 MOZ_ALWAYS_INLINE PLDHashNumber
 PLDHashTable::ComputeKeyHash(const void* aKey)
 {
+  MOZ_ASSERT(mEntryStore);
+
   PLDHashNumber keyHash = mOps->hashKey(this, aKey);
   keyHash *= PL_DHASH_GOLDEN_RATIO;
 
@@ -569,8 +572,9 @@ PLDHashTable::Search(const void* aKey)
 
   METER(mStats.mSearches++);
 
-  PLDHashNumber keyHash = ComputeKeyHash(aKey);
-  PLDHashEntryHdr* entry = SearchTable<ForSearchOrRemove>(aKey, keyHash);
+  PLDHashEntryHdr* entry =
+    mEntryStore ? SearchTable<ForSearchOrRemove>(aKey, ComputeKeyHash(aKey))
+                : nullptr;
 
   DECREMENT_RECURSION_LEVEL(this);
 
@@ -584,16 +588,32 @@ PLDHashTable::Add(const void* aKey, const mozilla::fallible_t&)
 
   PLDHashNumber keyHash;
   PLDHashEntryHdr* entry;
+  uint32_t capacity;
 
   MOZ_ASSERT(mRecursionLevel == 0);
   INCREMENT_RECURSION_LEVEL(this);
 
+  // Allocate the entry storage if it hasn't already been allocated.
+  if (!mEntryStore) {
+    uint32_t nbytes;
+    // We already checked this in Init(), so it must still be true.
+    MOZ_RELEASE_ASSERT(SizeOfEntryStore(CapacityFromHashShift(), mEntrySize,
+                                        &nbytes));
+    mEntryStore = (char*)malloc(nbytes);
+    if (!mEntryStore) {
+      METER(mStats.mAddFailures++);
+      entry = nullptr;
+      goto exit;
+    }
+    memset(mEntryStore, 0, nbytes);
+  }
+
   /*
    * If alpha is >= .75, grow or compress the table.  If aKey is already
    * in the table, we may grow once more than necessary, but only if we
    * are on the edge of being overloaded.
    */
-  uint32_t capacity = Capacity();
+  capacity = Capacity();
   if (mEntryCount + mRemovedCount >= MaxLoad(capacity)) {
     /* Compress if a quarter or more of all entries are removed. */
     int deltaLog2;
@@ -647,6 +667,27 @@ exit:
   return entry;
 }
 
+MOZ_ALWAYS_INLINE PLDHashEntryHdr*
+PLDHashTable::Add(const void* aKey)
+{
+  PLDHashEntryHdr* entry = Add(aKey, fallible);
+  if (!entry) {
+    if (!mEntryStore) {
+      // We OOM'd while allocating the initial entry storage.
+      uint32_t nbytes;
+      (void) SizeOfEntryStore(CapacityFromHashShift(), mEntrySize, &nbytes);
+      NS_ABORT_OOM(nbytes);
+    } else {
+      // We failed to resize the existing entry storage, either due to OOM or
+      // because we exceeded the maximum table capacity or size; report it as
+      // an OOM. The multiplication by 2 gets us the size we tried to allocate,
+      // which is double the current size.
+      NS_ABORT_OOM(2 * EntrySize() * EntryCount());
+    }
+  }
+  return entry;
+}
+
 MOZ_ALWAYS_INLINE void
 PLDHashTable::Remove(const void* aKey)
 {
@@ -655,8 +696,9 @@ PLDHashTable::Remove(const void* aKey)
   MOZ_ASSERT(mRecursionLevel == 0);
   INCREMENT_RECURSION_LEVEL(this);
 
-  PLDHashNumber keyHash = ComputeKeyHash(aKey);
-  PLDHashEntryHdr* entry = SearchTable<ForSearchOrRemove>(aKey, keyHash);
+  PLDHashEntryHdr* entry =
+    mEntryStore ? SearchTable<ForSearchOrRemove>(aKey, ComputeKeyHash(aKey))
+                : nullptr;
   if (entry) {
     /* Clear this entry and mark it as "removed". */
     METER(mStats.mRemoveHits++);
@@ -693,12 +735,7 @@ PL_DHashTableAdd(PLDHashTable* aTable, const void* aKey,
 PLDHashEntryHdr* PL_DHASH_FASTCALL
 PL_DHashTableAdd(PLDHashTable* aTable, const void* aKey)
 {
-  PLDHashEntryHdr* entry = PL_DHashTableAdd(aTable, aKey, fallible);
-  if (!entry) {
-    // Entry storage reallocation failed.
-    NS_ABORT_OOM(aTable->EntrySize() * aTable->EntryCount());
-  }
-  return entry;
+  return aTable->Add(aKey);
 }
 
 void PL_DHASH_FASTCALL
@@ -711,6 +748,7 @@ MOZ_ALWAYS_INLINE void
 PLDHashTable::RawRemove(PLDHashEntryHdr* aEntry)
 {
   MOZ_ASSERT(IsInitialized());
+  MOZ_ASSERT(mEntryStore);
 
   MOZ_ASSERT(mRecursionLevel != IMMUTABLE_RECURSION_LEVEL);
 
@@ -740,6 +778,10 @@ PLDHashTable::Enumerate(PLDHashEnumerator aEtor, void* aArg)
 {
   MOZ_ASSERT(IsInitialized());
 
+  if (!mEntryStore) {
+    return 0;
+  }
+
   INCREMENT_RECURSION_LEVEL(this);
 
   // Please keep this method in sync with the PLDHashTable::Iterator constructor
@@ -842,6 +884,10 @@ PLDHashTable::SizeOfExcludingThis(
 {
   MOZ_ASSERT(IsInitialized());
 
+  if (!mEntryStore) {
+    return 0;
+  }
+
   size_t n = 0;
   n += aMallocSizeOf(mEntryStore);
   if (aSizeOfEntryExcludingThis) {
@@ -964,6 +1010,7 @@ PLDHashEntryHdr* PLDHashTable::Iterator::NextEntry()
   // checks pass, then this method will only iterate through the full capacity
   // once. If they fail, then this loop may end up returning the early entries
   // more than once.
+  MOZ_ASSERT_IF(capacity > 0, mTable->mEntryStore);
   for (uint32_t e = 0; e < capacity; ++e) {
     PLDHashEntryHdr* entry = (PLDHashEntryHdr*)mEntryAddr;
 
@@ -1021,6 +1068,7 @@ PLDHashTable::DumpMeter(PLDHashEnumerator aDump, FILE* aFp)
   hash2 = 0;
   sqsum = 0;
 
+  MOZ_ASSERT_IF(capacity > 0, mEntryStore);
   for (uint32_t i = 0; i < capacity; i++) {
     entry = (PLDHashEntryHdr*)entryAddr;
     entryAddr += mEntrySize;

xpcom/glue/pldhash.h

@@ -147,6 +147,9 @@ typedef size_t (*PLDHashSizeOfEntryExcludingThisFun)(
  * on most architectures, and may be allocated on the stack or within another
  * structure or class (see below for the Init and Finish functions to use).
  *
+ * No entry storage is allocated until the first element is added. This means
+ * that empty hash tables are cheap, which is good because they are common.
+ *
  * There used to be a long, math-heavy comment here about the merits of
  * double hashing vs. chaining; it was removed in bug 1058335. In short, double
  * hashing is more space-efficient unless the element size gets large (in which
@@ -175,8 +178,7 @@ private:
   uint32_t            mEntryCount;    /* number of entries in table */
   uint32_t            mRemovedCount;  /* removed entry sentinels in table */
   uint32_t            mGeneration;    /* entry storage generation number */
-  char*               mEntryStore;    /* entry storage */
-
+  char*               mEntryStore;    /* entry storage; allocated lazily */
 #ifdef PL_DHASHMETER
   struct PLDHashStats
   {
@@ -226,12 +228,12 @@ public:
 
   /*
    * Size in entries (gross, not net of free and removed sentinels) for table.
-   * We store mHashShift rather than sizeLog2 to optimize the collision-free
-   * case in SearchTable.
+   * This can be zero if no elements have been added yet, in which case the
+   * entry storage will not have yet been allocated.
    */
   uint32_t Capacity() const
   {
-    return ((uint32_t)1 << (PL_DHASH_BITS - mHashShift));
+    return mEntryStore ? CapacityFromHashShift() : 0;
   }
 
   uint32_t EntrySize()  const { return mEntrySize; }
@@ -245,6 +247,7 @@ public:
 
   PLDHashEntryHdr* Search(const void* aKey);
   PLDHashEntryHdr* Add(const void* aKey, const mozilla::fallible_t&);
+  PLDHashEntryHdr* Add(const void* aKey);
   void Remove(const void* aKey);
 
   void RawRemove(PLDHashEntryHdr* aEntry);
@@ -297,6 +300,13 @@ public:
 private:
   static bool EntryIsFree(PLDHashEntryHdr* aEntry);
 
+  // We store mHashShift rather than sizeLog2 to optimize the collision-free
+  // case in SearchTable.
+  uint32_t CapacityFromHashShift() const
+  {
+    return ((uint32_t)1 << (PL_DHASH_BITS - mHashShift));
+  }
+
   PLDHashNumber ComputeKeyHash(const void* aKey);
 
   enum SearchReason { ForSearchOrRemove, ForAdd };
@@ -439,12 +449,14 @@ PLDHashTable* PL_NewDHashTable(
 void PL_DHashTableDestroy(PLDHashTable* aTable);
 
 /*
- * Initialize aTable with aOps, aEntrySize, and aCapacity. The table's initial
- * capacity will be chosen such that |aLength| elements can be inserted without
- * rehashing. If |aLength| is a power-of-two, this capacity will be |2*length|.
+ * Initialize aTable with aOps and aEntrySize. The table's initial capacity
+ * will be chosen such that |aLength| elements can be inserted without
+ * rehashing; if |aLength| is a power-of-two, this capacity will be |2*length|.
+ * However, because entry storage is allocated lazily, this initial capacity
+ * won't be relevant until the first element is added; prior to that the
+ * capacity will be zero.
  *
- * This function will crash if it can't allocate enough memory, or if
- * |aEntrySize| and/or |aLength| are too large.
+ * This function will crash if |aEntrySize| and/or |aLength| are too large.
  */
 void PL_DHashTableInit(
   PLDHashTable* aTable, const PLDHashTableOps* aOps,

xpcom/tests/TestPLDHash.cpp

@@ -7,10 +7,8 @@
 #include <stdio.h>
 #include "pldhash.h"
 
-// pldhash is very widely used and so any basic bugs in it are likely to be
-// exposed through normal usage.  Therefore, this test currently focusses on
-// extreme cases relating to maximum table capacity and potential overflows,
-// which are unlikely to be hit during normal execution.
+// This test mostly focuses on edge cases. But more coverage of normal
+// operations wouldn't be a bad thing.
 
 namespace TestPLDHash {
 
@@ -106,6 +104,67 @@ static bool test_pldhash_Init_overflow()
   return true;
 }
 
+static bool test_pldhash_lazy_storage()
+{
+  PLDHashTable t;
+  PL_DHashTableInit(&t, PL_DHashGetStubOps(), sizeof(PLDHashEntryStub));
+
+  // PLDHashTable allocates entry storage lazily. Check that all the non-add
+  // operations work appropriately when the table is empty and the storage
+  // hasn't yet been allocated.
+
+  if (!t.IsInitialized()) {
+    return false;
+  }
+
+  if (t.Capacity() != 0) {
+    return false;
+  }
+
+  if (t.EntrySize() != sizeof(PLDHashEntryStub)) {
+    return false;
+  }
+
+  if (t.EntryCount() != 0) {
+    return false;
+  }
+
+  if (t.Generation() != 0) {
+    return false;
+  }
+
+  if (PL_DHashTableSearch(&t, (const void*)1)) {
+    return false;   // search succeeded?
+  }
+
+  // No result to check here, but call it to make sure it doesn't crash.
+  PL_DHashTableRemove(&t, (const void*)2);
+
+  // Using a null |enumerator| should be fine because it shouldn't be called
+  // for an empty table.
+  PLDHashEnumerator enumerator = nullptr;
+  if (PL_DHashTableEnumerate(&t, enumerator, nullptr) != 0) {
+    return false;   // enumeration count is non-zero?
+  }
+
+  for (PLDHashTable::Iterator iter = t.Iterate();
+       iter.HasMoreEntries();
+       iter.NextEntry()) {
+    return false; // shouldn't hit this on an empty table
+  }
+
+  // Using a null |mallocSizeOf| should be fine because it shouldn't be called
+  // for an empty table.
+  mozilla::MallocSizeOf mallocSizeOf = nullptr;
+  if (PL_DHashTableSizeOfExcludingThis(&t, nullptr, mallocSizeOf) != 0) {
+    return false;   // size is non-zero?
+  }
+
+  PL_DHashTableFinish(&t);
+
+  return true;
+}
+
 // See bug 931062, we skip this test on Android due to OOM.
 #ifndef MOZ_WIDGET_ANDROID
 // We insert the integers 0.., so this is has function is (a) as simple as
@@ -168,6 +227,7 @@ static const struct Test {
   DECL_TEST(test_pldhash_Init_capacity_ok),
   DECL_TEST(test_pldhash_Init_capacity_too_large),
   DECL_TEST(test_pldhash_Init_overflow),
+  DECL_TEST(test_pldhash_lazy_storage),
 // See bug 931062, we skip this test on Android due to OOM.
 #ifndef MOZ_WIDGET_ANDROID
   DECL_TEST(test_pldhash_grow_to_max_capacity),