<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi David,<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Mar 14, 2016, at 6:58 PM, David Blaikie via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">I think someone mentioned on IRC, might be worth renaming 'resize' to 'reserve' as well, to better match the naming/behavior of standard containers?</div></div></blockquote><div><br class=""></div><div>r264026.</div><div>Please also look into <a href="http://reviews.llvm.org/D18345" class="">http://reviews.llvm.org/D18345</a></div><div><br class=""></div><div>-- </div><div>Mehdi</div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Mon, Mar 14, 2016 at 6:50 PM, Fiona Glaser via llvm-commits <span dir="ltr" class=""><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: escha<br class="">
Date: Mon Mar 14 20:50:46 2016<br class="">
New Revision: 263522<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=263522&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=263522&view=rev</a><br class="">
Log:<br class="">
DenseMap: make .resize() do the intuitive thing<br class="">
<br class="">
In some places, like InstCombine, we resize a DenseMap to fit the elements<br class="">
we intend to put in it, then insert those elements (to avoid continual<br class="">
reallocations as it grows). But .resize(foo) doesn't actually do what<br class="">
people think; it resizes to foo buckets (which is really an<br class="">
implementation detail the user of DenseMap probably shouldn't care about),<br class="">
not the space required to fit foo elements. DenseMap grows if 3/4 of its<br class="">
buckets are full, so this actually causes one forced reallocation every<br class="">
time instead of avoiding a reallocation.<br class="">
<br class="">
This patch makes .resize(foo) do the intuitive thing: it grows to the size<br class="">
necessary to fit foo elements without new allocations.<br class="">
<br class="">
Also include a test to verify that .resize() actually does what we think it<br class="">
does.<br class="">
<br class="">
Modified:<br class="">
    llvm/trunk/include/llvm/ADT/DenseMap.h<br class="">
    llvm/trunk/unittests/ADT/DenseMapTest.cpp<br class="">
<br class="">
Modified: llvm/trunk/include/llvm/ADT/DenseMap.h<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/DenseMap.h?rev=263522&r1=263521&r2=263522&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/DenseMap.h?rev=263522&r1=263521&r2=263522&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/include/llvm/ADT/DenseMap.h (original)<br class="">
+++ llvm/trunk/include/llvm/ADT/DenseMap.h Mon Mar 14 20:50:46 2016<br class="">
@@ -81,8 +81,12 @@ public:<br class="">
   }<br class="">
   unsigned size() const { return getNumEntries(); }<br class="">
<br class="">
-  /// Grow the densemap so that it has at least Size buckets. Does not shrink<br class="">
+  /// Grow the densemap so that it can contain at least Size items before<br class="">
+  /// resizing again. This means somewhat more than Size buckets because<br class="">
+  /// densemap resizes upon reaching 3/4 full.<br class="">
   void resize(size_type Size) {<br class="">
+    // Size *= (4/3), rounding up.<br class="">
+    Size = (Size * 4 + 2) / 3;<br class="">
     incrementEpoch();<br class="">
     if (Size > getNumBuckets())<br class="">
       grow(Size);<br class="">
<br class="">
Modified: llvm/trunk/unittests/ADT/DenseMapTest.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/DenseMapTest.cpp?rev=263522&r1=263521&r2=263522&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/DenseMapTest.cpp?rev=263522&r1=263521&r2=263522&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/unittests/ADT/DenseMapTest.cpp (original)<br class="">
+++ llvm/trunk/unittests/ADT/DenseMapTest.cpp Mon Mar 14 20:50:46 2016<br class="">
@@ -339,6 +339,19 @@ TYPED_TEST(DenseMapTest, ConstIteratorTe<br class="">
   EXPECT_TRUE(cit == cit2);<br class="">
 }<br class="">
<br class="">
+// Make sure resize actually gives us enough buckets to insert N items<br class="">
+// without increasing allocation size.<br class="">
+TEST(DenseMapCustomTest, ResizeTest) {<br class="">
+  for (unsigned Size = 16; Size < 32; ++Size) {<br class="">
+    DenseMap<unsigned, unsigned> Map;<br class="">
+    Map.resize(Size);<br class="">
+    unsigned MemorySize = Map.getMemorySize();<br class="">
+    for (unsigned i = 0; i < Size; ++i)<br class="">
+      Map[i] = i;<br class="">
+    EXPECT_TRUE(Map.getMemorySize() == MemorySize);<br class="">
+  }<br class="">
+}<br class="">
+<br class="">
 // Make sure DenseMap works with StringRef keys.<br class="">
 TEST(DenseMapCustomTest, StringRefTest) {<br class="">
   DenseMap<StringRef, int> M;<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="">
</blockquote></div><br class=""></div>
_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></div></blockquote></div><br class=""></div></body></html>