Commit Graph

17 Commits

Author SHA1 Message Date
Ehsan Akhgari
43091e2839 Bug 1379282 - Improve XPCOM's pointer hashing functions for pointers to neighboring memory locations; r=erahm
The simplistic shift-based hashing function creates a lot of collisions
for pointers pointing to arrays as it doesn't do a great job at distributing
the data randomly based on the input bytes.
2017-07-19 14:16:35 -04:00
Ehsan Akhgari
3df4db262f Backout changeset 06f92d065a85 (bug 1377333) because we don't need keys that are that big 2017-07-18 23:06:00 -04:00
Ehsan Akhgari
ec3d589e4f Bug 1377333 - Make PLDHashNumber 64-bit on x86-64; r=froydnj 2017-07-04 11:05:21 -04:00
Sebastian Hengst
51f7ac9c81 Backed out changeset 3009a0b538da (bug 1377333) on suspicion of causing frequent failures in test_general.html. r=backout 2017-07-04 00:37:24 +02:00
Ehsan Akhgari
de718c51ec Bug 1377333 - Make PLDHashNumber 64-bit on x86-64; r=froydnj 2017-07-03 13:21:11 -04:00
Ehsan Akhgari
04f49b5fc2 Bug 1376563 - Improve the hash key generation for hashtables containing pointers on 64-bit platforms by using 2 more bits of the original pointer in calculating the hash key; r=froydnj 2017-06-27 18:18:32 -04:00
L. David Baron
07c2f2dc49 Bug 1352889 - Ensure that PLDHashTable's second hash doesn't have padding with 0 bits for tables with capacity larger than 2^16. r=njn
PLDHashTable takes the result of the hash function and multiplies it by
kGoldenRatio to ensure that it has a good distribution of bits across
the 32-bit hash value, and then zeroes out the low bit so that it can be
used for the collision flag.  This result is called hash0.  From hash0
it computes two different numbers used to find entries in the table
storage:  hash1 is used to find an initial position in the table to
begin searching for an entry; hash2 is then used to repeatedly offset
that position (mod the size of the table) to build a chain of positions
to search.

In a table with capacity 2^c entries, hash1 is simply the upper c bits
of hash0.  This patch does not change this.

Prior to this patch, hash2 was the c bits below hash1, padded at the low
end with zeroes when c > 16.  (Note that bug 927705, changeset
1a02bec165e16f370cace3da21bb2b377a0a7242, increased the maximum capacity
from 2^23 to 2^26 since 2^23 was sometimes insufficient!)  This manner
of computing hash2 is problematic because it increases the risk of long
chains for very large tables, since there is less variation in the hash2
result due to the zero padding.

So this patch changes the hash2 computation by using the low bits of
hash0 instead of shifting it around, thus avoiding 0 bits in parts of
the hash2 value that are significant.

Note that this changes what hash2 is in all cases except when the table
capacity is exactly 2^16, so it does change our hashing characteristics.
For tables with capacity less than 2^16, it should be using a different
second hash, but with the same amount of random-ish data.  For tables
with capacity greater than 2^16, it should be using more random-ish
data.

Note that this patch depends on the patch for bug 1353458 in order to
avoid causing test failures.

MozReview-Commit-ID: JvnxAMBY711

--HG--
extra : transplant_source : 2%D2%C2%CE%E1%92%C8%F8H%D7%15%A4%86%5B%3Ac%0B%08%3DA
2017-05-31 13:44:02 -07:00
L. David Baron
f22aab91c2 Bug 1352888 - Don't set the collision flag when adding to PLDHashTable if we've already found the entry we're going to add. r=njn
PLDHashTable's entry store has two types of unoccupied entries:  free
entries and removed entries.  The search of a chain of entries
(determined by the hash value) in the entry store to search for an entry
can stop at free entries, but it continues across removed entries,
because removed entries are entries that may have been skipped over when
we were adding the value we're searching for to the hash, but have since
been removed.  For live entries, we also maintain this distinction by
using one bit of storage for a collision flag, which notes that if the
hashtable entry is removed, its place in the entry store must become a
removed entry rather than a free entry.

When we add a new entry to the table, Add's semantics require that we
return an existing entry if there is one, and only create a new entry if
no existing entry exists.  (Bug 1352198 suggests the possibility of a
faster alternative Add API where the caller guarantees that the key is
not already in the hashtable.)  When we search for the existing entry,
we must thus continue the search across removed entries, even though we
record the first removed entry found to return if the search for an
existing entry fails.

