Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Commit

Permalink
Add mitigation for waiter starvation
Browse files Browse the repository at this point in the history
There are cases where waiters can be easily starved as a result. For example, a thread that holds a lock for a significant amount of time (much longer than the time it takes to do a context switch), then releases and reacquires the lock in quick succession, and repeats. Though a waiter would be woken upon lock release, usually it will not have enough time to context-switch-in and take the lock, and can be starved for an unreasonably long duration.

In order to prevent such starvation and force a bit of fair forward progress, it is sometimes necessary to change the normal policy and disallow threads to preempt waiters. A new bit was added to LockState and ShouldNotPreemptWaiters() indicates the current state of the policy.
- When the first waiter begins waiting, it records the current time as a "waiter starvation start time". That is a point in time after which no forward progress has occurred for waiters. When a waiter acquires the lock, the time is updated to the current time.
- Before a spinner begins spinning, and when a waiter is signaled to wake, it checks whether the starvation duration has crossed a threshold (currently one second) and if so, sets ShouldNotPreemptWaiters()

When unreasonable starvation is occurring, the lock will be released occasionally and if caused by spinners, spinners will be starting to spin.
- Before starting to spin, if ShouldNotPreemptWaiters() is set, the spinner will skip spinning and wait instead. Spinners that are already registered at the time ShouldNotPreemptWaiters() is set will stop spinning as necessary. Eventually, all spinners will drain and no new ones will be registered.
- After spinners have drained, only a waiter will be able to acquire the lock. When a waiter acquires the lock, or when the last waiter unregisters itself, ShouldNotPreemptWaiters() is cleared to restore the normal policy.
  • Loading branch information
kouvel committed Nov 16, 2017
1 parent b84350c commit 9f94b68
Show file tree
Hide file tree
Showing 3 changed files with 288 additions and 42 deletions.
15 changes: 10 additions & 5 deletions src/vm/syncblk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1959,10 +1959,15 @@ AwareLock::EnterHelperResult ObjHeader::EnterObjMonitorHelperSpin(Thread* pCurTh
break;
}

if (awareLock->TryEnterInsideSpinLoopHelper(pCurThread))
result = awareLock->TryEnterInsideSpinLoopHelper(pCurThread);
if (result == AwareLock::EnterHelperResult_Entered)
{
return AwareLock::EnterHelperResult_Entered;
}
if (result == AwareLock::EnterHelperResult_UseSlowPath)
{
break;
}
}
}

