Skip to content

[DenseMap] Optimize find/erase #100517

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 25, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 49 additions & 31 deletions llvm/include/llvm/ADT/DenseMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@ class DenseMapBase : public DebugEpochBase {

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

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

iterator find(const_arg_type_t<KeyT> Val) {
BucketT *TheBucket;
if (LookupBucketFor(Val, TheBucket))
return makeIterator(TheBucket,
shouldReverseIterate<KeyT>() ? getBuckets()
: getBucketsEnd(),
*this, true);
if (BucketT *Bucket = doFind(Val))
return makeIterator(
Bucket, shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd(),
*this, true);
return end();
}
const_iterator find(const_arg_type_t<KeyT> Val) const {
const BucketT *TheBucket;
if (LookupBucketFor(Val, TheBucket))
return makeConstIterator(TheBucket,
shouldReverseIterate<KeyT>() ? getBuckets()
: getBucketsEnd(),
*this, true);
if (const BucketT *Bucket = doFind(Val))
return makeConstIterator(
Bucket, shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd(),
*this, true);
return end();
}

Expand All @@ -178,31 +173,26 @@ class DenseMapBase : public DebugEpochBase {
/// type used.
template<class LookupKeyT>
iterator find_as(const LookupKeyT &Val) {
BucketT *TheBucket;
if (LookupBucketFor(Val, TheBucket))
return makeIterator(TheBucket,
shouldReverseIterate<KeyT>() ? getBuckets()
: getBucketsEnd(),
*this, true);
if (BucketT *Bucket = doFind(Val))
return makeIterator(
Bucket, shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd(),
*this, true);
return end();
}
template<class LookupKeyT>
const_iterator find_as(const LookupKeyT &Val) const {
const BucketT *TheBucket;
if (LookupBucketFor(Val, TheBucket))
return makeConstIterator(TheBucket,
shouldReverseIterate<KeyT>() ? getBuckets()
: getBucketsEnd(),
*this, true);
if (const BucketT *Bucket = doFind(Val))
return makeConstIterator(
Bucket, shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd(),
*this, true);
return end();
}

/// lookup - Return the entry for the specified key, or a default
/// constructed value if no such entry exists.
ValueT lookup(const_arg_type_t<KeyT> Val) const {
const BucketT *TheBucket;
if (LookupBucketFor(Val, TheBucket))
return TheBucket->getSecond();
if (const BucketT *Bucket = doFind(Val))
return Bucket->getSecond();
return ValueT();
}

Expand Down Expand Up @@ -343,8 +333,8 @@ class DenseMapBase : public DebugEpochBase {
}

bool erase(const KeyT &Val) {
BucketT *TheBucket;
if (!LookupBucketFor(Val, TheBucket))
BucketT *TheBucket = doFind(Val);
if (!TheBucket)
return false; // not in map.

TheBucket->getSecond().~ValueT();
Expand Down Expand Up @@ -643,6 +633,34 @@ class DenseMapBase : public DebugEpochBase {
return TheBucket;
}

template <typename LookupKeyT> BucketT *doFind(const LookupKeyT &Val) {
BucketT *BucketsPtr = getBuckets();
const unsigned NumBuckets = getNumBuckets();
if (NumBuckets == 0)
return nullptr;

const KeyT EmptyKey = getEmptyKey();
unsigned BucketNo = getHashValue(Val) & (NumBuckets - 1);
unsigned ProbeAmt = 1;
while (true) {
BucketT *Bucket = BucketsPtr + BucketNo;
if (LLVM_LIKELY(KeyInfoT::isEqual(Val, Bucket->getFirst())))
return Bucket;
if (LLVM_LIKELY(KeyInfoT::isEqual(Bucket->getFirst(), EmptyKey)))
return nullptr;

// Otherwise, it's a hash collision or a tombstone, continue quadratic
// probing.
BucketNo += ProbeAmt++;
BucketNo &= NumBuckets - 1;
}
}

template <typename LookupKeyT>
const BucketT *doFind(const LookupKeyT &Val) const {
return const_cast<DenseMapBase *>(this)->doFind(Val); // NOLINT
}

/// LookupBucketFor - Lookup the appropriate bucket for Val, returning it in
/// FoundBucket. If the bucket contains the key and a value, this returns
/// true, otherwise it returns a bucket with an empty marker or tombstone and
Expand Down
Loading