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

Commit

Permalink
Address feedback, miscellaneous cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@kouvel
kouvel committed Oct 6, 2017
1 parent 8c2268b commit a0c4f95
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 42 deletions.
6 changes: 3 additions & 3 deletions src/debug/daccess/request.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3726,9 +3726,9 @@ ClrDataAccess::GetSyncBlockData(unsigned int SBNumber, struct DacpSyncBlockData
}
#endif // FEATURE_COMINTEROP

pSyncBlockData->MonitorHeld = pBlock->m_Monitor.m_lockState.VolatileLoad().GetMonitorHeldState();
pSyncBlockData->Recursion = pBlock->m_Monitor.m_Recursion;
pSyncBlockData->HoldingThread = HOST_CDADDR(pBlock->m_Monitor.m_HoldingThread);
pSyncBlockData->MonitorHeld = pBlock->m_Monitor.GetMonitorHeldStateVolatile();
pSyncBlockData->Recursion = pBlock->m_Monitor.GetRecursionLevel();
pSyncBlockData->HoldingThread = HOST_CDADDR(pBlock->m_Monitor.GetHoldingThread());

if (pBlock->GetAppDomainIndex().m_dwIndex)
{
Expand Down
2 changes: 1 addition & 1 deletion src/inc/clrconfigvalues.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ RETAIL_CONFIG_DWORD_INFO_EX(EXTERNAL_SpinBackoffFactor, W("SpinBackoffFactor"),
RETAIL_CONFIG_DWORD_INFO_EX(EXTERNAL_SpinLimitProcCap, W("SpinLimitProcCap"), 0xFFFFFFFF, "Hex value specifying the largest value of NumProcs to use when calculating the maximum spin duration", EEConfig_default)
RETAIL_CONFIG_DWORD_INFO_EX(EXTERNAL_SpinLimitProcFactor, W("SpinLimitProcFactor"), 0x4E20, "Hex value specifying the multiplier on NumProcs to use when calculating the maximum spin duration", EEConfig_default)
RETAIL_CONFIG_DWORD_INFO_EX(EXTERNAL_SpinLimitConstant, W("SpinLimitConstant"), 0x0, "Hex value specifying the constant to add when calculating the maximum spin duration", EEConfig_default)
RETAIL_CONFIG_DWORD_INFO_EX(EXTERNAL_SpinRetryCount, W("SpinRetryCount"), 0x0, "Hex value specifying the number of times the entire spin process is repeated (when applicable)", EEConfig_default)
RETAIL_CONFIG_DWORD_INFO_EX(EXTERNAL_SpinRetryCount, W("SpinRetryCount"), 0xA, "Hex value specifying the number of times the entire spin process is repeated (when applicable)", EEConfig_default)

//
// Native Binder
Expand Down
2 changes: 1 addition & 1 deletion src/vm/interpreter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1756,7 +1756,7 @@ static void MonitorEnter(Object* obj, BYTE* pbLockTaken)
GET_THREAD()->PulseGCMode();
}
objRef->EnterObjMonitor();
_ASSERTE ((objRef->GetSyncBlock()->GetMonitor()->m_Recursion == 1 && pThread->m_dwLockCount == lockCount + 1) ||
_ASSERTE ((objRef->GetSyncBlock()->GetMonitor()->GetRecursionLevel() == 1 && pThread->m_dwLockCount == lockCount + 1) ||
pThread->m_dwLockCount == lockCount);
if (pbLockTaken != 0) *pbLockTaken = 1;

Expand Down
2 changes: 1 addition & 1 deletion src/vm/jithelpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4416,7 +4416,7 @@ NOINLINE static void JIT_MonEnter_Helper(Object* obj, BYTE* pbLockTaken, LPVOID
GET_THREAD()->PulseGCMode();
}
objRef->EnterObjMonitor();
_ASSERTE ((objRef->GetSyncBlock()->GetMonitor()->m_Recursion == 1 && pThread->m_dwLockCount == lockCount + 1) ||
_ASSERTE ((objRef->GetSyncBlock()->GetMonitor()->GetRecursionLevel() == 1 && pThread->m_dwLockCount == lockCount + 1) ||
pThread->m_dwLockCount == lockCount);
if (pbLockTaken != 0) *pbLockTaken = 1;

Expand Down
7 changes: 3 additions & 4 deletions src/vm/syncblk.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2157,7 +2157,7 @@ BOOL ObjHeader::GetThreadOwningMonitorLock(DWORD *pThreadId, DWORD *pAcquisition
SyncBlock* psb = g_pSyncTable[(int)index].m_SyncBlock;

_ASSERTE(psb->GetMonitor() != NULL);
Thread* pThread = psb->GetMonitor()->m_HoldingThread;
Thread* pThread = psb->GetMonitor()->GetHoldingThread();
if(pThread == NULL)
{
*pThreadId = 0;
Expand All @@ -2167,7 +2167,7 @@ BOOL ObjHeader::GetThreadOwningMonitorLock(DWORD *pThreadId, DWORD *pAcquisition
else
{
*pThreadId = pThread->GetThreadId();
*pAcquisitionCount = psb->GetMonitor()->m_Recursion;
*pAcquisitionCount = psb->GetMonitor()->GetRecursionLevel();
return TRUE;
}
}
Expand Down Expand Up @@ -2774,8 +2774,7 @@ SyncBlock *ObjHeader::GetSyncBlock()
// The lock is orphaned.
pThread = (Thread*) -1;
}
syncBlock->InitState();
syncBlock->SetAwareLock(pThread, recursionLevel + 1);
syncBlock->InitState(recursionLevel + 1, pThread);
}
}
else if ((bits & BIT_SBLK_IS_HASHCODE) != 0)
Expand Down
78 changes: 57 additions & 21 deletions src/vm/syncblk.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ class AwareLock
LeaveHelperAction_Error,
};

public:
private:
class LockState
{
private:
Expand All @@ -221,6 +221,12 @@ class AwareLock
}

public:
UINT32 GetState() const
{
LIMITED_METHOD_CONTRACT;
return m_state;
}

UINT32 GetMonitorHeldState() const
{
LIMITED_METHOD_CONTRACT;
Expand All @@ -241,7 +247,7 @@ class AwareLock
return !(m_state & (IsLockedMask + WaiterCountMask));
}

void SetIsLockedWithNoWaiters()
void InitializeToLockedWithNoWaiters()
{
LIMITED_METHOD_CONTRACT;
_ASSERTE(!m_state);
Expand Down Expand Up @@ -333,7 +339,7 @@ class AwareLock
private:
bool NeedToSignalWaiter() const
{
LIMITED_METHOD_CONTRACT;
WRAPPER_NO_CONTRACT;
return HasAnyWaiters() && !(m_state & (SpinnerCountMask + IsWaiterSignaledToWakeMask));
}

Expand All @@ -355,7 +361,7 @@ class AwareLock
public:
LockState VolatileLoad() const
{
LIMITED_METHOD_CONTRACT;
WRAPPER_NO_CONTRACT;
return ::VolatileLoad(&m_state);
}

Expand Down Expand Up @@ -385,12 +391,11 @@ class AwareLock
bool InterlockedObserveWakeSignal_Try_LockAndUnregisterWaiter();
};

public:
private:
LockState m_lockState;
ULONG m_Recursion;
PTR_Thread m_HoldingThread;

private:

LONG m_TransientPrecious;


Expand Down Expand Up @@ -437,6 +442,47 @@ class AwareLock
}
#endif // defined(ENABLE_CONTRACTS_IMPL)