The existing code adds the collision flag through the entire table
search during an Add.  This patch changes that behavior so that we only
add the collision flag prior to finding the first removed entry.  Adding
it after we find the first removed entry is unnecessary, since we are
not making that entry part of a path to a new entry.  If it is part of a
path to an existing entry, it will already have the collision flag set.

This patch effectively puts an if (!firstRemoved) around the else branch
of the if (MOZ_UNLIKELY(EntryIsRemoved(entry))), and then refactors that
condition outwards since it is now around the contents of both the if
and else branches.

MozReview-Commit-ID: CsXnMYttHVy

--HG--
extra : transplant_source : %80%9E%83%EC%CCY%B4%B0%86%86%18%99%B6U%21o%5D%29%AD%04
2017-05-31 13:44:02 -07:00
L. David Baron
f2c24eea94 Backed out changeset e4ac2148c920 (bug 1352888) to see if it is responsible for input latency regression bug 1362094.
--HG--
extra : rebase_source : bfb451e911058496c2a6498d658cb6ab777da8e6
2017-05-24 11:42:19 -04:00
L. David Baron
34315c4eee Backed out changeset 52fff3b1e209 (bug 1352889) to see if it is responsible for input latency regressions in bug 1365334 or bug 1366156.
--HG--
extra : rebase_source : 387401d1417a29c19e5dbc1ee7413d9d6f810d23
2017-05-24 11:41:02 -04:00
L. David Baron
5191ef7b3b Bug 1352889 - Ensure that PLDHashTable's second hash doesn't have padding with 0 bits for tables with capacity larger than 2^16. r=njn
PLDHashTable takes the result of the hash function and multiplies it by
kGoldenRatio to ensure that it has a good distribution of bits across
the 32-bit hash value, and then zeroes out the low bit so that it can be
used for the collision flag.  This result is called hash0.  From hash0
it computes two different numbers used to find entries in the table
storage:  hash1 is used to find an initial position in the table to
begin searching for an entry; hash2 is then used to repeatedly offset
that position (mod the size of the table) to build a chain of positions
to search.

In a table with capacity 2^c entries, hash1 is simply the upper c bits
of hash0.  This patch does not change this.

Prior to this patch, hash2 was the c bits below hash1, padded at the low
end with zeroes when c > 16.  (Note that bug 927705, changeset
1a02bec165e16f370cace3da21bb2b377a0a7242, increased the maximum capacity
from 2^23 to 2^26 since 2^23 was sometimes insufficient!)  This manner
of computing hash2 is problematic because it increases the risk of long
chains for very large tables, since there is less variation in the hash2
result due to the zero padding.

So this patch changes the hash2 computation by using the low bits of
hash0 instead of shifting it around, thus avoiding 0 bits in parts of
the hash2 value that are significant.

Note that this changes what hash2 is in all cases except when the table
capacity is exactly 2^16, so it does change our hashing characteristics.
For tables with capacity less than 2^16, it should be using a different
second hash, but with the same amount of random-ish data.  For tables
with capacity greater than 2^16, it should be using more random-ish
data.

Note that this patch depends on the patch for bug 1353458 in order to
avoid causing test failures.

MozReview-Commit-ID: JvnxAMBY711

--HG--
extra : rebase_source : dbde93b9312dd10fb3a549ee4098b57e1df5cadf
2017-05-04 15:17:50 -07:00
L. David Baron
443904b36a Bug 1352888 - Don't set the collision flag when adding to PLDHashTable if we've already found the entry we're going to add. r=njn
PLDHashTable's entry store has two types of unoccupied entries:  free
entries and removed entries.  The search of a chain of entries
(determined by the hash value) in the entry store to search for an entry
can stop at free entries, but it continues across removed entries,
because removed entries are entries that may have been skipped over when
we were adding the value we're searching for to the hash, but have since
been removed.  For live entries, we also maintain this distinction by
using one bit of storage for a collision flag, which notes that if the
hashtable entry is removed, its place in the entry store must become a
removed entry rather than a free entry.

When we add a new entry to the table, Add's semantics require that we
return an existing entry if there is one, and only create a new entry if
no existing entry exists.  (Bug 1352198 suggests the possibility of a
faster alternative Add API where the caller guarantees that the key is
not already in the hashtable.)  When we search for the existing entry,
we must thus continue the search across removed entries, even though we
record the first removed entry found to return if the search for an
existing entry fails.

