i2p/haiku-beta6
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

kernel/condition_variable: Granularize locking.

Browse Source 
Before this commit, *all* ConditionVariable operations (yes, all;
even Wait, Notify, etc.) went through a single spinlock, that also
protected the sConditionVariableHash. This obviously does not scale
so well with core count, to say the least!

With this commit, we add spinlocks to each Variable and Entry.
This makes locking somewhat more complicated (and nuanced; see
inline comment), but the trade-off seems completely worth it:

(compile HaikuDepot in VMware, 2 cores)
before
real 1m20.219s
user 1m5.619s
sys  0m40.724s

after
real 1m12.667s
user 0m57.684s
sys  0m37.251s

The more cores there are, the more of an optimization this will
likely prove to be. But 10%-across-the-board is not bad to say
the least.

Change-Id: I1e40a997fff58a79e987d7cdcafa8f7358e1115a
This commit is contained in:
Augustin Cavalier
2019-08-03 11:24:34 -04:00
parent 489612d43f
commit 37eda488be
2 changed files with 67 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/system/kernel/condition_variable.cpp
+62 -37
View File
@@ -1,5 +1,6 @@
/*
* Copyright 2007-2011, Ingo Weinhold, [email protected].
* Copyright 2019, Haiku, Inc. All rights reserved.
* Distributed under the terms of the MIT License.
*/
@@ -42,7 +43,7 @@ struct ConditionVariableHashDefinition {
typedef BOpenHashTable<ConditionVariableHashDefinition> ConditionVariableHash;
static ConditionVariableHash sConditionVariableHash;
static spinlock sConditionVariablesLock;
static rw_spinlock sConditionVariableHashLock;
static int
@@ -93,46 +94,64 @@ ConditionVariableEntry::Add(const void* object)
{
ASSERT(object != NULL);
fThread = thread_get_current_thread();
InterruptsLocker _;
ReadSpinLocker hashLocker(sConditionVariableHashLock);
InterruptsSpinLocker _(sConditionVariablesLock);
ConditionVariable* variable = sConditionVariableHash.Lookup(object);
fVariable = sConditionVariableHash.Lookup(object);
if (fVariable == NULL) {
if (variable == NULL) {
fWaitStatus = B_ENTRY_NOT_FOUND;
return false;
}
fWaitStatus = STATUS_ADDED;
fVariable->fEntries.Add(this);
SpinLocker variableLocker(variable->fLock);
hashLocker.Unlock();
AddToLockedVariable(variable);
return true;
}
inline void
ConditionVariableEntry::AddToLockedVariable(ConditionVariable* variable)
{
ASSERT(fVariable == NULL);
B_INITIALIZE_SPINLOCK(&fLock);
fThread = thread_get_current_thread();
fVariable = variable;
fWaitStatus = STATUS_ADDED;
fVariable->fEntries.Add(this);
}
status_t
ConditionVariableEntry::Wait(uint32 flags, bigtime_t timeout)
{
#if KDEBUG
if (!are_interrupts_enabled()) {
panic("ConditionVariableEntry::Wait() called with interrupts "
"disabled, entry: %p, variable: %p", this, fVariable);
return B_ERROR;
}
#endif
InterruptsLocker _;
SpinLocker conditionLocker(sConditionVariablesLock);
SpinLocker entryLocker(fLock);
if (fVariable == NULL)
return fWaitStatus;
SpinLocker conditionLocker(fVariable->fLock);
thread_prepare_to_block(fThread, flags,
THREAD_BLOCK_TYPE_CONDITION_VARIABLE, fVariable);
fWaitStatus = STATUS_WAITING;
conditionLocker.Unlock();
entryLocker.Unlock();
status_t error;
if ((flags & (B_RELATIVE_TIMEOUT | B_ABSOLUTE_TIMEOUT)) != 0)
@@ -140,11 +159,15 @@ ConditionVariableEntry::Wait(uint32 flags, bigtime_t timeout)
else
error = thread_block();
conditionLocker.Lock();
entryLocker.Lock();
// remove entry from variable, if not done yet
if (fVariable != NULL) {
fVariable->fEntries.Remove(this);
conditionLocker.Lock();
// we need to check we are still in the variable's entries; there may
// have been a race for our removal.
if (fVariable->fEntries.Contains(this))
fVariable->fEntries.Remove(this);
fVariable = NULL;
}
@@ -162,19 +185,6 @@ ConditionVariableEntry::Wait(const void* object, uint32 flags,
}
inline void
ConditionVariableEntry::AddToVariable(ConditionVariable* variable)
{
fThread = thread_get_current_thread();
InterruptsSpinLocker _(sConditionVariablesLock);
fVariable = variable;
fWaitStatus = STATUS_ADDED;
fVariable->fEntries.Add(this);
}
// #pragma mark - ConditionVariable
@@ -186,6 +196,7 @@ ConditionVariable::Init(const void* object, const char* objectType)
fObject = object;
fObjectType = objectType;
new(&fEntries) EntryList;
B_INITIALIZE_SPINLOCK(&fLock);
T_SCHEDULING_ANALYSIS(InitConditionVariable(this, object, objectType));
NotifyWaitObjectListeners(&WaitObjectListener::ConditionVariableInitialized,
@@ -201,13 +212,13 @@ ConditionVariable::Publish(const void* object, const char* objectType)
fObject = object;
fObjectType = objectType;
new(&fEntries) EntryList;
B_INITIALIZE_SPINLOCK(&fLock);
T_SCHEDULING_ANALYSIS(InitConditionVariable(this, object, objectType));
NotifyWaitObjectListeners(&WaitObjectListener::ConditionVariableInitialized,
this);
InterruptsLocker _;
SpinLocker locker(sConditionVariablesLock);
InterruptsWriteSpinLocker _(sConditionVariableHashLock);
ASSERT_PRINT(sConditionVariableHash.Lookup(object) == NULL,
"condition variable: %p\n", sConditionVariableHash.Lookup(object));
@@ -221,7 +232,9 @@ ConditionVariable::Unpublish()
{
ASSERT(fObject != NULL);
InterruptsSpinLocker locker(sConditionVariablesLock);
InterruptsLocker _;
WriteSpinLocker hashLocker(sConditionVariableHashLock);
SpinLocker selfLocker(fLock);
#if KDEBUG
ConditionVariable* variable = sConditionVariableHash.Lookup(fObject);
@@ -235,6 +248,8 @@ ConditionVariable::Unpublish()
fObject = NULL;
fObjectType = NULL;
hashLocker.Unlock();
if (!fEntries.IsEmpty())
_NotifyLocked(true, B_ENTRY_NOT_FOUND);
}
@@ -243,7 +258,8 @@ ConditionVariable::Unpublish()
void
ConditionVariable::Add(ConditionVariableEntry* entry)
{
entry->AddToVariable(this);
InterruptsSpinLocker _(fLock);
entry->AddToLockedVariable(this);
}
@@ -259,7 +275,7 @@ ConditionVariable::Wait(uint32 flags, bigtime_t timeout)
/*static*/ void
ConditionVariable::NotifyOne(const void* object, status_t result)
{
InterruptsSpinLocker locker(sConditionVariablesLock);
InterruptsReadSpinLocker locker(sConditionVariableHashLock);
ConditionVariable* variable = sConditionVariableHash.Lookup(object);
locker.Unlock();
if (variable == NULL)
@@ -272,7 +288,7 @@ ConditionVariable::NotifyOne(const void* object, status_t result)
/*static*/ void
ConditionVariable::NotifyAll(const void* object, status_t result)
{
InterruptsSpinLocker locker(sConditionVariablesLock);
InterruptsReadSpinLocker locker(sConditionVariableHashLock);
ConditionVariable* variable = sConditionVariableHash.Lookup(object);
locker.Unlock();
if (variable == NULL)
@@ -285,7 +301,7 @@ ConditionVariable::NotifyAll(const void* object, status_t result)
void
ConditionVariable::_Notify(bool all, status_t result)
{
InterruptsSpinLocker locker(sConditionVariablesLock);
InterruptsSpinLocker _(fLock);
if (!fEntries.IsEmpty()) {
if (result > B_OK) {
@@ -298,18 +314,25 @@ ConditionVariable::_Notify(bool all, status_t result)
}
/*! Called with interrupts disabled and the condition variable spinlock and
scheduler lock held.
*/
/*! Called with interrupts disabled and the condition variable's spinlock held.
*/
void
ConditionVariable::_NotifyLocked(bool all, status_t result)
{
// dequeue and wake up the blocked threads
// Dequeue and wake up the blocked threads.
// We *cannot* hold our own lock while acquiring the Entry's lock,
// as this leads to a (non-theoretical!) race between the Entry
// entering Wait() and acquiring its own lock, and then acquiring ours.
while (ConditionVariableEntry* entry = fEntries.RemoveHead()) {
release_spinlock(&fLock);
SpinLocker _(entry->fLock);
entry->fVariable = NULL;
if (entry->fWaitStatus <= 0)
if (entry->fWaitStatus <= 0) {
acquire_spinlock(&fLock);
continue;
}
if (entry->fWaitStatus == STATUS_WAITING) {
SpinLocker _(entry->fThread->scheduler_lock);
@@ -318,6 +341,8 @@ ConditionVariable::_NotifyLocked(bool all, status_t result)
entry->fWaitStatus = result;
acquire_spinlock(&fLock);
if (!all)
break;
}