[PATCH] D18154: DenseMap: make .resize() do the intuitive thing

[Justin Bogner via llvm-commits]

escha <escha at apple.com> writes:
> escha updated this revision to Diff 50637.
> escha added a comment.
>
> Added a test to make sure that resizing is enough to insert N elements
> without reallocation for a variety of sizes.
>
> I also looked quickly over the uses and none of the uses seem to be
> already-compensating for this.

LGTM

> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D18154
>
> Files:
>   include/llvm/ADT/DenseMap.h
>   unittests/ADT/DenseMapTest.cpp
>
> Index: unittests/ADT/DenseMapTest.cpp
> ===================================================================
> --- unittests/ADT/DenseMapTest.cpp
> +++ unittests/ADT/DenseMapTest.cpp
> @@ -339,6 +339,19 @@
>    EXPECT_TRUE(cit == cit2);
>  }
>  
> +// Make sure resize actually gives us enough buckets to insert N items
> +// without increasing allocation size.
> +TEST(DenseMapCustomTest, ResizeTest) {
> +  for (unsigned Size = 16; Size < 32; ++Size) {
> +    DenseMap<unsigned, unsigned> Map;
> +    Map.resize(Size);
> +    unsigned MemorySize = Map.getMemorySize();
> +    for (unsigned i = 0; i < Size; ++i)
> +      Map[i] = i;
> +    EXPECT_TRUE(Map.getMemorySize() == MemorySize);
> +  }
> +}
> +
>  // Make sure DenseMap works with StringRef keys.
>  TEST(DenseMapCustomTest, StringRefTest) {
>    DenseMap<StringRef, int> M;
> Index: include/llvm/ADT/DenseMap.h
> ===================================================================
> --- include/llvm/ADT/DenseMap.h
> +++ include/llvm/ADT/DenseMap.h
> @@ -81,8 +81,12 @@
>    }
>    unsigned size() const { return getNumEntries(); }
>  
> -  /// Grow the densemap so that it has at least Size buckets. Does not shrink
> +  /// Grow the densemap so that it can contain at least Size items before
> +  /// resizing again. This means somewhat more than Size buckets because
> +  /// densemap resizes upon reaching 3/4 full.
>    void resize(size_type Size) {
> +    // Size *= (4/3), rounding up.
> +    Size = (Size * 4 + 2) / 3;
>      incrementEpoch();
>      if (Size > getNumBuckets())
>        grow(Size);
>