The existing code adds the collision flag through the entire table
search during an Add.  This patch changes that behavior so that we only
add the collision flag prior to finding the first removed entry.  Adding
it after we find the first removed entry is unnecessary, since we are
not making that entry part of a path to a new entry.  If it is part of a
path to an existing entry, it will already have the collision flag set.

This patch effectively puts an if (!firstRemoved) around the else branch
of the if (MOZ_UNLIKELY(EntryIsRemoved(entry))), and then refactors that
condition outwards since it is now around the contents of both the if
and else branches.

MozReview-Commit-ID: CsXnMYttHVy

--HG--
extra : transplant_source : W4%B8%BA%D5p%102%1B%8D%83%23%E0s%B3%B0f%0D%05%AE
2017-04-04 20:59:21 -07:00
Carsten "Tomcat" Book
92b960417c Backed out changeset 8f8e8cd713ad (bug 1352888) 2017-04-04 09:54:55 +02:00
Carsten "Tomcat" Book
6fa4a7de2c Backed out changeset dfdb5742823a (bug 1352889) 2017-04-04 09:54:53 +02:00
L. David Baron
4d700b54f1 Bug 1352889 - Ensure that PLDHashTable's second hash doesn't have padding with 0 bits for tables with capacity larger than 2^16. r=njn
PLDHashTable takes the result of the hash function and multiplies it by
kGoldenRatio to ensure that it has a good distribution of bits across
the 32-bit hash value, and then zeroes out the low bit so that it can be
used for the collision flag.  This result is called hash0.  From hash0
it computes two different numbers used to find entries in the table
storage:  hash1 is used to find an initial position in the table to
begin searching for an entry; hash2 is then used to repeatedly offset
that position (mod the size of the table) to build a chain of positions
to search.

In a table with capacity 2^c entries, hash1 is simply the upper c bits
of hash0.  This patch does not change this.

Prior to this patch, hash2 was the c bits below hash1, padded at the low
end with zeroes when c > 16.  (Note that bug 927705, changeset
1a02bec165e16f370cace3da21bb2b377a0a7242, increased the maximum capacity
from 2^23 to 2^26 since 2^23 was sometimes insufficient!)  This manner
of computing hash2 is problematic because it increases the risk of long
chains for very large tables, since there is less variation in the hash2
result due to the zero padding.

So this patch changes the hash2 computation by using the low bits of
hash0 instead of shifting it around, thus avoiding 0 bits in parts of
the hash2 value that are significant.

Note that this changes what hash2 is in all cases except when the table
capacity is exactly 2^16, so it does change our hashing characteristics.
For tables with capacity less than 2^16, it should be using a different
second hash, but with the same amount of random-ish data.  For tables
with capacity greater than 2^16, it should be using more random-ish
data.

MozReview-Commit-ID: JvnxAMBY711

--HG--
extra : transplant_source : %8A%25%FB%E3H%B8_%F1G%F6%3E%0B%29%DF%20%FF%D8%E1%AEw
2017-04-03 20:43:30 -07:00
L. David Baron
db2f1da78f Bug 1352888 - Don't set the collision flag when adding to PLDHashTable if we've already found the entry we're going to add. r=njn
PLDHashTable's entry store has two types of unoccupied entries:  free
entries and removed entries.  The search of a chain of entries
(determined by the hash value) in the entry store to search for an entry
can stop at free entries, but it continues across removed entries,
because removed entries are entries that may have been skipped over when
we were adding the value we're searching for to the hash, but have since
been removed.  For live entries, we also maintain this distinction by
using one bit of storage for a collision flag, which notes that if the
hashtable entry is removed, its place in the entry store must become a
removed entry rather than a free entry.

When we add a new entry to the table, Add's semantics require that we
return an existing entry if there is one, and only create a new entry if
no existing entry exists.  (Bug 1352198 suggests the possibility of a
faster alternative Add API where the caller guarantees that the key is
not already in the hashtable.)  When we search for the existing entry,
we must thus continue the search across removed entries, even though we
record the first removed entry found to return if the search for an
existing entry fails.

