<div dir="ltr">ah, sure - no worries then. Thanks for the explanation!</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 3, 2016 at 8:55 AM, Craig, Ben <span dir="ltr"><<a href="mailto:ben.craig@codeaurora.org" target="_blank">ben.craig@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<p>The elements are never moved or copied, as FoldingSet doesn't own
the elements (I think intrusive_hash_set would have been a better
name). The elements are part of a singly linked list, with the
FoldingSet's buckets as the heads of those lists.</p>
<p>So the publicly visible way to write the test that you describe
would be to force a hash-table collision, but not a full
hash-value collision. The last element that collided would have a
non-null NextInFoldingSetBucket member. When you rehash /
reserve, if the unmasked parts of the hash-value start to differ,
then you would get a null NextInFoldingSetBucket member after the
rehash.</p>
<p>Making a partial hash-value collision seems like a lot of
trouble. It would be easier if the hashing algorithm could be
chosen, but most of that is baked into the implementation of
FoldingSetNodeID.<br>
</p><div><div class="h5">
<div>On 6/3/2016 10:30 AM, David Blaikie
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Would it be worth adding a test that tests the
functionality of reserve? (start with a default constructed
container, add some number of elements and demonstrate that at
some point a reallocation occurs and elements are moved (I
assume they're moved, right? like move-semantic moved, or
copied, not just buffers passed around) and then do the same
thing but with a reserve call before adding any elements and
demonstrate that elements are not moved around)</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Fri, Jun 3, 2016 at 6:54 AM, Ben
Craig via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank"></a><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author:
bcraig<br>
Date: Fri Jun 3 08:54:48 2016<br>
New Revision: 271669<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=271669&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=271669&view=rev</a><br>
Log:<br>
Adding reserve and capacity methods to FoldingSet<br>
<br>
<a href="http://reviews.llvm.org/D20930" rel="noreferrer" target="_blank">http://reviews.llvm.org/D20930</a><br>
<br>
Modified:<br>
llvm/trunk/include/llvm/ADT/FoldingSet.h<br>
llvm/trunk/lib/Support/FoldingSet.cpp<br>
llvm/trunk/unittests/ADT/FoldingSet.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/ADT/FoldingSet.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/FoldingSet.h?rev=271669&r1=271668&r2=271669&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/FoldingSet.h?rev=271669&r1=271668&r2=271669&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ADT/FoldingSet.h (original)<br>
+++ llvm/trunk/include/llvm/ADT/FoldingSet.h Fri Jun 3
08:54:48 2016<br>
@@ -180,11 +180,21 @@ public:<br>
/// empty - Returns true if there are no nodes in the
folding set.<br>
bool empty() const { return NumNodes == 0; }<br>
<br>
+ void reserve(unsigned EltCount);<br>
+ unsigned capacity() {<br>
+ // We allow a load factor of up to 2.0,<br>
+ // so that means our capacity is NumBuckets * 2<br>
+ return NumBuckets * 2;<br>
+ }<br>
+<br>
private:<br>
/// GrowHashTable - Double the size of the hash table and
rehash everything.<br>
- ///<br>
void GrowHashTable();<br>
<br>
+ /// GrowBucketCount - resize the hash table and rehash
everything.<br>
+ /// NewBucketCount must be a power of two, and must be
greater than the old<br>
+ /// bucket count.<br>
+ void GrowBucketCount(unsigned NewBucketCount);<br>
protected:<br>
/// GetNodeProfile - Instantiations of the FoldingSet
template implement<br>
/// this function to gather data bits for the given node.<br>
<br>
Modified: llvm/trunk/lib/Support/FoldingSet.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/FoldingSet.cpp?rev=271669&r1=271668&r2=271669&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/FoldingSet.cpp?rev=271669&r1=271668&r2=271669&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Support/FoldingSet.cpp (original)<br>
+++ llvm/trunk/lib/Support/FoldingSet.cpp Fri Jun 3
08:54:48 2016<br>
@@ -266,12 +266,12 @@ void FoldingSetImpl::clear() {<br>
NumNodes = 0;<br>
}<br>
<br>
-/// GrowHashTable - Double the size of the hash table and
rehash everything.<br>
-///<br>
-void FoldingSetImpl::GrowHashTable() {<br>
+void FoldingSetImpl::GrowBucketCount(unsigned
NewBucketCount) {<br>
+ assert((NewBucketCount > NumBuckets) && "Can't
shrink a folding set with GrowBucketCount");<br>
+ assert(isPowerOf2_32(NewBucketCount) && "Bad
bucket count!");<br>
void **OldBuckets = Buckets;<br>
unsigned OldNumBuckets = NumBuckets;<br>
- NumBuckets <<= 1;<br>
+ NumBuckets = NewBucketCount;<br>
<br>
// Clear out new buckets.<br>
Buckets = AllocateBuckets(NumBuckets);<br>
@@ -298,6 +298,21 @@ void FoldingSetImpl::GrowHashTable() {<br>
free(OldBuckets);<br>
}<br>
<br>
+/// GrowHashTable - Double the size of the hash table and
rehash everything.<br>
+///<br>
+void FoldingSetImpl::GrowHashTable() {<br>
+ GrowBucketCount(NumBuckets * 2);<br>
+}<br>
+<br>
+void FoldingSetImpl::reserve(unsigned EltCount) {<br>
+ // This will give us somewhere between EltCount / 2 and<br>
+ // EltCount buckets. This puts us in the load factor<br>
+ // range of 1.0 - 2.0.<br>
+ if(EltCount < capacity())<br>
+ return;<br>
+ GrowBucketCount(PowerOf2Floor(EltCount));<br>
+}<br>
+<br>
/// FindNodeOrInsertPos - Look up the node specified by
ID. If it exists,<br>
/// return it. If not, return the insertion token that
will make insertion<br>
/// faster.<br>
@@ -330,7 +345,7 @@ FoldingSetImpl::Node<br>
void FoldingSetImpl::InsertNode(Node *N, void *InsertPos) {<br>
assert(!N->getNextInBucket());<br>
// Do we need to grow the hashtable?<br>
- if (NumNodes+1 > NumBuckets*2) {<br>
+ if (NumNodes+1 > capacity()) {<br>
GrowHashTable();<br>
FoldingSetNodeID TempID;<br>
InsertPos = GetBucketFor(ComputeNodeHash(N, TempID),
Buckets, NumBuckets);<br>
<br>
Modified: llvm/trunk/unittests/ADT/FoldingSet.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/FoldingSet.cpp?rev=271669&r1=271668&r2=271669&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/FoldingSet.cpp?rev=271669&r1=271668&r2=271669&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ADT/FoldingSet.cpp (original)<br>
+++ llvm/trunk/unittests/ADT/FoldingSet.cpp Fri Jun 3
08:54:48 2016<br>
@@ -35,5 +35,138 @@ TEST(FoldingSetTest, UnalignedStringTest<br>
EXPECT_EQ(a.ComputeHash(), b.ComputeHash());<br>
}<br>
<br>
+struct TrivialPair : public FoldingSetNode {<br>
+ unsigned Key = 0;<br>
+ unsigned Value = 0;<br>
+ TrivialPair(unsigned K, unsigned V) : FoldingSetNode(),
Key(K), Value(V) {}<br>
+<br>
+ void Profile(FoldingSetNodeID &ID) const {<br>
+ ID.AddInteger(Key);<br>
+ ID.AddInteger(Value);<br>
+ }<br>
+};<br>
+<br>
+TEST(FoldingSetTest, IDComparison) {<br>
+ FoldingSet<TrivialPair> Trivial;<br>
+<br>
+ TrivialPair T(99, 42);<br>
+ Trivial.InsertNode(&T);<br>
+<br>
+ void *InsertPos = nullptr;<br>
+ FoldingSetNodeID ID;<br>
+ T.Profile(ID);<br>
+ TrivialPair *N = Trivial.FindNodeOrInsertPos(ID,
InsertPos);<br>
+ EXPECT_EQ(&T, N);<br>
+ EXPECT_EQ(nullptr, InsertPos);<br>
+}<br>
+<br>
+TEST(FoldingSetTest, MissedIDComparison) {<br>
+ FoldingSet<TrivialPair> Trivial;<br>
+<br>
+ TrivialPair S(100, 42);<br>
+ TrivialPair T(99, 42);<br>
+ Trivial.InsertNode(&T);<br>
+<br>
+ void *InsertPos = nullptr;<br>
+ FoldingSetNodeID ID;<br>
+ S.Profile(ID);<br>
+ TrivialPair *N = Trivial.FindNodeOrInsertPos(ID,
InsertPos);<br>
+ EXPECT_EQ(nullptr, N);<br>
+ EXPECT_NE(nullptr, InsertPos);<br>
+}<br>
+<br>
+TEST(FoldingSetTest, RemoveNodeThatIsPresent) {<br>
+ FoldingSet<TrivialPair> Trivial;<br>
+<br>
+ TrivialPair T(99, 42);<br>
+ Trivial.InsertNode(&T);<br>
+ EXPECT_EQ(Trivial.size(), 1U);<br>
+<br>
+ bool WasThere = Trivial.RemoveNode(&T);<br>
+ EXPECT_TRUE(WasThere);<br>
+ EXPECT_EQ(0U, Trivial.size());<br>
+}<br>
+<br>
+TEST(FoldingSetTest, RemoveNodeThatIsAbsent) {<br>
+ FoldingSet<TrivialPair> Trivial;<br>
+<br>
+ TrivialPair T(99, 42);<br>
+ bool WasThere = Trivial.RemoveNode(&T);<br>
+ EXPECT_FALSE(WasThere);<br>
+ EXPECT_EQ(0U, Trivial.size());<br>
+}<br>
+<br>
+TEST(FoldingSetTest, GetOrInsertInserting) {<br>
+ FoldingSet<TrivialPair> Trivial;<br>
+<br>
+ TrivialPair T(99, 42);<br>
+ TrivialPair *N = Trivial.GetOrInsertNode(&T);<br>
+ EXPECT_EQ(&T, N);<br>
+}<br>
+<br>
+TEST(FoldingSetTest, GetOrInsertGetting) {<br>
+ FoldingSet<TrivialPair> Trivial;<br>
+<br>
+ TrivialPair T(99, 42);<br>
+ TrivialPair T2(99, 42);<br>
+ Trivial.InsertNode(&T);<br>
+ TrivialPair *N = Trivial.GetOrInsertNode(&T2);<br>
+ EXPECT_EQ(&T, N);<br>
+}<br>
+<br>
+TEST(FoldingSetTest, InsertAtPos) {<br>
+ FoldingSet<TrivialPair> Trivial;<br>
+<br>
+ void *InsertPos = nullptr;<br>
+ TrivialPair Finder(99, 42);<br>
+ FoldingSetNodeID ID;<br>
+ Finder.Profile(ID);<br>
+ Trivial.FindNodeOrInsertPos(ID, InsertPos);<br>
+<br>
+ TrivialPair T(99, 42);<br>
+ Trivial.InsertNode(&T, InsertPos);<br>
+ EXPECT_EQ(1U, Trivial.size());<br>
+}<br>
+<br>
+TEST(FoldingSetTest, EmptyIsTrue) {<br>
+ FoldingSet<TrivialPair> Trivial;<br>
+ EXPECT_TRUE(Trivial.empty());<br>
+}<br>
+<br>
+TEST(FoldingSetTest, EmptyIsFalse) {<br>
+ FoldingSet<TrivialPair> Trivial;<br>
+ TrivialPair T(99, 42);<br>
+ Trivial.InsertNode(&T);<br>
+ EXPECT_FALSE(Trivial.empty());<br>
+}<br>
+<br>
+TEST(FoldingSetTest, ClearOnEmpty) {<br>
+ FoldingSet<TrivialPair> Trivial;<br>
+ Trivial.clear();<br>
+ EXPECT_TRUE(Trivial.empty());<br>
+}<br>
+<br>
+TEST(FoldingSetTest, ClearOnNonEmpty) {<br>
+ FoldingSet<TrivialPair> Trivial;<br>
+ TrivialPair T(99, 42);<br>
+ Trivial.InsertNode(&T);<br>
+ Trivial.clear();<br>
+ EXPECT_TRUE(Trivial.empty());<br>
+}<br>
+<br>
+TEST(FoldingSetTest, CapacityLargerThanReserve) {<br>
+ FoldingSet<TrivialPair> Trivial;<br>
+ auto OldCapacity = Trivial.capacity();<br>
+ Trivial.reserve(OldCapacity + 1);<br>
+ EXPECT_GE(Trivial.capacity(), OldCapacity + 1);<br>
+}<br>
+<br>
+TEST(FoldingSetTest, SmallReserveChangesNothing) {<br>
+ FoldingSet<TrivialPair> Trivial;<br>
+ auto OldCapacity = Trivial.capacity();<br>
+ Trivial.reserve(OldCapacity - 1);<br>
+ EXPECT_EQ(Trivial.capacity(), OldCapacity);<br>
+}<br>
+<br>
}<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</div></div><span class="HOEnZb"><font color="#888888"><pre cols="72">--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
</pre>
</font></span></div>
</blockquote></div><br></div>