Bug 1050035 (part 1) - 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. There is more to be done w.r.t. simplifying initialization, which will occur in the next patch. --HG-- extra : rebase_source : b9bfdcd680f39f3c947a49ae8462c04bc5e38805
2015-02-01 14:56:33 -08:00 · 2015-02-01 14:56:33 -08:00 · 2438e023a8
--- a/xpcom/glue/pldhash.cpp
+++ b/xpcom/glue/pldhash.cpp
@ -247,11 +247,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.
@ -348,6 +345,7 @@ PLDHashTable::Finish()

  /* Free entry storage last. */
  free(mEntryStore);
+  mEntryStore = nullptr;
 }

 void
@ -365,6 +363,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)");
@ -445,6 +444,7 @@ PLDHashEntryHdr* PL_DHASH_FASTCALL
 PLDHashTable::FindFreeEntry(PLDHashNumber aKeyHash)
 {
  METER(mStats.mSearches++);
+  MOZ_ASSERT(mEntryStore);
  NS_ASSERTION(!(aKeyHash & COLLISION_FLAG),
               "!(aKeyHash & COLLISION_FLAG)");

@ -486,6 +486,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;
@ -561,8 +563,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);

@ -574,16 +577,29 @@ PLDHashTable::Add(const void* aKey)
 {
  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;
+    if (!SizeOfEntryStore(CapacityFromHashShift(), mEntrySize, &nbytes) ||
+        !(mEntryStore = (char*)malloc(nbytes))) {
+      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;
@ -613,7 +629,7 @@ PLDHashTable::Add(const void* aKey)
   * then skip it while growing the table and re-add it after.
   */
  keyHash = ComputeKeyHash(aKey);
-  entry = SearchTable<ForAdd>(aKey, keyHash);
+  entry = mEntryStore ? SearchTable<ForAdd>(aKey, keyHash) : nullptr;
  if (!ENTRY_IS_LIVE(entry)) {
    /* Initialize the entry, indicating that it's no longer free. */
    METER(mStats.mAddMisses++);
@ -646,8 +662,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++);
@ -736,6 +753,7 @@ PLDHashTable::Enumerate(PLDHashEnumerator aEtor, void* aArg)
    }
  }

+  MOZ_ASSERT_IF(capacity > 0, mEntryStore);
  for (uint32_t e = 0; e < capacity; ++e) {
    PLDHashEntryHdr* entry = (PLDHashEntryHdr*)entryAddr;
    if (ENTRY_IS_LIVE(entry)) {
@ -815,6 +833,10 @@ PLDHashTable::SizeOfExcludingThis(
    PLDHashSizeOfEntryExcludingThisFun aSizeOfEntryExcludingThis,
    MallocSizeOf aMallocSizeOf, void* aArg /* = nullptr */) const
 {
+  if (!mEntryStore) {
+    return 0;
+  }
+
  size_t n = 0;
  n += aMallocSizeOf(mEntryStore);
  if (aSizeOfEntryExcludingThis) {
@ -927,6 +949,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;

@ -980,6 +1003,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;
--- a/xpcom/glue/pldhash.h
+++ b/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; }
@ -297,6 +299,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 };
@ -441,12 +450,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,
--- a/xpcom/tests/TestPLDHash.cpp
+++ b/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),