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

[Justin Bogner via llvm-commits]

escha <escha at apple.com> writes:
> escha created this revision.
> escha added reviewers: bogner, resistor.
> escha added a subscriber: llvm-commits.
> escha set the repository for this revision to rL LLVM.
>
> 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.

Could you add a unittest for this please? You can probably find
something similar to base it off of in unittests/ADT/DenseMapTest.cpp.

> Repository:
>   rL LLVM
>
> http://reviews.llvm.org/D18154
>
> Files:
>   include/llvm/ADT/DenseMap.h
>
> Index: include/llvm/ADT/DenseMap.h
> ===================================================================
> --- include/llvm/ADT/DenseMap.h
> +++ include/llvm/ADT/DenseMap.h
> @@ -81,8 +81,11 @@
>    }
>    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 = (Size * 4 + 2) / 3;

Why +2? This probably deserves a comment.

>      incrementEpoch();
>      if (Size > getNumBuckets())
>        grow(Size);