Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

This patch consolidates the grow() logic in DenseMapBase::grow.

With this patch, DenseMapBase::grow() creates a temporary grown
instance and then lets DenseMap/SmallDenseMap attempt to move the
instance back to *this. If it doesn't work, we move again.

The "attempt to move" always succeeds for DenseMap. For
SmallDenseMap, it succeeds only in the large mode.

This is part of the effort outlined in #168255.

This patch consolidates the grow() logic in DenseMapBase::grow.

With this patch, DenseMapBase::grow() creates a temporary grown
instance and then lets DenseMap/SmallDenseMap attempt to move the
instance back to *this.  If it doesn't work, we move again.

The "attempt to move" always succeeds for DenseMap.  For
SmallDenseMap, it succeeds only in the large mode.

This is part of the effort outlined in llvm#168255.
@llvmbot
Copy link
Member

llvmbot commented Nov 17, 2025

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

This patch consolidates the grow() logic in DenseMapBase::grow.

With this patch, DenseMapBase::grow() creates a temporary grown
instance and then lets DenseMap/SmallDenseMap attempt to move the
instance back to *this. If it doesn't work, we move again.

The "attempt to move" always succeeds for DenseMap. For
SmallDenseMap, it succeeds only in the large mode.

This is part of the effort outlined in #168255.


Full diff: https://github.com/llvm/llvm-project/pull/168316.diff

1 Files Affected:

  • (modified) llvm/include/llvm/ADT/DenseMap.h (+18-21)
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index 60448927248d7..61f29a482391b 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -558,7 +558,12 @@ class DenseMapBase : public DebugEpochBase {
 
   void grow(unsigned MinNumBuckets) {
     unsigned NumBuckets = DerivedT::roundUpNumBuckets(MinNumBuckets);
-    derived().grow(NumBuckets);
+    DerivedT Tmp(NumBuckets, ExactBucketCount{});
+    Tmp.moveFrom(derived());
+    if (derived().maybeMoveFast(std::move(Tmp)))
+      return;
+    initWithExactBucketCount(NumBuckets);
+    moveFrom(Tmp);
   }
 
   template <typename LookupKeyT>
@@ -848,10 +853,9 @@ class DenseMap : public DenseMapBase<DenseMap<KeyT, ValueT, KeyInfoT, BucketT>,
                     static_cast<unsigned>(NextPowerOf2(MinNumBuckets - 1)));
   }
 
-  void grow(unsigned AtLeast) {
-    DenseMap Tmp(AtLeast, typename BaseT::ExactBucketCount{});
-    Tmp.moveFrom(*this);
-    swapImpl(Tmp);
+  bool maybeMoveFast(DenseMap &&Other) {
+    swapImpl(Other);
+    return true;
   }
 
   // Plan how to shrink the bucket table.  Return:
@@ -1120,23 +1124,16 @@ class SmallDenseMap
                     static_cast<unsigned>(NextPowerOf2(MinNumBuckets - 1)));
   }
 
-  void grow(unsigned NumBuckets) {
-    SmallDenseMap Tmp(NumBuckets, typename BaseT::ExactBucketCount{});
-    Tmp.moveFrom(*this);
+  bool maybeMoveFast(SmallDenseMap &&Other) {
+    if (Other.Small)
+      return false;
 
-    if (Tmp.Small) {
-      // Use moveFrom in those rare cases where we stay in the small mode.  This
-      // can happen when we have many tombstones.
-      Small = true;
-      this->BaseT::initEmpty();
-      this->moveFrom(Tmp);
-    } else {
-      Small = false;
-      NumEntries = Tmp.NumEntries;
-      NumTombstones = Tmp.NumTombstones;
-      *getLargeRep() = std::move(*Tmp.getLargeRep());
-      Tmp.getLargeRep()->NumBuckets = 0;
-    }
+    Small = false;
+    NumEntries = Other.NumEntries;
+    NumTombstones = Other.NumTombstones;
+    *getLargeRep() = std::move(*Other.getLargeRep());
+    Other.getLargeRep()->NumBuckets = 0;
+    return true;
   }
 
   // Plan how to shrink the bucket table.  Return:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants