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

[David Blaikie via llvm-commits]

On Mon, Mar 14, 2016 at 1:31 PM, escha via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> 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.
>
>
> 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);
>

This is probably sufficient (just checking that the size didn't change) -
but in case it's of interest, I was thinking of a unit test with a
move-or-copy-counting object in the key or value of the map, then checking
no extra moves-or-copies occur.


> +  }
> +}
> +
>  // 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);
>
>
>
> _______________________________________________
> 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/20160314/062a93af/attachment.html>