[llvm] r271669 - Adding reserve and capacity methods to FoldingSet
Mehdi Amini via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 3 10:15:40 PDT 2016
> On Jun 3, 2016, at 8:30 AM, David Blaikie via llvm-commits <llvm-commits at lists.llvm.org> 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)
>
You may be able to mimic "struct CountCopyAndMove" and the associated tests in unittests/ADT/DenseMapTest.cpp
> 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 <http://llvm.org/viewvc/llvm-project?rev=271669&view=rev>
> Log:
> Adding reserve and capacity methods to FoldingSet
>
> http://reviews.llvm.org/D20930 <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 <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);
Doxygen on public API
> + unsigned capacity() {
Doxygen on public APIs
> + // 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 <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));
> +}
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?
--
Mehdi
> +
> /// 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 <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 <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160603/1becd4f9/attachment.html>
More information about the llvm-commits
mailing list