[llvm] r271669 - Adding reserve and capacity methods to FoldingSet
Craig, Ben via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 3 08:55:09 PDT 2016
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 <mailto: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 <mailto: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/0f9204de/attachment.html>
More information about the llvm-commits
mailing list