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

[Fiona Glaser via llvm-commits]

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;