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

[Daniel Berlin via llvm-commits]

and can you check the existing uses?


On Mon, Mar 14, 2016 at 12:55 PM, Justin Bogner via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> 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);
> _______________________________________________
> 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/e5f3ce39/attachment.html>