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

[Mehdi Amini via llvm-commits]

Hi David,

> On Mar 14, 2016, at 6:58 PM, David Blaikie via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> I think someone mentioned on IRC, might be worth renaming 'resize' to 'reserve' as well, to better match the naming/behavior of standard containers?

r264026.
Please also look into http://reviews.llvm.org/D18345 <http://reviews.llvm.org/D18345>

-- 
Mehdi


> 
> On Mon, Mar 14, 2016 at 6:50 PM, Fiona Glaser via llvm-commits <llvm-commits at lists.llvm.org <mailto: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 <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 <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 <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 <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/20160322/56eeb1fd/attachment.html>