The existing code adds the collision flag through the entire table
search during an Add.  This patch changes that behavior so that we only
add the collision flag prior to finding the first removed entry.  Adding
it after we find the first removed entry is unnecessary, since we are
not making that entry part of a path to a new entry.  If it is part of a
path to an existing entry, it will already have the collision flag set.

This patch effectively puts an if (!firstRemoved) around the else branch
of the if (MOZ_UNLIKELY(EntryIsRemoved(entry))), and then refactors that
condition outwards since it is now around the contents of both the if
and else branches.

MozReview-Commit-ID: CsXnMYttHVy

--HG--
extra : transplant_source : 0T%B0%FA%C0%85v%8B%16%E7%81%03p%F5K%97%B1%9E%92%27
2017-04-03 20:43:29 -07:00
Benjamin Smedberg
2b3617b084 Bug 1332631 part C - file moves from xpcom/glue to xpcom/ds, r=froydnj
MozReview-Commit-ID: 9ZhC5bROWdO

--HG--
rename : xpcom/glue/Observer.h => xpcom/ds/Observer.h
rename : xpcom/glue/PLDHashTable.cpp => xpcom/ds/PLDHashTable.cpp
rename : xpcom/glue/PLDHashTable.h => xpcom/ds/PLDHashTable.h
rename : xpcom/glue/nsArrayEnumerator.cpp => xpcom/ds/nsArrayEnumerator.cpp
rename : xpcom/glue/nsArrayEnumerator.h => xpcom/ds/nsArrayEnumerator.h
rename : xpcom/glue/nsArrayUtils.cpp => xpcom/ds/nsArrayUtils.cpp
rename : xpcom/glue/nsArrayUtils.h => xpcom/ds/nsArrayUtils.h
rename : xpcom/glue/nsBaseHashtable.h => xpcom/ds/nsBaseHashtable.h
rename : xpcom/glue/nsCOMArray.cpp => xpcom/ds/nsCOMArray.cpp
rename : xpcom/glue/nsCOMArray.h => xpcom/ds/nsCOMArray.h
rename : xpcom/glue/nsClassHashtable.h => xpcom/ds/nsClassHashtable.h
rename : xpcom/glue/nsDataHashtable.h => xpcom/ds/nsDataHashtable.h
rename : xpcom/glue/nsDeque.cpp => xpcom/ds/nsDeque.cpp
rename : xpcom/glue/nsDeque.h => xpcom/ds/nsDeque.h
rename : xpcom/glue/nsEnumeratorUtils.cpp => xpcom/ds/nsEnumeratorUtils.cpp
rename : xpcom/glue/nsEnumeratorUtils.h => xpcom/ds/nsEnumeratorUtils.h
rename : xpcom/glue/nsHashKeys.h => xpcom/ds/nsHashKeys.h
rename : xpcom/glue/nsInterfaceHashtable.h => xpcom/ds/nsInterfaceHashtable.h
rename : xpcom/glue/nsJSThingHashtable.h => xpcom/ds/nsJSThingHashtable.h
rename : xpcom/glue/nsPointerHashKeys.h => xpcom/ds/nsPointerHashKeys.h
rename : xpcom/glue/nsQuickSort.cpp => xpcom/ds/nsQuickSort.cpp
rename : xpcom/glue/nsQuickSort.h => xpcom/ds/nsQuickSort.h
rename : xpcom/glue/nsRefPtrHashtable.h => xpcom/ds/nsRefPtrHashtable.h
rename : xpcom/glue/nsTArray-inl.h => xpcom/ds/nsTArray-inl.h
rename : xpcom/glue/nsTArray.cpp => xpcom/ds/nsTArray.cpp
rename : xpcom/glue/nsTArray.h => xpcom/ds/nsTArray.h
rename : xpcom/glue/nsTArrayForwardDeclare.h => xpcom/ds/nsTArrayForwardDeclare.h
rename : xpcom/glue/nsTHashtable.h => xpcom/ds/nsTHashtable.h
rename : xpcom/glue/nsTObserverArray.cpp => xpcom/ds/nsTObserverArray.cpp
rename : xpcom/glue/nsTObserverArray.h => xpcom/ds/nsTObserverArray.h
rename : xpcom/glue/nsTPriorityQueue.h => xpcom/ds/nsTPriorityQueue.h
extra : rebase_source : 5f0638e3268acb932e1a8d3f499d283bc1922acd
extra : histedit_source : dcea9485885877bb02f95a1ecc627465a507f757
2017-01-20 13:59:21 -05:00