[llvm] r271669 - Adding reserve and capacity methods to FoldingSet

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 3 08:30:58 PDT 2016


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> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160603/a7629ebf/attachment.html>


More information about the llvm-commits mailing list