public:
UINT32 GetLockState() const
{
WRAPPER_NO_CONTRACT;
return m_lockState.GetState();
}

bool IsUnlockedWithNoWaiters() const
{
WRAPPER_NO_CONTRACT;
return m_lockState.IsUnlockedWithNoWaiters();
}

UINT32 GetMonitorHeldStateVolatile() const
{
WRAPPER_NO_CONTRACT;
return m_lockState.VolatileLoad().GetMonitorHeldState();
}

ULONG GetRecursionLevel() const
{
LIMITED_METHOD_CONTRACT;
return m_Recursion;
}

PTR_Thread GetHoldingThread() const
{
LIMITED_METHOD_CONTRACT;
return m_HoldingThread;
}

private: // friend access is required for this unsafe function
void InitializeToLockedWithNoWaiters(ULONG recursionLevel, PTR_Thread holdingThread)
{
WRAPPER_NO_CONTRACT;

m_lockState.InitializeToLockedWithNoWaiters();
m_Recursion = recursionLevel;
m_HoldingThread = holdingThread;
}

public:
static void SpinWait(DWORD spinCount);

Expand Down Expand Up @@ -819,7 +865,7 @@ class SyncBlock
{
WRAPPER_NO_CONTRACT;
return (!IsPrecious() &&
m_Monitor.m_lockState.IsUnlockedWithNoWaiters() &&
m_Monitor.IsUnlockedWithNoWaiters() &&
m_Monitor.m_TransientPrecious == 0);
}

Expand Down Expand Up @@ -904,16 +950,6 @@ class SyncBlock
m_dwAppDomainIndex = dwAppDomainIndex;
}

