<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jun 3, 2016, at 8:30 AM, David Blaikie via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">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 class=""></div></div></blockquote><div><br class=""></div><div>You may be able to mimic "struct CountCopyAndMove" and the associated tests in unittests/ADT/DenseMapTest.cpp</div><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_extra"><div class="gmail_quote">On Fri, Jun 3, 2016 at 6:54 AM, Ben Craig via llvm-commits <span dir="ltr" class=""><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: bcraig<br class="">
Date: Fri Jun 3 08:54:48 2016<br class="">
New Revision: 271669<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=271669&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=271669&view=rev</a><br class="">
Log:<br class="">
Adding reserve and capacity methods to FoldingSet<br class="">
<br class="">
<a href="http://reviews.llvm.org/D20930" rel="noreferrer" target="_blank" class="">http://reviews.llvm.org/D20930</a><br class="">
<br class="">
Modified:<br class="">
llvm/trunk/include/llvm/ADT/FoldingSet.h<br class="">
llvm/trunk/lib/Support/FoldingSet.cpp<br class="">
llvm/trunk/unittests/ADT/FoldingSet.cpp<br class="">
<br class="">
Modified: llvm/trunk/include/llvm/ADT/FoldingSet.h<br class="">
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" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/FoldingSet.h?rev=271669&r1=271668&r2=271669&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/include/llvm/ADT/FoldingSet.h (original)<br class="">
+++ llvm/trunk/include/llvm/ADT/FoldingSet.h Fri Jun 3 08:54:48 2016<br class="">
@@ -180,11 +180,21 @@ public:<br class="">
/// empty - Returns true if there are no nodes in the folding set.<br class="">
bool empty() const { return NumNodes == 0; }<br class="">
<br class="">
+ void reserve(unsigned EltCount);<br class=""></blockquote></div></div></div></blockquote><div><br class=""></div><div>Doxygen on public API</div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ unsigned capacity() {<br class=""></blockquote></div></div></div></blockquote><div><br class=""></div><div>Doxygen on public APIs</div><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ // We allow a load factor of up to 2.0,<br class="">
+ // so that means our capacity is NumBuckets * 2<br class="">
+ return NumBuckets * 2;<br class="">
+ }<br class="">
+<br class="">
private:<br class="">
/// GrowHashTable - Double the size of the hash table and rehash everything.<br class="">
- ///<br class="">
void GrowHashTable();<br class="">
<br class="">
+ /// GrowBucketCount - resize the hash table and rehash everything.<br class="">
+ /// NewBucketCount must be a power of two, and must be greater than the old<br class="">
+ /// bucket count.<br class="">
+ void GrowBucketCount(unsigned NewBucketCount);<br class="">
protected:<br class="">
/// GetNodeProfile - Instantiations of the FoldingSet template implement<br class="">
/// this function to gather data bits for the given node.<br class="">
<br class="">
Modified: llvm/trunk/lib/Support/FoldingSet.cpp<br class="">
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" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/FoldingSet.cpp?rev=271669&r1=271668&r2=271669&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/lib/Support/FoldingSet.cpp (original)<br class="">
+++ llvm/trunk/lib/Support/FoldingSet.cpp Fri Jun 3 08:54:48 2016<br class="">
@@ -266,12 +266,12 @@ void FoldingSetImpl::clear() {<br class="">
NumNodes = 0;<br class="">
}<br class="">
<br class="">
-/// GrowHashTable - Double the size of the hash table and rehash everything.<br class="">
-///<br class="">
-void FoldingSetImpl::GrowHashTable() {<br class="">
+void FoldingSetImpl::GrowBucketCount(unsigned NewBucketCount) {<br class="">
+ assert((NewBucketCount > NumBuckets) && "Can't shrink a folding set with GrowBucketCount");<br class="">
+ assert(isPowerOf2_32(NewBucketCount) && "Bad bucket count!");<br class="">
void **OldBuckets = Buckets;<br class="">
unsigned OldNumBuckets = NumBuckets;<br class="">
- NumBuckets <<= 1;<br class="">
+ NumBuckets = NewBucketCount;<br class="">
<br class="">
// Clear out new buckets.<br class="">
Buckets = AllocateBuckets(NumBuckets);<br class="">
@@ -298,6 +298,21 @@ void FoldingSetImpl::GrowHashTable() {<br class="">
free(OldBuckets);<br class="">
}<br class="">
<br class="">
+/// GrowHashTable - Double the size of the hash table and rehash everything.<br class="">
+///<br class="">
+void FoldingSetImpl::GrowHashTable() {<br class="">
+ GrowBucketCount(NumBuckets * 2);<br class="">
+}<br class="">
+<br class="">
+void FoldingSetImpl::reserve(unsigned EltCount) {<br class="">
+ // This will give us somewhere between EltCount / 2 and<br class="">
+ // EltCount buckets. This puts us in the load factor<br class="">
+ // range of 1.0 - 2.0.<br class="">
+ if(EltCount < capacity())<br class="">
+ return;<br class="">
+ GrowBucketCount(PowerOf2Floor(EltCount));<br class="">
+}<br class=""></blockquote></div></div></div></blockquote><div><br class=""></div><div>I was surprised that we target a load factor >1, but I'm still unsure about the right use-case for FoldingSet compare to a DenseSet for instance. Is it about consuming less memory? </div><div><br class=""></div><div>-- </div><div>Mehdi</div><div><br class=""></div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br class="">
/// FindNodeOrInsertPos - Look up the node specified by ID. If it exists,<br class="">
/// return it. If not, return the insertion token that will make insertion<br class="">
/// faster.<br class="">
@@ -330,7 +345,7 @@ FoldingSetImpl::Node<br class="">
void FoldingSetImpl::InsertNode(Node *N, void *InsertPos) {<br class="">
assert(!N->getNextInBucket());<br class="">
// Do we need to grow the hashtable?<br class="">
- if (NumNodes+1 > NumBuckets*2) {<br class="">
+ if (NumNodes+1 > capacity()) {<br class="">
GrowHashTable();<br class="">
FoldingSetNodeID TempID;<br class="">
InsertPos = GetBucketFor(ComputeNodeHash(N, TempID), Buckets, NumBuckets);<br class="">
<br class="">
Modified: llvm/trunk/unittests/ADT/FoldingSet.cpp<br class="">
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" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/FoldingSet.cpp?rev=271669&r1=271668&r2=271669&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/unittests/ADT/FoldingSet.cpp (original)<br class="">
+++ llvm/trunk/unittests/ADT/FoldingSet.cpp Fri Jun 3 08:54:48 2016<br class="">
@@ -35,5 +35,138 @@ TEST(FoldingSetTest, UnalignedStringTest<br class="">
EXPECT_EQ(a.ComputeHash(), b.ComputeHash());<br class="">
}<br class="">
<br class="">
+struct TrivialPair : public FoldingSetNode {<br class="">
+ unsigned Key = 0;<br class="">
+ unsigned Value = 0;<br class="">
+ TrivialPair(unsigned K, unsigned V) : FoldingSetNode(), Key(K), Value(V) {}<br class="">
+<br class="">
+ void Profile(FoldingSetNodeID &ID) const {<br class="">
+ ID.AddInteger(Key);<br class="">
+ ID.AddInteger(Value);<br class="">
+ }<br class="">
+};<br class="">
+<br class="">
+TEST(FoldingSetTest, IDComparison) {<br class="">
+ FoldingSet<TrivialPair> Trivial;<br class="">
+<br class="">
+ TrivialPair T(99, 42);<br class="">
+ Trivial.InsertNode(&T);<br class="">
+<br class="">
+ void *InsertPos = nullptr;<br class="">
+ FoldingSetNodeID ID;<br class="">
+ T.Profile(ID);<br class="">
+ TrivialPair *N = Trivial.FindNodeOrInsertPos(ID, InsertPos);<br class="">
+ EXPECT_EQ(&T, N);<br class="">
+ EXPECT_EQ(nullptr, InsertPos);<br class="">
+}<br class="">
+<br class="">
+TEST(FoldingSetTest, MissedIDComparison) {<br class="">
+ FoldingSet<TrivialPair> Trivial;<br class="">
+<br class="">
+ TrivialPair S(100, 42);<br class="">
+ TrivialPair T(99, 42);<br class="">
+ Trivial.InsertNode(&T);<br class="">
+<br class="">
+ void *InsertPos = nullptr;<br class="">
+ FoldingSetNodeID ID;<br class="">
+ S.Profile(ID);<br class="">
+ TrivialPair *N = Trivial.FindNodeOrInsertPos(ID, InsertPos);<br class="">
+ EXPECT_EQ(nullptr, N);<br class="">
+ EXPECT_NE(nullptr, InsertPos);<br class="">
+}<br class="">
+<br class="">
+TEST(FoldingSetTest, RemoveNodeThatIsPresent) {<br class="">
+ FoldingSet<TrivialPair> Trivial;<br class="">
+<br class="">
+ TrivialPair T(99, 42);<br class="">
+ Trivial.InsertNode(&T);<br class="">
+ EXPECT_EQ(Trivial.size(), 1U);<br class="">
+<br class="">
+ bool WasThere = Trivial.RemoveNode(&T);<br class="">
+ EXPECT_TRUE(WasThere);<br class="">
+ EXPECT_EQ(0U, Trivial.size());<br class="">
+}<br class="">
+<br class="">
+TEST(FoldingSetTest, RemoveNodeThatIsAbsent) {<br class="">
+ FoldingSet<TrivialPair> Trivial;<br class="">
+<br class="">
+ TrivialPair T(99, 42);<br class="">
+ bool WasThere = Trivial.RemoveNode(&T);<br class="">
+ EXPECT_FALSE(WasThere);<br class="">
+ EXPECT_EQ(0U, Trivial.size());<br class="">
+}<br class="">
+<br class="">
+TEST(FoldingSetTest, GetOrInsertInserting) {<br class="">
+ FoldingSet<TrivialPair> Trivial;<br class="">
+<br class="">
+ TrivialPair T(99, 42);<br class="">
+ TrivialPair *N = Trivial.GetOrInsertNode(&T);<br class="">
+ EXPECT_EQ(&T, N);<br class="">
+}<br class="">
+<br class="">
+TEST(FoldingSetTest, GetOrInsertGetting) {<br class="">
+ FoldingSet<TrivialPair> Trivial;<br class="">
+<br class="">
+ TrivialPair T(99, 42);<br class="">
+ TrivialPair T2(99, 42);<br class="">
+ Trivial.InsertNode(&T);<br class="">
+ TrivialPair *N = Trivial.GetOrInsertNode(&T2);<br class="">
+ EXPECT_EQ(&T, N);<br class="">
+}<br class="">
+<br class="">
+TEST(FoldingSetTest, InsertAtPos) {<br class="">
+ FoldingSet<TrivialPair> Trivial;<br class="">
+<br class="">
+ void *InsertPos = nullptr;<br class="">
+ TrivialPair Finder(99, 42);<br class="">
+ FoldingSetNodeID ID;<br class="">
+ Finder.Profile(ID);<br class="">
+ Trivial.FindNodeOrInsertPos(ID, InsertPos);<br class="">
+<br class="">
+ TrivialPair T(99, 42);<br class="">
+ Trivial.InsertNode(&T, InsertPos);<br class="">
+ EXPECT_EQ(1U, Trivial.size());<br class="">
+}<br class="">
+<br class="">
+TEST(FoldingSetTest, EmptyIsTrue) {<br class="">
+ FoldingSet<TrivialPair> Trivial;<br class="">
+ EXPECT_TRUE(Trivial.empty());<br class="">
+}<br class="">
+<br class="">
+TEST(FoldingSetTest, EmptyIsFalse) {<br class="">
+ FoldingSet<TrivialPair> Trivial;<br class="">
+ TrivialPair T(99, 42);<br class="">
+ Trivial.InsertNode(&T);<br class="">
+ EXPECT_FALSE(Trivial.empty());<br class="">
+}<br class="">
+<br class="">
+TEST(FoldingSetTest, ClearOnEmpty) {<br class="">
+ FoldingSet<TrivialPair> Trivial;<br class="">
+ Trivial.clear();<br class="">
+ EXPECT_TRUE(Trivial.empty());<br class="">
+}<br class="">
+<br class="">
+TEST(FoldingSetTest, ClearOnNonEmpty) {<br class="">
+ FoldingSet<TrivialPair> Trivial;<br class="">
+ TrivialPair T(99, 42);<br class="">
+ Trivial.InsertNode(&T);<br class="">
+ Trivial.clear();<br class="">
+ EXPECT_TRUE(Trivial.empty());<br class="">
+}<br class="">
+<br class="">
+TEST(FoldingSetTest, CapacityLargerThanReserve) {<br class="">
+ FoldingSet<TrivialPair> Trivial;<br class="">
+ auto OldCapacity = Trivial.capacity();<br class="">
+ Trivial.reserve(OldCapacity + 1);<br class="">
+ EXPECT_GE(Trivial.capacity(), OldCapacity + 1);<br class="">
+}<br class="">
+<br class="">
+TEST(FoldingSetTest, SmallReserveChangesNothing) {<br class="">
+ FoldingSet<TrivialPair> Trivial;<br class="">
+ auto OldCapacity = Trivial.capacity();<br class="">
+ Trivial.reserve(OldCapacity - 1);<br class="">
+ EXPECT_EQ(Trivial.capacity(), OldCapacity);<br class="">
+}<br class="">
+<br class="">
}<br class="">
<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="">
</blockquote></div><br class=""></div>
_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></div></blockquote></div><br class=""></body></html>