Skip to content

Commit

Permalink
Revert "lldb: Cache string hash during ConstString pool queries/inser…
Browse files Browse the repository at this point in the history
…tions"

Underlying StringMap API for providing a hash has caused some problems
(observed a crash in lld) - so reverting this until I can figure out/fix
what's going on there.

This reverts commit 52ba075.
This reverts commit 2e19760.
  • Loading branch information
dwblaikie committed Dec 14, 2023
1 parent 7537c3c commit 5bc1adf
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 29 deletions.
50 changes: 22 additions & 28 deletions lldb/source/Utility/ConstString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ class Pool {
return 0;
}

StringPoolValueType GetMangledCounterpart(const char *ccstr) {
StringPoolValueType GetMangledCounterpart(const char *ccstr) const {
if (ccstr != nullptr) {
const PoolEntry &pool = selectPool(llvm::StringRef(ccstr));
llvm::sys::SmartScopedReader<false> rlock(pool.m_mutex);
const uint8_t h = hash(llvm::StringRef(ccstr));
llvm::sys::SmartScopedReader<false> rlock(m_string_pools[h].m_mutex);
return GetStringMapEntryFromKeyData(ccstr).getValue();
}
return nullptr;
Expand All @@ -100,20 +100,19 @@ class Pool {

const char *GetConstCStringWithStringRef(llvm::StringRef string_ref) {
if (string_ref.data()) {
const uint32_t string_hash = StringPool::hash(string_ref);
PoolEntry &pool = selectPool(string_hash);
const uint8_t h = hash(string_ref);

{
llvm::sys::SmartScopedReader<false> rlock(pool.m_mutex);
auto it = pool.m_string_map.find(string_ref, string_hash);
if (it != pool.m_string_map.end())
llvm::sys::SmartScopedReader<false> rlock(m_string_pools[h].m_mutex);
auto it = m_string_pools[h].m_string_map.find(string_ref);
if (it != m_string_pools[h].m_string_map.end())
return it->getKeyData();
}

llvm::sys::SmartScopedWriter<false> wlock(pool.m_mutex);
llvm::sys::SmartScopedWriter<false> wlock(m_string_pools[h].m_mutex);
StringPoolEntryType &entry =
*pool.m_string_map
.insert(std::make_pair(string_ref, nullptr), string_hash)
*m_string_pools[h]
.m_string_map.insert(std::make_pair(string_ref, nullptr))
.first;
return entry.getKeyData();
}
Expand All @@ -126,14 +125,12 @@ class Pool {
const char *demangled_ccstr = nullptr;

{
const uint32_t demangled_hash = StringPool::hash(demangled);
PoolEntry &pool = selectPool(demangled_hash);
llvm::sys::SmartScopedWriter<false> wlock(pool.m_mutex);
const uint8_t h = hash(demangled);
llvm::sys::SmartScopedWriter<false> wlock(m_string_pools[h].m_mutex);

// Make or update string pool entry with the mangled counterpart
StringPool &map = pool.m_string_map;
StringPoolEntryType &entry =
*map.try_emplace_with_hash(demangled, demangled_hash).first;
StringPool &map = m_string_pools[h].m_string_map;
StringPoolEntryType &entry = *map.try_emplace(demangled).first;

entry.second = mangled_ccstr;

Expand All @@ -144,8 +141,8 @@ class Pool {
{
// Now assign the demangled const string as the counterpart of the
// mangled const string...
PoolEntry &pool = selectPool(llvm::StringRef(mangled_ccstr));
llvm::sys::SmartScopedWriter<false> wlock(pool.m_mutex);
const uint8_t h = hash(llvm::StringRef(mangled_ccstr));
llvm::sys::SmartScopedWriter<false> wlock(m_string_pools[h].m_mutex);
GetStringMapEntryFromKeyData(mangled_ccstr).setValue(demangled_ccstr);
}

Expand Down Expand Up @@ -174,20 +171,17 @@ class Pool {
}

protected:
uint8_t hash(llvm::StringRef s) const {
uint32_t h = llvm::djbHash(s);
return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
}

struct PoolEntry {
mutable llvm::sys::SmartRWMutex<false> m_mutex;
StringPool m_string_map;
};

std::array<PoolEntry, 256> m_string_pools;

PoolEntry &selectPool(const llvm::StringRef &s) {
return selectPool(StringPool::hash(s));
}

PoolEntry &selectPool(uint32_t h) {
return m_string_pools[((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff];
}
};

// Frameworks and dylibs aren't supposed to have global C++ initializers so we
Expand All @@ -203,7 +197,7 @@ static Pool &StringPool() {
static Pool *g_string_pool = nullptr;

llvm::call_once(g_pool_initialization_flag,
[]() { g_string_pool = new Pool(); });
[]() { g_string_pool = new Pool(); });

return *g_string_pool;
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Support/StringMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ int StringMapImpl::FindKey(StringRef Key, uint32_t FullHashValue) const {
if (NumBuckets == 0)
return -1; // Really empty table?
#ifdef EXPENSIVE_CHECKS
assert(FullHashValue == hash(Key));
assert(FullHashValue == hash(Key);
#endif
if (shouldReverseIterate())
FullHashValue = ~FullHashValue;
Expand Down

0 comments on commit 5bc1adf

Please sign in to comment.