Expand Down Expand Up @@ -2943,7 +2948,7 @@ void AwareLock::Enter()
LockState state = m_lockState;
if (!state.IsLocked() || m_HoldingThread != pCurThread)
{
if (m_lockState.InterlockedTryLock_Or_RegisterWaiter(state))
if (m_lockState.InterlockedTryLock_Or_RegisterWaiter(this, state))
{
// We get here if we successfully acquired the mutex.
m_HoldingThread = pCurThread;
Expand Down Expand Up @@ -3007,7 +3012,7 @@ BOOL AwareLock::TryEnter(INT32 timeOut)
{
if (timeOut == 0
? m_lockState.InterlockedTryLock(state)
: m_lockState.InterlockedTryLock_Or_RegisterWaiter(state))
: m_lockState.InterlockedTryLock_Or_RegisterWaiter(this, state))
{
// We get here if we successfully acquired the mutex.
m_HoldingThread = pCurThread;
Expand Down Expand Up @@ -3193,7 +3198,7 @@ BOOL AwareLock::EnterEpilogHelper(Thread* pCurThread, INT32 timeOut)
const DWORD spinCount = g_SpinConstants.dwMonitorSpinCount;
for (DWORD spinIteration = 0; spinIteration < spinCount; ++spinIteration)
{
if (m_lockState.InterlockedTry_LockAndUnregisterWaiterAndObserveWakeSignal())
if (m_lockState.InterlockedTry_LockAndUnregisterWaiterAndObserveWakeSignal(this))
{
acquiredLock = true;
break;
Expand All @@ -3207,7 +3212,7 @@ BOOL AwareLock::EnterEpilogHelper(Thread* pCurThread, INT32 timeOut)
}
}

if (m_lockState.InterlockedObserveWakeSignal_Try_LockAndUnregisterWaiter())
if (m_lockState.InterlockedObserveWakeSignal_Try_LockAndUnregisterWaiter(this))
{
break;
}
Expand Down
62 changes: 51 additions & 11 deletions src/vm/syncblk.h
Original file line number Diff line number Diff line change
Expand Up @@ -205,13 +205,14 @@ class AwareLock
{
private:
// Layout constants for m_state
static const UINT32 IsLockedMask = 0x1; // bit 0
static const UINT32 SpinnerCountIncrement = 0x2;
static const UINT32 SpinnerCountMask = 0xe; // bits 1-3
static const UINT32 IsWaiterSignaledToWakeMask = 0x10; // bit 4
static const UINT8 WaiterCountShift = 5;
static const UINT32 IsLockedMask = (UINT32)1 << 0; // bit 0
static const UINT32 ShouldNotPreemptWaitersMask = (UINT32)1 << 1; // bit 1
static const UINT32 SpinnerCountIncrement = (UINT32)1 << 2;
static const UINT32 SpinnerCountMask = (UINT32)0x7 << 2; // bits 2-4
static const UINT32 IsWaiterSignaledToWakeMask = (UINT32)1 << 5; // bit 5
static const UINT8 WaiterCountShift = 6;
static const UINT32 WaiterCountIncrement = (UINT32)1 << WaiterCountShift;
static const UINT32 WaiterCountMask = (UINT32)-1 >> WaiterCountShift << WaiterCountShift; // bits 5-31
static const UINT32 WaiterCountMask = (UINT32)-1 >> WaiterCountShift << WaiterCountShift; // bits 6-31

private:
UINT32 m_state;
Expand Down Expand Up @@ -271,6 +272,30 @@ class AwareLock
m_state ^= IsLockedMask;
}

public:
bool ShouldNotPreemptWaiters() const
{
LIMITED_METHOD_CONTRACT;
return !!(m_state & ShouldNotPreemptWaitersMask);
}

private:
void InvertShouldNotPreemptWaiters()
{
WRAPPER_NO_CONTRACT;

m_state ^= ShouldNotPreemptWaitersMask;
_ASSERTE(!ShouldNotPreemptWaiters() || HasAnyWaiters());
}

bool ShouldNonWaiterAttemptToAcquireLock() const
{
WRAPPER_NO_CONTRACT;
_ASSERTE(!ShouldNotPreemptWaiters() || HasAnyWaiters());

return !(m_state & (IsLockedMask + ShouldNotPreemptWaitersMask));
}

public:
bool HasAnySpinners() const
{
Expand Down Expand Up @@ -384,15 +409,19 @@ class AwareLock
bool InterlockedTryLock();
bool InterlockedTryLock(LockState state);
bool InterlockedUnlock();
bool InterlockedTrySetShouldNotPreemptWaitersIfNecessary(AwareLock *awareLock);
bool InterlockedTrySetShouldNotPreemptWaitersIfNecessary(AwareLock *awareLock, LockState state);
EnterHelperResult InterlockedTry_LockOrRegisterSpinner(LockState state);
bool InterlockedTry_LockAndUnregisterSpinner();
EnterHelperResult InterlockedTry_LockAndUnregisterSpinner();
bool InterlockedUnregisterSpinner_TryLock();
bool InterlockedTryLock_Or_RegisterWaiter(LockState state);
bool InterlockedTryLock_Or_RegisterWaiter(AwareLock *awareLock, LockState state);
void InterlockedUnregisterWaiter();
bool InterlockedTry_LockAndUnregisterWaiterAndObserveWakeSignal();
bool InterlockedObserveWakeSignal_Try_LockAndUnregisterWaiter();
bool InterlockedTry_LockAndUnregisterWaiterAndObserveWakeSignal(AwareLock *awareLock);
bool InterlockedObserveWakeSignal_Try_LockAndUnregisterWaiter(AwareLock *awareLock);
};

friend class LockState;

private:
LockState m_lockState;
ULONG m_Recursion;
Expand All @@ -407,6 +436,10 @@ class AwareLock

CLREvent m_SemEvent;

DWORD m_waiterStarvationStartTimeMs;

static const DWORD WaiterStarvationDurationMsBeforeStoppingPreemptingWaiters = 1000;

// Only SyncBlocks can create AwareLocks. Hence this private constructor.
AwareLock(DWORD indx)
: m_Recursion(0),
Expand Down Expand Up @@ -475,6 +508,11 @@ class AwareLock
return m_HoldingThread;
}

private:
void ResetWaiterStarvationStartTime();
void RecordWaiterStarvationStartTime();
bool ShouldStopPreemptingWaiters() const;

private: // friend access is required for this unsafe function
void InitializeToLockedWithNoWaiters(ULONG recursionLevel, PTR_Thread holdingThread)
{
Expand All @@ -492,7 +530,7 @@ class AwareLock
bool TryEnterHelper(Thread* pCurThread);

EnterHelperResult TryEnterBeforeSpinLoopHelper(Thread *pCurThread);
bool TryEnterInsideSpinLoopHelper(Thread *pCurThread);
EnterHelperResult TryEnterInsideSpinLoopHelper(Thread *pCurThread);
bool TryEnterAfterSpinLoopHelper(Thread *pCurThread);

// Helper encapsulating the core logic for leaving monitor. Returns what kind of
Expand All @@ -511,6 +549,8 @@ class AwareLock

// CLREvent::SetMonitorEvent works even if the event has not been intialized yet
m_SemEvent.SetMonitorEvent();

m_lockState.InterlockedTrySetShouldNotPreemptWaitersIfNecessary(this);
}

void AllocLockSemEvent();
Expand Down
Loading

0 comments on commit 9f94b68

Please sign in to comment.