Skip to content

Commit

Permalink
22391: Reduces lock contention for string interning (#322)
Browse files Browse the repository at this point in the history
  • Loading branch information
@howsohazard
howsohazard authored Dec 10, 2024
1 parent ce17738 commit b1bcabf
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 47 deletions.
2 changes: 1 addition & 1 deletion src/Amalgam/PartialSum.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class PartialSumCollection
numMaskBuckets = 1;
}

//resizes the buffer to accomodate the dimensions and instances specified and clears all data
//resizes the buffer to accommodate the dimensions and instances specified and clears all data
void ResizeAndClear(size_t num_dimensions, size_t num_instances)
{
numTerms = num_dimensions;
Expand Down
79 changes: 33 additions & 46 deletions src/Amalgam/string/StringInternPool.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class StringInternPool
return EMPTY_STRING;

#ifdef STRING_INTERN_POOL_VALIDATION
ValidateStringIdExistance(id);
ValidateStringIdExistence(id);
#endif

return id->string;
Expand All @@ -73,7 +73,7 @@ class StringInternPool
inline StringID GetIDFromString(const std::string &str)
{
#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE)
Concurrency::ReadLock lock(sharedMutex);
Concurrency::SingleLock lock(mutex);
#endif

auto id_iter = stringToID.find(str);
Expand All @@ -82,7 +82,7 @@ class StringInternPool

StringID id = id_iter->second.get();
#ifdef STRING_INTERN_POOL_VALIDATION
ValidateStringIdExistanceUnderLock(id);
ValidateStringIdExistenceUnderLock(id);
#endif
return id;
}
Expand All @@ -94,7 +94,7 @@ class StringInternPool
return emptyStringId;

#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE)
Concurrency::WriteLock lock(sharedMutex);
Concurrency::SingleLock lock(mutex);
#endif

//try to insert it as a new string
Expand All @@ -106,7 +106,7 @@ class StringInternPool

StringID id = inserted.first->second.get();
#ifdef STRING_INTERN_POOL_VALIDATION
ValidateStringIdExistanceUnderLock(id);
ValidateStringIdExistenceUnderLock(id);
#endif
return id;
}
Expand All @@ -117,7 +117,7 @@ class StringInternPool
if(id != NOT_A_STRING_ID)
{
#ifdef STRING_INTERN_POOL_VALIDATION
ValidateStringIdExistance(id);
ValidateStringIdExistence(id);
#endif
id->refCount++;
}
Expand All @@ -136,7 +136,7 @@ class StringInternPool
if(id != NOT_A_STRING_ID)
{
#ifdef STRING_INTERN_POOL_VALIDATION
ValidateStringIdExistance(id);
ValidateStringIdExistence(id);
#endif
id->refCount++;
}
Expand All @@ -157,7 +157,7 @@ class StringInternPool
if(id != NOT_A_STRING_ID)
{
#ifdef STRING_INTERN_POOL_VALIDATION
ValidateStringIdExistance(id);
ValidateStringIdExistence(id);
#endif
id->refCount += additional_reference_count;
}
Expand All @@ -177,7 +177,7 @@ class StringInternPool
if(id != NOT_A_STRING_ID)
{
#ifdef STRING_INTERN_POOL_VALIDATION
ValidateStringIdExistance(id);
ValidateStringIdExistence(id);
#endif
id->refCount++;
}
Expand All @@ -190,14 +190,8 @@ class StringInternPool
if(id == NOT_A_STRING_ID || id == emptyStringId)
return;

//get the reference count before decrement
#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE)
//make sure have a read lock first so that the idToStringAndRefCount vector heap location doesn't change
Concurrency::ReadLock lock(sharedMutex);
#endif

#ifdef STRING_INTERN_POOL_VALIDATION
ValidateStringIdExistanceUnderLock(id);
ValidateStringIdExistence(id);
#endif

int64_t refcount = id->refCount--;
Expand All @@ -207,15 +201,13 @@ class StringInternPool
return;

#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE)
//this thread is about to free the reference, but need to acquire a write lock
// so, keep the reference alive by incrementing it *before* attempting the write lock
//this thread is about to free the reference, but need to acquire a lock
// so, keep the reference alive by incrementing it *before* attempting the lock
id->refCount++;

//grab a write lock
lock.unlock();
Concurrency::WriteLock write_lock(sharedMutex);
Concurrency::SingleLock lock(mutex);

//with the write lock, decrement reference count in case this string should stay active
//with the lock, decrement reference count in case this string should stay active
refcount = id->refCount--;

//if other references, then can't clear it
Expand Down Expand Up @@ -244,17 +236,14 @@ class StringInternPool
// removal can be done after reference count decreases are done
bool ids_need_removal = false;

//only need a ReadLock because the count is atomic
Concurrency::ReadLock lock(sharedMutex);

