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

Commit

Permalink
Merge remote-tracking branch 'refs/remotes/origin/WorkerThreadStart' …
Browse files Browse the repository at this point in the history
…into threapool-falsesharing
  • Loading branch information
benaadams committed Aug 12, 2016
2 parents 4e39e49 + a0597da commit 5f13774
Show file tree
Hide file tree
Showing 5 changed files with 201 additions and 153 deletions.
2 changes: 1 addition & 1 deletion src/vm/comthreadpool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ FCIMPL0(FC_BOOL_RET, ThreadPoolNative::NotifyRequestComplete)
}

if (needReset)
pThread->InternalReset (FALSE, TRUE, TRUE, FALSE);
pThread->InternalReset(FALSE, TRUE, TRUE, FALSE);

HELPER_METHOD_FRAME_END();
}
Expand Down
183 changes: 98 additions & 85 deletions src/vm/threadpoolrequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@
#endif
#include "appdomain.inl"

UnManagedPerAppDomainTPCount PerAppDomainTPCountList::s_unmanagedTPCount;
BYTE PerAppDomainTPCountList::s_padding[64 - sizeof(LONG)];
// Make this point to unmanaged TP in case, no appdomains have initialized yet.
// Cacheline aligned, hot variable
DECLSPEC_ALIGN(64) LONG PerAppDomainTPCountList::s_ADHint = -1;

// Move out of from preceeding variables' cache line
DECLSPEC_ALIGN(64) UnManagedPerAppDomainTPCount PerAppDomainTPCountList::s_unmanagedTPCount;
//The list of all per-appdomain work-request counts.
ArrayListStatic PerAppDomainTPCountList::s_appDomainIndexList;

//Make this point to unmanaged TP in case, no appdomains have initialized yet.
LONG PerAppDomainTPCountList::s_ADHint=-1;
ArrayListStatic PerAppDomainTPCountList::s_appDomainIndexList;

void PerAppDomainTPCountList::InitAppDomainIndexList()
{
Expand Down Expand Up @@ -74,7 +76,14 @@ TPIndex PerAppDomainTPCountList::AddNewTPIndex()
return index;
}

#ifdef _MSC_VER
// Disable this warning - we intentionally want __declspec(align()) to insert trailing padding for us
#pragma warning(disable:4316) // Object allocated on the heap may not be aligned for this type.
#endif
ManagedPerAppDomainTPCount * pAdCount = new ManagedPerAppDomainTPCount(index);
#ifdef _MSC_VER
#pragma warning(default:4316) // Object allocated on the heap may not be aligned for this type.
#endif
pAdCount->ResetState();

IfFailThrow(s_appDomainIndexList.Append(pAdCount));
Expand Down Expand Up @@ -274,34 +283,33 @@ LONG PerAppDomainTPCountList::GetAppDomainIndexForThreadpoolDispatch()
}
CONTRACTL_END;

LONG hint = s_ADHint;
DWORD count = s_appDomainIndexList.GetCount();
IPerAppDomainTPCount * pAdCount;
DWORD DwnumADs = s_appDomainIndexList.GetCount();
DWORD Dwi;

LONG hint = s_ADHint;
LONG temphint = hint;

if (hint == -1)
if (hint != -1)
{
pAdCount = &s_unmanagedTPCount;
pAdCount = dac_cast<PTR_IPerAppDomainTPCount>(s_appDomainIndexList.Get(hint));
}
else
{
pAdCount = dac_cast<PTR_IPerAppDomainTPCount>(s_appDomainIndexList.Get(hint));
pAdCount = &s_unmanagedTPCount;
}

//temphint ensures that the check for appdomains proceeds in a pure round robin fashion.
LONG temphint = hint;

_ASSERTE( pAdCount);

if (pAdCount->TakeActiveRequest())
goto HintDone;

//temphint ensures that the check for appdomains proceeds in a pure round robin fashion.
temphint = hint;

//If there is no work in any appdomains, check the unmanaged queue,
hint = -1;

