diff options
author | Fiona Glaser <escha@apple.com> | 2016-03-15 01:50:46 +0000 |
---|---|---|
committer | Fiona Glaser <escha@apple.com> | 2016-03-15 01:50:46 +0000 |
commit | a4b1ace4610fb6ec2a1d06b3b978285f3dafbb20 (patch) | |
tree | 77afa28718fcd15c79abaebc900de79aacd22119 | |
parent | 1b640e05ba57e805ed00831771b75cd2e617f656 (diff) | |
download | bcm5719-llvm-a4b1ace4610fb6ec2a1d06b3b978285f3dafbb20.tar.gz bcm5719-llvm-a4b1ace4610fb6ec2a1d06b3b978285f3dafbb20.zip |
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.
llvm-svn: 263522
-rw-r--r-- | llvm/include/llvm/ADT/DenseMap.h | 6 | ||||
-rw-r--r-- | llvm/unittests/ADT/DenseMapTest.cpp | 13 |
2 files changed, 18 insertions, 1 deletions
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h index 4cf0d6d328b..4dee41f1729 100644 --- a/llvm/include/llvm/ADT/DenseMap.h +++ b/llvm/include/llvm/ADT/DenseMap.h @@ -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); diff --git a/llvm/unittests/ADT/DenseMapTest.cpp b/llvm/unittests/ADT/DenseMapTest.cpp index f3dcf95e92f..0fda2c62332 100644 --- a/llvm/unittests/ADT/DenseMapTest.cpp +++ b/llvm/unittests/ADT/DenseMapTest.cpp @@ -339,6 +339,19 @@ TYPED_TEST(DenseMapTest, ConstIteratorTest) { 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; |