Skip to content

Commit c9e5af3

Browse files
authored
[DenseMap] Optimize find/erase
`LookupBucketFor` is used for `find`, `insert`, `erase`, and their variants. While tombstone comparison isn't needed by `find`/`erase`, `LookupBucketFor` calls `getTombstoneKey` regardless, which might have an opaque implementation or just not optimized out, leading to unnecessary overhead for `find` and `erase`. Pull Request: #100517
1 parent 4aa4ee9 commit c9e5af3

File tree

1 file changed

+49
-31
lines changed

1 file changed

+49
-31
lines changed

llvm/include/llvm/ADT/DenseMap.h

Lines changed: 49 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,7 @@ class DenseMapBase : public DebugEpochBase {
143143

144144
/// Return true if the specified key is in the map, false otherwise.
145145
bool contains(const_arg_type_t<KeyT> Val) const {
146-
const BucketT *TheBucket;
147-
return LookupBucketFor(Val, TheBucket);
146+
return doFind(Val) != nullptr;
148147
}
149148

150149
/// Return 1 if the specified key is in the map, 0 otherwise.
@@ -153,21 +152,17 @@ class DenseMapBase : public DebugEpochBase {
153152
}
154153

155154
iterator find(const_arg_type_t<KeyT> Val) {
156-
BucketT *TheBucket;
157-
if (LookupBucketFor(Val, TheBucket))
158-
return makeIterator(TheBucket,
159-
shouldReverseIterate<KeyT>() ? getBuckets()
160-
: getBucketsEnd(),
161-
*this, true);
155+
if (BucketT *Bucket = doFind(Val))
156+
return makeIterator(
157+
Bucket, shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd(),
158+
*this, true);
162159
return end();
163160
}
164161
const_iterator find(const_arg_type_t<KeyT> Val) const {
165-
const BucketT *TheBucket;
166-
if (LookupBucketFor(Val, TheBucket))
167-
return makeConstIterator(TheBucket,
168-
shouldReverseIterate<KeyT>() ? getBuckets()
169-
: getBucketsEnd(),
170-
*this, true);
162+
if (const BucketT *Bucket = doFind(Val))
163+
return makeConstIterator(
164+
Bucket, shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd(),
165+
*this, true);
171166
return end();
172167
}
173168

@@ -178,31 +173,26 @@ class DenseMapBase : public DebugEpochBase {
178173
/// type used.
179174
template<class LookupKeyT>
180175
iterator find_as(const LookupKeyT &Val) {
181-
BucketT *TheBucket;
182-
if (LookupBucketFor(Val, TheBucket))
183-
return makeIterator(TheBucket,
184-
shouldReverseIterate<KeyT>() ? getBuckets()
185-
: getBucketsEnd(),
186-
*this, true);
176+
if (BucketT *Bucket = doFind(Val))
177+
return makeIterator(
178+
Bucket, shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd(),
179+
*this, true);
187180
return end();
188181
}
189182
template<class LookupKeyT>
190183
const_iterator find_as(const LookupKeyT &Val) const {
191-
const BucketT *TheBucket;
192-
if (LookupBucketFor(Val, TheBucket))
193-
return makeConstIterator(TheBucket,
194-
shouldReverseIterate<KeyT>() ? getBuckets()
195-
: getBucketsEnd(),
196-
*this, true);
184+
if (const BucketT *Bucket = doFind(Val))
185+
return makeConstIterator(
186+
Bucket, shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd(),
187+
*this, true);
197188
return end();
198189
}
199190

200191
/// lookup - Return the entry for the specified key, or a default
201192
/// constructed value if no such entry exists.
202193
ValueT lookup(const_arg_type_t<KeyT> Val) const {
203-
const BucketT *TheBucket;
204-
if (LookupBucketFor(Val, TheBucket))
205-
return TheBucket->getSecond();
194+
if (const BucketT *Bucket = doFind(Val))
195+
return Bucket->getSecond();
206196
return ValueT();
207197
}
208198

@@ -343,8 +333,8 @@ class DenseMapBase : public DebugEpochBase {
343333
}
344334

345335
bool erase(const KeyT &Val) {
346-
BucketT *TheBucket;
347-
if (!LookupBucketFor(Val, TheBucket))
336+
BucketT *TheBucket = doFind(Val);
337+
if (!TheBucket)
348338
return false; // not in map.
349339

350340
TheBucket->getSecond().~ValueT();
@@ -643,6 +633,34 @@ class DenseMapBase : public DebugEpochBase {
643633
return TheBucket;
644634
}
645635

636+
template <typename LookupKeyT> BucketT *doFind(const LookupKeyT &Val) {
637+
BucketT *BucketsPtr = getBuckets();
638+
const unsigned NumBuckets = getNumBuckets();
639+
if (NumBuckets == 0)
640+
return nullptr;
641+
642+
const KeyT EmptyKey = getEmptyKey();
643+
unsigned BucketNo = getHashValue(Val) & (NumBuckets - 1);
644+
unsigned ProbeAmt = 1;
645+
while (true) {
646+
BucketT *Bucket = BucketsPtr + BucketNo;
647+
if (LLVM_LIKELY(KeyInfoT::isEqual(Val, Bucket->getFirst())))
648+
return Bucket;
649+
if (LLVM_LIKELY(KeyInfoT::isEqual(Bucket->getFirst(), EmptyKey)))
650+
return nullptr;
651+
652+
// Otherwise, it's a hash collision or a tombstone, continue quadratic
653+
// probing.
654+
BucketNo += ProbeAmt++;
655+
BucketNo &= NumBuckets - 1;
656+
}
657+
}
658+
659+
template <typename LookupKeyT>
660+
const BucketT *doFind(const LookupKeyT &Val) const {
661+
return const_cast<DenseMapBase *>(this)->doFind(Val); // NOLINT
662+
}
663+
646664
/// LookupBucketFor - Lookup the appropriate bucket for Val, returning it in
647665
/// FoundBucket. If the bucket contains the key and a value, this returns
648666
/// true, otherwise it returns a bucket with an empty marker or tombstone and

0 commit comments

Comments
 (0)