ah, sure - no worries then. Thanks for the explanation!
On Fri, Jun 3, 2016 at 8:55 AM, Craig, Ben <ben.craig at codeaurora.org> wrote:
> 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.
>
> 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.
>
> 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.
> On 6/3/2016 10:30 AM, David Blaikie wrote:
>
> 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)
>
> On Fri, Jun 3, 2016 at 6:54 AM, Ben Craig via llvm-commits <
> <llvm-commits at lists.llvm.org>llvm-commits at lists.llvm.org> wrote:
>
>> Author: bcraig
>> Date: Fri Jun 3 08:54:48 2016
>> New Revision: 271669
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=271669&view=rev
>> Log:
>> Adding reserve and capacity methods to FoldingSet
>>
>> http://reviews.llvm.org/D20930
>>
>> Modified:
>> llvm/trunk/include/llvm/ADT/FoldingSet.h
>> llvm/trunk/lib/Support/FoldingSet.cpp
>> llvm/trunk/unittests/ADT/FoldingSet.cpp
>>
>> Modified: llvm/trunk/include/llvm/ADT/FoldingSet.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/FoldingSet.h?rev=271669&r1=271668&r2=271669&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/ADT/FoldingSet.h (original)
>> +++ llvm/trunk/include/llvm/ADT/FoldingSet.h Fri Jun 3 08:54:48 2016
>> @@ -180,11 +180,21 @@ public:
>> /// empty - Returns true if there are no nodes in the folding set.
>> bool empty() const { return NumNodes == 0; }
>>
>> + void reserve(unsigned EltCount);
>> + unsigned capacity() {
>> + // We allow a load factor of up to 2.0,
>> + // so that means our capacity is NumBuckets * 2
>> + return NumBuckets * 2;
>> + }
>> +
>> private:
>> /// GrowHashTable - Double the size of the hash table and rehash
>> everything.
>> - ///
>> void GrowHashTable();
>>
>> + /// GrowBucketCount - resize the hash table and rehash everything.
>> + /// NewBucketCount must be a power of two, and must be greater than
>> the old
>> + /// bucket count.
>> + void GrowBucketCount(unsigned NewBucketCount);
>> protected:
>> /// GetNodeProfile - Instantiations of the FoldingSet template
>> implement
>> /// this function to gather data bits for the given node.
>>
>> Modified: llvm/trunk/lib/Support/FoldingSet.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/FoldingSet.cpp?rev=271669&r1=271668&r2=271669&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Support/FoldingSet.cpp (original)
>> +++ llvm/trunk/lib/Support/FoldingSet.cpp Fri Jun 3 08:54:48 2016
>> @@ -266,12 +266,12 @@ void FoldingSetImpl::clear() {
>> NumNodes = 0;
>> }
>>
>> -/// GrowHashTable - Double the size of the hash table and rehash
>> everything.
>> -///
>> -void FoldingSetImpl::GrowHashTable() {
>> +void FoldingSetImpl::GrowBucketCount(unsigned NewBucketCount) {
>> + assert((NewBucketCount > NumBuckets) && "Can't shrink a folding set
>> with GrowBucketCount");
>> + assert(isPowerOf2_32(NewBucketCount) && "Bad bucket count!");
>> void **OldBuckets = Buckets;
>> unsigned OldNumBuckets = NumBuckets;
>> - NumBuckets <<= 1;
>> + NumBuckets = NewBucketCount;
>>
>> // Clear out new buckets.
>> Buckets = AllocateBuckets(NumBuckets);
>> @@ -298,6 +298,21 @@ void FoldingSetImpl::GrowHashTable() {
>> free(OldBuckets);
>> }
>>
>> +/// GrowHashTable - Double the size of the hash table and rehash
>> everything.
>> +///
>> +void FoldingSetImpl::GrowHashTable() {
>> + GrowBucketCount(NumBuckets * 2);
>> +}
>> +
>> +void FoldingSetImpl::reserve(unsigned EltCount) {
>> + // This will give us somewhere between EltCount / 2 and
>> + // EltCount buckets. This puts us in the load factor
>> + // range of 1.0 - 2.0.
>> + if(EltCount < capacity())
>> + return;
>> + GrowBucketCount(PowerOf2Floor(EltCount));
>> +}
>> +
>> /// FindNodeOrInsertPos - Look up the node specified by ID. If it
>> exists,
>> /// return it. If not, return the insertion token that will make
>> insertion
>> /// faster.
>> @@ -330,7 +345,7 @@ FoldingSetImpl::Node
>> void FoldingSetImpl::InsertNode(Node *N, void *InsertPos) {
>> assert(!N->getNextInBucket());
>> // Do we need to grow the hashtable?
>> - if (NumNodes+1 > NumBuckets*2) {
>> + if (NumNodes+1 > capacity()) {
>> GrowHashTable();
>> FoldingSetNodeID TempID;
>> InsertPos = GetBucketFor(ComputeNodeHash(N, TempID), Buckets,
>> NumBuckets);
>>
>> Modified: llvm/trunk/unittests/ADT/FoldingSet.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/FoldingSet.cpp?rev=271669&r1=271668&r2=271669&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/ADT/FoldingSet.cpp (original)
>> +++ llvm/trunk/unittests/ADT/FoldingSet.cpp Fri Jun 3 08:54:48 2016
>> @@ -35,5 +35,138 @@ TEST(FoldingSetTest, UnalignedStringTest
>> EXPECT_EQ(a.ComputeHash(), b.ComputeHash());
>> }
>>
>> +struct TrivialPair : public FoldingSetNode {
>> + unsigned Key = 0;
>> + unsigned Value = 0;
>> + TrivialPair(unsigned K, unsigned V) : FoldingSetNode(), Key(K),
>> Value(V) {}
>> +
>> + void Profile(FoldingSetNodeID &ID) const {
>> + ID.AddInteger(Key);
>> + ID.AddInteger(Value);
>> + }
>> +};
>> +
>> +TEST(FoldingSetTest, IDComparison) {
>> + FoldingSet<TrivialPair> Trivial;
>> +
>> + TrivialPair T(99, 42);
>> + Trivial.InsertNode(&T);
>> +
>> + void *InsertPos = nullptr;
>> + FoldingSetNodeID ID;
>> + T.Profile(ID);
>> + TrivialPair *N = Trivial.FindNodeOrInsertPos(ID, InsertPos);
>> + EXPECT_EQ(&T, N);
>> + EXPECT_EQ(nullptr, InsertPos);
>> +}
>> +
>> +TEST(FoldingSetTest, MissedIDComparison) {
>> + FoldingSet<TrivialPair> Trivial;
>> +
>> + TrivialPair S(100, 42);
>> + TrivialPair T(99, 42);
>> + Trivial.InsertNode(&T);
>> +
>> + void *InsertPos = nullptr;
>> + FoldingSetNodeID ID;
>> + S.Profile(ID);
>> + TrivialPair *N = Trivial.FindNodeOrInsertPos(ID, InsertPos);
>> + EXPECT_EQ(nullptr, N);
>> + EXPECT_NE(nullptr, InsertPos);
>> +}
>> +
>> +TEST(FoldingSetTest, RemoveNodeThatIsPresent) {
>> + FoldingSet<TrivialPair> Trivial;
>> +
>> + TrivialPair T(99, 42);
>> + Trivial.InsertNode(&T);
>> + EXPECT_EQ(Trivial.size(), 1U);
>> +
>> + bool WasThere = Trivial.RemoveNode(&T);
>> + EXPECT_TRUE(WasThere);
>> + EXPECT_EQ(0U, Trivial.size());
>> +}
>> +
>> +TEST(FoldingSetTest, RemoveNodeThatIsAbsent) {
>> + FoldingSet<TrivialPair> Trivial;
>> +
>> + TrivialPair T(99, 42);
>> + bool WasThere = Trivial.RemoveNode(&T);
>> + EXPECT_FALSE(WasThere);
>> + EXPECT_EQ(0U, Trivial.size());
>> +}
>> +
>> +TEST(FoldingSetTest, GetOrInsertInserting) {
>> + FoldingSet<TrivialPair> Trivial;
>> +
>> + TrivialPair T(99, 42);
>> + TrivialPair *N = Trivial.GetOrInsertNode(&T);
>> + EXPECT_EQ(&T, N);
>> +}
>> +
>> +TEST(FoldingSetTest, GetOrInsertGetting) {
>> + FoldingSet<TrivialPair> Trivial;
>> +
>> + TrivialPair T(99, 42);
>> + TrivialPair T2(99, 42);
>> + Trivial.InsertNode(&T);
>> + TrivialPair *N = Trivial.GetOrInsertNode(&T2);
>> + EXPECT_EQ(&T, N);
>> +}
>> +
>> +TEST(FoldingSetTest, InsertAtPos) {
>> + FoldingSet<TrivialPair> Trivial;
>> +
>> + void *InsertPos = nullptr;
>> + TrivialPair Finder(99, 42);
>> + FoldingSetNodeID ID;
>> + Finder.Profile(ID);
>> + Trivial.FindNodeOrInsertPos(ID, InsertPos);
>> +
>> + TrivialPair T(99, 42);
>> + Trivial.InsertNode(&T, InsertPos);
>> + EXPECT_EQ(1U, Trivial.size());
>> +}
>> +
>> +TEST(FoldingSetTest, EmptyIsTrue) {
>> + FoldingSet<TrivialPair> Trivial;
>> + EXPECT_TRUE(Trivial.empty());
>> +}
>> +
>> +TEST(FoldingSetTest, EmptyIsFalse) {
>> + FoldingSet<TrivialPair> Trivial;
>> + TrivialPair T(99, 42);
>> + Trivial.InsertNode(&T);
>> + EXPECT_FALSE(Trivial.empty());
>> +}
>> +
>> +TEST(FoldingSetTest, ClearOnEmpty) {
>> + FoldingSet<TrivialPair> Trivial;
>> + Trivial.clear();
>> + EXPECT_TRUE(Trivial.empty());
>> +}
>> +
>> +TEST(FoldingSetTest, ClearOnNonEmpty) {
>> + FoldingSet<TrivialPair> Trivial;
>> + TrivialPair T(99, 42);
>> + Trivial.InsertNode(&T);
>> + Trivial.clear();
>> + EXPECT_TRUE(Trivial.empty());
>> +}
>> +
>> +TEST(FoldingSetTest, CapacityLargerThanReserve) {
>> + FoldingSet<TrivialPair> Trivial;
>> + auto OldCapacity = Trivial.capacity();
>> + Trivial.reserve(OldCapacity + 1);
>> + EXPECT_GE(Trivial.capacity(), OldCapacity + 1);
>> +}
>> +
>> +TEST(FoldingSetTest, SmallReserveChangesNothing) {
>> + FoldingSet<TrivialPair> Trivial;
>> + auto OldCapacity = Trivial.capacity();
>> + Trivial.reserve(OldCapacity - 1);
>> + EXPECT_EQ(Trivial.capacity(), OldCapacity);
>> +}
>> +
>> }
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160603/d65d1594/attachment.html>