Backed out changeset 3009a0b538da (bug 1377333) on suspicion of causing frequent failures in test_general.html. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Tue, 04 Jul 2017 00:37:24 +0200
changeset 367196 f5564d2bbc096bb5bc6c7a0e2fba2852c5e81739
parent 367195 2475913ce86b273aa55fcc9dddb457098616d829
child 367197 ca52840495166f1d15460ac281de809a4bfcaf9a
push id32125
push usercbook@mozilla.com
push dateTue, 04 Jul 2017 08:48:50 +0000
treeherdermozilla-central@fef489e8c2a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1377333
milestone56.0a1
backs out3009a0b538da9fa3f6d9d9bf49f24ab46e5fc553
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 3009a0b538da (bug 1377333) on suspicion of causing frequent failures in test_general.html. r=backout
xpcom/ds/PLDHashTable.cpp
xpcom/ds/PLDHashTable.h
xpcom/ds/nsPointerHashKeys.h
--- a/xpcom/ds/PLDHashTable.cpp
+++ b/xpcom/ds/PLDHashTable.cpp
@@ -67,17 +67,20 @@ public:
 PLDHashTable::HashStringKey(const void* aKey)
 {
   return HashString(static_cast<const char*>(aKey));
 }
 
 /* static */ PLDHashNumber
 PLDHashTable::HashVoidPtrKeyStub(const void* aKey)
 {
-  return uintptr_t(aKey) >> 2;
+  // Be careful!  We don't want to do the cast to PLDHashNumber which is a
+  // trimming cast on 64-bit platforms before the shift, otherwise we will lose
+  // valuable bits from our hash key!
+  return PLDHashNumber(uintptr_t(aKey) >> 2);
 }
 
 /* static */ bool
 PLDHashTable::MatchEntryStub(const PLDHashEntryHdr* aEntry, const void* aKey)
 {
   const PLDHashEntryStub* stub = (const PLDHashEntryStub*)aEntry;
 
   return stub->key == aKey;
@@ -250,20 +253,20 @@ PLDHashTable::operator=(PLDHashTable&& a
 PLDHashNumber
 PLDHashTable::Hash1(PLDHashNumber aHash0)
 {
   return aHash0 >> mHashShift;
 }
 
 void
 PLDHashTable::Hash2(PLDHashNumber aHash0,
-                    size_t& aHash2Out, size_t& aSizeMaskOut)
+                    uint32_t& aHash2Out, uint32_t& aSizeMaskOut)
 {
-  size_t sizeLog2 = kHashBits - mHashShift;
-  size_t sizeMask = (PLDHashNumber(1) << sizeLog2) - 1;
+  uint32_t sizeLog2 = kHashBits - mHashShift;
+  uint32_t sizeMask = (PLDHashNumber(1) << sizeLog2) - 1;
   aSizeMaskOut = sizeMask;
 
   // The incoming aHash0 always has the low bit unset (since we leave it
   // free for the collision flag), and should have reasonably random
   // data in the other 31 bits.  We used the high bits of aHash0 for
   // Hash1, so we use the low bits here.  If the table size is large,
   // the bits we use may overlap, but that's still more random than
   // filling with 0s.
@@ -287,17 +290,17 @@ PLDHashTable::Hash2(PLDHashNumber aHash0
 /* static */ bool
 PLDHashTable::MatchEntryKeyhash(PLDHashEntryHdr* aEntry, PLDHashNumber aKeyHash)
 {
   return (aEntry->mKeyHash & ~kCollisionFlag) == aKeyHash;
 }
 
 // Compute the address of the indexed entry in table.
 PLDHashEntryHdr*
-PLDHashTable::AddressEntry(size_t aIndex)
+PLDHashTable::AddressEntry(uint32_t aIndex)
 {
   return reinterpret_cast<PLDHashEntryHdr*>(
     mEntryStore.Get() + aIndex * mEntrySize);
 }
 
 PLDHashTable::~PLDHashTable()
 {
 #ifdef DEBUG
@@ -365,17 +368,17 @@ PLDHashTable::SearchTable(const void* aK
   PLDHashMatchEntry matchEntry = mOps->matchEntry;
   if (MatchEntryKeyhash(entry, aKeyHash) &&
       matchEntry(entry, aKey)) {
     return entry;
   }
 
   // Collision: double hash.
   PLDHashNumber hash2;
-  size_t sizeMask;
+  uint32_t sizeMask;
   Hash2(aKeyHash, hash2, sizeMask);
 
   // Save the first removed entry pointer so Add() can recycle it. (Only used
   // if Reason==ForAdd.)
   PLDHashEntryHdr* firstRemoved = nullptr;
 
   for (;;) {
     if (Reason == ForAdd && !firstRemoved) {
@@ -425,17 +428,17 @@ PLDHashTable::FindFreeEntry(PLDHashNumbe
 
   // Miss: return space for a new entry.
   if (EntryIsFree(entry)) {
     return entry;
   }
 
   // Collision: double hash.
   PLDHashNumber hash2;
-  size_t sizeMask;
+  uint32_t sizeMask;
   Hash2(aKeyHash, hash2, sizeMask);
 
   for (;;) {
     NS_ASSERTION(!EntryIsRemoved(entry),
                  "!EntryIsRemoved(entry)");
     entry->mKeyHash |= kCollisionFlag;
 
     hash1 -= hash2;
--- a/xpcom/ds/PLDHashTable.h
+++ b/xpcom/ds/PLDHashTable.h
@@ -10,17 +10,17 @@
 #include "mozilla/Atomics.h"
 #include "mozilla/Attributes.h" // for MOZ_ALWAYS_INLINE
 #include "mozilla/fallible.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Move.h"
 #include "mozilla/Types.h"
 #include "nscore.h"
 
-typedef size_t PLDHashNumber;
+typedef uint32_t PLDHashNumber;
 
 class PLDHashTable;
 struct PLDHashTableOps;
 
 // Table entry header structure.
 //
 // In order to allow in-line allocation of key and value, we do not declare
 // either here. Instead, the API uses const void *key as a formal parameter.
@@ -485,25 +485,20 @@ public:
   // Use this if you need to initialize an Iterator in a const method. If you
   // use this case, you should not call Remove() on the iterator.
   Iterator ConstIter() const
   {
     return Iterator(const_cast<PLDHashTable*>(this));
   }
 
 private:
-  // Multiplicative hash uses an unsigned pointer sized integer and the golden ratio,
-  // expressed as a fixed-point pointer sized fraction.
-  static const uint32_t kHashBits = sizeof(size_t) * 8;
-#ifdef HAVE_64BIT_BUILD
-  // Golden ratio borrowed from http://burtleburtle.net/bob/hash/evahash.html.
-  static const uint64_t kGoldenRatio = 0X9E3779B97F4A7C13ULL;
-#else
+  // Multiplicative hash uses an unsigned 32 bit integer and the golden ratio,
+  // expressed as a fixed-point 32-bit fraction.
+  static const uint32_t kHashBits = 32;
   static const uint32_t kGoldenRatio = 0x9E3779B9U;
-#endif
 
   static uint32_t HashShift(uint32_t aEntrySize, uint32_t aLength);
 
   static const PLDHashNumber kCollisionFlag = 1;
 
   static bool EntryIsFree(PLDHashEntryHdr* aEntry)
   {
     return aEntry->mKeyHash == 0;
@@ -522,20 +517,20 @@ private:
     aEntry->mKeyHash = 0;
   }
   static void MarkEntryRemoved(PLDHashEntryHdr* aEntry)
   {
     aEntry->mKeyHash = 1;
   }
 
   PLDHashNumber Hash1(PLDHashNumber aHash0);
-  void Hash2(PLDHashNumber aHash, size_t& aHash2Out, size_t& aSizeMaskOut);
+  void Hash2(PLDHashNumber aHash, uint32_t& aHash2Out, uint32_t& aSizeMaskOut);
 
   static bool MatchEntryKeyhash(PLDHashEntryHdr* aEntry, PLDHashNumber aHash);
-  PLDHashEntryHdr* AddressEntry(size_t aIndex);
+  PLDHashEntryHdr* AddressEntry(uint32_t aIndex);
 
   // We store mHashShift rather than sizeLog2 to optimize the collision-free
   // case in SearchTable.
   uint32_t CapacityFromHashShift() const
   {
     return ((uint32_t)1 << (kHashBits - mHashShift));
   }
 
--- a/xpcom/ds/nsPointerHashKeys.h
+++ b/xpcom/ds/nsPointerHashKeys.h
@@ -30,17 +30,20 @@ public:
   ~nsPtrHashKey() {}
 
   KeyType GetKey() const { return mKey; }
   bool KeyEquals(KeyTypePointer aKey) const { return aKey == mKey; }
 
   static KeyTypePointer KeyToPointer(KeyType aKey) { return aKey; }
   static PLDHashNumber HashKey(KeyTypePointer aKey)
   {
-    return uintptr_t(aKey) >> 2;
+    // Be careful!  We don't want to do the cast to PLDHashNumber which is a
+    // trimming cast on 64-bit platforms before the shift, otherwise we will
+    // lose valuable bits from our hash key!
+    return PLDHashNumber(uintptr_t(aKey) >> 2);
   }
   enum { ALLOW_MEMMOVE = true };
 
 protected:
   T* MOZ_NON_OWNING_REF mKey;
 };
 
 typedef nsPtrHashKey<const void> nsVoidPtrHashKey;