void SetAwareLock(Thread *holdingThread, DWORD recursionLevel)
{
LIMITED_METHOD_CONTRACT;
// <NOTE>
// DO NOT SET m_lockState HERE! THIS IS NOT PROTECTED BY ANY LOCK!!
// </NOTE>
m_Monitor.m_HoldingThread = PTR_Thread(holdingThread);
m_Monitor.m_Recursion = recursionLevel;
}

DWORD GetHashCode()
{
LIMITED_METHOD_CONTRACT;
Expand Down Expand Up @@ -1058,10 +1094,10 @@ class SyncBlock
// This should ONLY be called when initializing a SyncBlock (i.e. ONLY from
// ObjHeader::GetSyncBlock()), otherwise we'll have a race condition.
// </NOTE>
void InitState()
void InitState(ULONG recursionLevel, PTR_Thread holdingThread)
{
LIMITED_METHOD_CONTRACT;
m_Monitor.m_lockState.SetIsLockedWithNoWaiters();
WRAPPER_NO_CONTRACT;
m_Monitor.InitializeToLockedWithNoWaiters(recursionLevel, holdingThread);
}

#if defined(ENABLE_CONTRACTS_IMPL)
Expand Down
8 changes: 5 additions & 3 deletions src/vm/syncblk.inl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ FORCEINLINE bool AwareLock::LockState::InterlockedUnregisterSpinner_TryLock()
return false;
}

LockState state = stateBeforeUpdate - SpinnerCountIncrement;
LockState state = stateBeforeUpdate;
state.DecrementSpinnerCount();
_ASSERTE(!state.IsLocked());
do
{
Expand Down Expand Up @@ -218,8 +219,8 @@ FORCEINLINE bool AwareLock::LockState::InterlockedTry_LockAndUnregisterWaiterAnd

LockState newState = state;
newState.InvertIsLocked();
newState.DecrementWaiterCount();
newState.InvertIsWaiterSignaledToWake();
newState.DecrementWaiterCount();

LockState stateBeforeUpdate = CompareExchange(newState, state);
if (stateBeforeUpdate == state)
Expand Down Expand Up @@ -247,7 +248,8 @@ FORCEINLINE bool AwareLock::LockState::InterlockedObserveWakeSignal_Try_LockAndU
return false;
}

LockState state = stateBeforeUpdate - IsWaiterSignaledToWakeMask;
LockState state = stateBeforeUpdate;
state.InvertIsWaiterSignaledToWake();
_ASSERTE(!state.IsLocked());
do
{
Expand Down
16 changes: 8 additions & 8 deletions src/vm/threads.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1579,11 +1579,11 @@ void Dbg_TrackSyncStack::EnterSync(UINT_PTR caller, void *pAwareLock)
{
LIMITED_METHOD_CONTRACT;

STRESS_LOG4(LF_SYNC, LL_INFO100, "Dbg_TrackSyncStack::EnterSync, IP=%p, Recursion=%d, MonitorHeld=%d, HoldingThread=%p.\n",
STRESS_LOG4(LF_SYNC, LL_INFO100, "Dbg_TrackSyncStack::EnterSync, IP=%p, Recursion=%u, LockState=%x, HoldingThread=%p.\n",
caller,
((AwareLock*)pAwareLock)->m_Recursion,
(int)((AwareLock*)pAwareLock)->m_lockState,
((AwareLock*)pAwareLock)->m_HoldingThread );
((AwareLock*)pAwareLock)->GetRecursionLevel(),
((AwareLock*)pAwareLock)->GetState(),
((AwareLock*)pAwareLock)->GetHoldingThread());

if (m_Active)
{
Expand All @@ -1605,11 +1605,11 @@ void Dbg_TrackSyncStack::LeaveSync(UINT_PTR caller, void *pAwareLock)
{
WRAPPER_NO_CONTRACT;

STRESS_LOG4(LF_SYNC, LL_INFO100, "Dbg_TrackSyncStack::LeaveSync, IP=%p, Recursion=%d, MonitorHeld=%d, HoldingThread=%p.\n",
STRESS_LOG4(LF_SYNC, LL_INFO100, "Dbg_TrackSyncStack::LeaveSync, IP=%p, Recursion=%u, LockState=%x, HoldingThread=%p.\n",
caller,
((AwareLock*)pAwareLock)->m_Recursion,
(int)((AwareLock*)pAwareLock)->m_lockState,
((AwareLock*)pAwareLock)->m_HoldingThread );
((AwareLock*)pAwareLock)->GetRecursionLevel(),
((AwareLock*)pAwareLock)->GetState(),
((AwareLock*)pAwareLock)->GetHoldingThread());

if (m_Active)
{
Expand Down

0 comments on commit a0c4f95

Please sign in to comment.