for(auto r : references_container)
{
StringID id = get_string_id(r);
if(id == NOT_A_STRING_ID || id == emptyStringId)
continue;

#ifdef STRING_INTERN_POOL_VALIDATION
ValidateStringIdExistanceUnderLock(id);
ValidateStringIdExistence(id);
#endif

int64_t refcount = id->refCount--;
Expand All @@ -267,17 +256,15 @@ class StringInternPool
if(!ids_need_removal)
return;

//need to remove at least one reference, so put all counts back while wait for write lock
//need to remove at least one reference, so put all counts back while wait for lock
for(auto r : references_container)
{
StringID id = get_string_id(r);
if(id != NOT_A_STRING_ID && id != emptyStringId)
id->refCount++;
}

//grab a write lock
lock.unlock();
Concurrency::WriteLock write_lock(sharedMutex);
Concurrency::SingleLock lock(mutex);

for(auto r : references_container)
{
Expand All @@ -286,7 +273,7 @@ class StringInternPool
continue;

#ifdef STRING_INTERN_POOL_VALIDATION
ValidateStringIdExistanceUnderLock(id);
ValidateStringIdExistenceUnderLock(id);
#endif

//remove any that are the last reference
Expand All @@ -311,7 +298,7 @@ class StringInternPool
inline size_t GetNumStringsInUse()
{
#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE)
Concurrency::ReadLock lock(sharedMutex);
Concurrency::SingleLock lock(mutex);
#endif

return stringToID.size();
Expand All @@ -321,7 +308,7 @@ class StringInternPool
inline size_t GetNumDynamicStringsInUse()
{
#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE)
Concurrency::ReadLock lock(sharedMutex);
Concurrency::SingleLock lock(mutex);
#endif

return stringToID.size() - staticStringIDToIndex.size();
Expand All @@ -331,7 +318,7 @@ class StringInternPool
inline std::vector<std::pair<std::string, int64_t>> GetDynamicStringsInUse()
{
#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE)
Concurrency::ReadLock lock(sharedMutex);
Concurrency::SingleLock lock(mutex);
#endif

std::vector<std::pair<std::string, int64_t>> in_use;
Expand All @@ -346,19 +333,19 @@ class StringInternPool
}

//validates the string id, throwing an assert if it is not valid
inline void ValidateStringIdExistance(StringID sid)
inline void ValidateStringIdExistence(StringID sid)
{
#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE)
Concurrency::ReadLock lock(sharedMutex);
Concurrency::SingleLock lock(mutex);
#endif
ValidateStringIdExistanceUnderLock(sid);
ValidateStringIdExistenceUnderLock(sid);
}

protected:

//validates the string id, throwing an assert if it is not valid
//requires being under a lock
inline void ValidateStringIdExistanceUnderLock(StringID sid)
inline void ValidateStringIdExistenceUnderLock(StringID sid)
{
if(sid == NOT_A_STRING_ID)
return;
Expand All @@ -381,7 +368,7 @@ class StringInternPool
void InitializeStaticStrings();

#if defined(MULTITHREAD_SUPPORT) || defined(MULTITHREAD_INTERFACE)
Concurrency::ReadWriteMutex sharedMutex;
Concurrency::SingleMutex mutex;
#endif

//mapping from string to ID (index of idToRefCountAndString)
Expand Down Expand Up @@ -410,7 +397,7 @@ class StringRef
inline StringRef(StringInternPool::StringID sid)
{
#ifdef STRING_INTERN_POOL_VALIDATION
string_intern_pool.ValidateStringIdExistance(sid);
string_intern_pool.ValidateStringIdExistence(sid);
#endif
id = string_intern_pool.CreateStringReference(sid);
}
Expand All @@ -419,7 +406,7 @@ class StringRef
{
id = string_intern_pool.CreateStringReference(str);
#ifdef STRING_INTERN_POOL_VALIDATION
string_intern_pool.ValidateStringIdExistance(id);
string_intern_pool.ValidateStringIdExistence(id);
#endif
}

Expand All @@ -428,7 +415,7 @@ class StringRef
{
id = string_intern_pool.CreateStringReference(sir.id);
#ifdef STRING_INTERN_POOL_VALIDATION
string_intern_pool.ValidateStringIdExistance(id);
string_intern_pool.ValidateStringIdExistence(id);
#endif
}

Expand All @@ -437,7 +424,7 @@ class StringRef
{
id = sir.id;
#ifdef STRING_INTERN_POOL_VALIDATION
string_intern_pool.ValidateStringIdExistance(id);
string_intern_pool.ValidateStringIdExistence(id);
#endif
sir.id = nullptr;
}
Expand Down Expand Up @@ -465,7 +452,7 @@ class StringRef
string_intern_pool.DestroyStringReference(id);
id = string_intern_pool.CreateStringReference(sir.id);
#ifdef STRING_INTERN_POOL_VALIDATION
string_intern_pool.ValidateStringIdExistance(id);
string_intern_pool.ValidateStringIdExistence(id);
#endif
}
return *this;
Expand All @@ -487,7 +474,7 @@ class StringRef
inline void SetIDAndCreateReference(StringInternPool::StringID sid)
{
#ifdef STRING_INTERN_POOL_VALIDATION
string_intern_pool.ValidateStringIdExistance(sid);
string_intern_pool.ValidateStringIdExistence(sid);
#endif

//if changing id, need to delete previous
Expand All @@ -505,7 +492,7 @@ class StringRef
inline void SetIDWithReferenceHandoff(StringInternPool::StringID sid)
{
#ifdef STRING_INTERN_POOL_VALIDATION
string_intern_pool.ValidateStringIdExistance(sid);
string_intern_pool.ValidateStringIdExistence(sid);
#endif

//if the ids are different, then need to delete old
Expand Down

0 comments on commit b1bcabf

Please sign in to comment.