[llvm] r263522 - DenseMap: make .resize() do the intuitive thing

[David Blaikie via llvm-commits]

I think someone mentioned on IRC, might be worth renaming 'resize' to
'reserve' as well, to better match the naming/behavior of standard
containers?

On Mon, Mar 14, 2016 at 6:50 PM, Fiona Glaser via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: escha
> Date: Mon Mar 14 20:50:46 2016
> New Revision: 263522
>
> URL: http://llvm.org/viewvc/llvm-project?rev=263522&view=rev
> Log:
> DenseMap: make .resize() do the intuitive thing
>
> In some places, like InstCombine, we resize a DenseMap to fit the elements
> we intend to put in it, then insert those elements (to avoid continual
> reallocations as it grows). But .resize(foo) doesn't actually do what
> people think; it resizes to foo buckets (which is really an
> implementation detail the user of DenseMap probably shouldn't care about),
> not the space required to fit foo elements. DenseMap grows if 3/4 of its
> buckets are full, so this actually causes one forced reallocation every
> time instead of avoiding a reallocation.
>
> This patch makes .resize(foo) do the intuitive thing: it grows to the size
> necessary to fit foo elements without new allocations.
>
> Also include a test to verify that .resize() actually does what we think it
> does.
>
> Modified:
>     llvm/trunk/include/llvm/ADT/DenseMap.h
>     llvm/trunk/unittests/ADT/DenseMapTest.cpp
>
> Modified: llvm/trunk/include/llvm/ADT/DenseMap.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/DenseMap.h?rev=263522&r1=263521&r2=263522&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/DenseMap.h (original)
> +++ llvm/trunk/include/llvm/ADT/DenseMap.h Mon Mar 14 20:50:46 2016
> @@ -81,8 +81,12 @@ public:
>    }
>    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);
>
> Modified: llvm/trunk/unittests/ADT/DenseMapTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/DenseMapTest.cpp?rev=263522&r1=263521&r2=263522&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/ADT/DenseMapTest.cpp (original)
> +++ llvm/trunk/unittests/ADT/DenseMapTest.cpp Mon Mar 14 20:50:46 2016
> @@ -339,6 +339,19 @@ TYPED_TEST(DenseMapTest, ConstIteratorTe
>    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;
>
>
> _______________________________________________
> 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/f0992817/attachment.html>