for (Dwi=0;Dwi<DwnumADs;Dwi++)
for (Dwi=0;Dwi<count;Dwi++)
{
if (temphint == -1)
{
Expand All @@ -317,9 +325,9 @@ LONG PerAppDomainTPCountList::GetAppDomainIndexForThreadpoolDispatch()

temphint++;

_ASSERTE( temphint <= (LONG) DwnumADs);
_ASSERTE( temphint <= (LONG)count);

if(temphint == (LONG) DwnumADs)
if(temphint == (LONG)count)
{
temphint = 0;
}
Expand All @@ -331,11 +339,9 @@ LONG PerAppDomainTPCountList::GetAppDomainIndexForThreadpoolDispatch()
return 0;
}

HintDone:
HintDone:

LONG count = (LONG) s_appDomainIndexList.GetCount();

if((hint+1) < count)
if((hint+1) < (LONG)count)
{
s_ADHint = hint+1;
}
Expand All @@ -359,7 +365,7 @@ void UnManagedPerAppDomainTPCount::SetAppDomainRequestsActive()
{
WRAPPER_NO_CONTRACT;
#ifndef DACCESS_COMPILE
LONG count = m_outstandingThreadRequestCount;
LONG count = VolatileLoad(&m_outstandingThreadRequestCount);
while (count < (LONG)ThreadpoolMgr::NumberOfProcessors)
{
LONG prevCount = FastInterlockCompareExchange(&m_outstandingThreadRequestCount, count+1, count);
Expand All @@ -380,7 +386,7 @@ void UnManagedPerAppDomainTPCount::SetAppDomainRequestsActive()
bool FORCEINLINE UnManagedPerAppDomainTPCount::TakeActiveRequest()
{
LIMITED_METHOD_CONTRACT;
LONG count = m_outstandingThreadRequestCount;
LONG count = VolatileLoad(&m_outstandingThreadRequestCount);

while (count > 0)
{
Expand Down Expand Up @@ -599,7 +605,7 @@ void ManagedPerAppDomainTPCount::SetAppDomainRequestsActive()
_ASSERTE(m_id.m_dwId != 0);

#ifndef DACCESS_COMPILE
LONG count = m_numRequestsPending;
LONG count = VolatileLoad(&m_numRequestsPending);
while (count != ADUnloading)
{
LONG prev = FastInterlockCompareExchange(&m_numRequestsPending, count+1, count);
Expand Down Expand Up @@ -628,12 +634,15 @@ void ManagedPerAppDomainTPCount::ClearAppDomainRequestsActive(BOOL bADU)

if (bADU)
{
m_numRequestsPending = ADUnloading;
VolatileStore(&m_numRequestsPending, ADUnloading);
}
else
{
LONG count = m_numRequestsPending;
while (count != ADUnloading && count > 0)
LONG count = VolatileLoad(&m_numRequestsPending);
// Test is: count > 0 && count != ADUnloading
// Since: const ADUnloading == -1
// Both are tested: (count > 0) means following also true (count != ADUnloading)
while (count > 0)
{
LONG prev = FastInterlockCompareExchange(&m_numRequestsPending, 0, count);
if (prev == count)
Expand All @@ -646,8 +655,11 @@ void ManagedPerAppDomainTPCount::ClearAppDomainRequestsActive(BOOL bADU)
bool ManagedPerAppDomainTPCount::TakeActiveRequest()
{
LIMITED_METHOD_CONTRACT;
LONG count = m_numRequestsPending;
while (count != ADUnloading && count > 0)
LONG count = VolatileLoad(&m_numRequestsPending);
// Test is: count > 0 && count != ADUnloading
// Since: const ADUnloading == -1
// Both are tested: (count > 0) means following also true (count != ADUnloading)
while (count > 0)
{
LONG prev = FastInterlockCompareExchange(&m_numRequestsPending, count-1, count);
if (prev == count)
Expand Down Expand Up @@ -678,7 +690,7 @@ void ManagedPerAppDomainTPCount::ClearAppDomainUnloading()
// it should be, but if it's smaller then we will be permanently out of sync with the
// AD.
//
m_numRequestsPending = ThreadpoolMgr::NumberOfProcessors;
VolatileStore(&m_numRequestsPending, (LONG)ThreadpoolMgr::NumberOfProcessors);
if (!CLRThreadpoolHosted() && ThreadpoolMgr::IsInitialized())
{
ThreadpoolMgr::MaybeAddWorkingWorker();
Expand Down Expand Up @@ -715,73 +727,74 @@ void ManagedPerAppDomainTPCount::DispatchWorkItem(bool* foundWork, bool* wasNotR
//We are in a state where AppDomain Unload has begun, but not all threads have been
//forced out of the unloading domain. This check below will prevent us from getting
//unmanaged AD unloaded exceptions while trying to enter an unloaded appdomain.

if(IsAppDomainUnloading())
{
__SwitchToThread(0, CALLER_LIMITS_SPINNING);
return;
}

CONTRACTL
if (!IsAppDomainUnloading())
{
MODE_PREEMPTIVE;
THROWS;
GC_TRIGGERS;
}
CONTRACTL_END;

GCX_COOP();
BEGIN_SO_INTOLERANT_CODE(pThread);
CONTRACTL
{
MODE_PREEMPTIVE;
THROWS;
GC_TRIGGERS;
}
CONTRACTL_END;

//
// NOTE: there is a potential race between the time we retrieve the app
// domain pointer, and the time which this thread enters the domain.
//
// To solve the race, we rely on the fact that there is a thread sync (via
// GC) between releasing an app domain's handle, and destroying the
// app domain. Thus it is important that we not go into preemptive gc mode
// in that window.
//
GCX_COOP();
BEGIN_SO_INTOLERANT_CODE(pThread);

{
ADID appDomainId(m_id);
//
// NOTE: there is a potential race between the time we retrieve the app
// domain pointer, and the time which this thread enters the domain.
//
// To solve the race, we rely on the fact that there is a thread sync (via
// GC) between releasing an app domain's handle, and destroying the
// app domain. Thus it is important that we not go into preemptive gc mode
// in that window.
//

// This TPIndex may have been recycled since we chose it for workitem dispatch.
// Thus it's possible for the ADID we just read to refer to an AppDomain that's still
// being created. If so, the new AppDomain will necessarily have zero requests
// pending (because the destruction of the previous AD that used this TPIndex
// will have reset this object). We don't want to call into such an AppDomain.
// TODO: fix this another way!
// if (IsRequestPending())
{
//This holder resets our thread's security state when exiting this scope
ThreadSecurityStateHolder secState(pThread);

ManagedThreadBase::ThreadPool(appDomainId, QueueUserWorkItemManagedCallback, wasNotRecalled);
}
ADID appDomainId(m_id);

// This TPIndex may have been recycled since we chose it for workitem dispatch.
// Thus it's possible for the ADID we just read to refer to an AppDomain that's still
// being created. If so, the new AppDomain will necessarily have zero requests
// pending (because the destruction of the previous AD that used this TPIndex
// will have reset this object). We don't want to call into such an AppDomain.
// TODO: fix this another way!
// if (IsRequestPending())
{
//This holder resets our thread's security state when exiting this scope
ThreadSecurityStateHolder secState(pThread);

if (pThread->IsAbortRequested())
{
// thread was aborted, and may not have had a chance to tell us it has work.
ENTER_DOMAIN_ID(m_id)
ManagedThreadBase::ThreadPool(appDomainId, QueueUserWorkItemManagedCallback, wasNotRecalled);
}

if (pThread->IsAbortRequested())
{
ThreadpoolMgr::SetAppDomainRequestsActive();
ThreadpoolMgr::QueueUserWorkItem(NULL,
NULL,
0,
FALSE);
// thread was aborted, and may not have had a chance to tell us it has work.
ENTER_DOMAIN_ID(m_id)
{
ThreadpoolMgr::SetAppDomainRequestsActive();
ThreadpoolMgr::QueueUserWorkItem(NULL,
NULL,
0,
FALSE);

}
END_DOMAIN_TRANSITION;
}
END_DOMAIN_TRANSITION;
}
}

// We should have released all locks.
_ASSERTE(g_fEEShutDown || pThread->m_dwLockCount == 0 || pThread->m_fRudeAborted);
END_SO_INTOLERANT_CODE;
// We should have released all locks.
_ASSERTE(g_fEEShutDown || pThread->m_dwLockCount == 0 || pThread->m_fRudeAborted);

END_SO_INTOLERANT_CODE;

*foundWork = true;
*foundWork = true;
}
else
{
__SwitchToThread(0, CALLER_LIMITS_SPINNING);
return;
}
}

#endif // !DACCESS_COMPILE
Loading

0 comments on commit 5f13774

Please sign in to comment.