Skip to content

Commit

Permalink
Review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@gbalykov
gbalykov committed Jun 4, 2021
1 parent a299f4b commit 2d7eede
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 69 deletions.
32 changes: 7 additions & 25 deletions src/coreclr/vm/multicorejit.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,6 @@ HRESULT MulticoreJitRecorder::WriteOutput(IStream * pStream)
header.shortCounters[ 8] = m_stats.m_nDelayCount;
header.shortCounters[ 9] = m_stats.m_nWalkBack;
header.shortCounters[10] = m_fAppxMode;
header.shortCounters[11] = m_stats.m_nNoJitCount;

_ASSERTE(HEADER_W_COUNTER >= 14);

Expand Down Expand Up @@ -601,14 +600,14 @@ unsigned MulticoreJitRecorder::GetOrAddModuleIndex(Module * pModule)
return slot;
}

void MulticoreJitRecorder::RecordMethodInfo(unsigned moduleIndex, MethodDesc * pMethod, bool application, bool dojit)
void MulticoreJitRecorder::RecordMethodInfo(unsigned moduleIndex, MethodDesc * pMethod, bool application)
{
LIMITED_METHOD_CONTRACT;

if (m_JitInfoArray != nullptr && m_JitInfoCount < (LONG) MAX_METHODS)
{
m_ModuleList[moduleIndex].methodCount++;
m_JitInfoArray[m_JitInfoCount++].PackMethod(moduleIndex, pMethod, application, dojit);
m_JitInfoArray[m_JitInfoCount++].PackMethod(moduleIndex, pMethod, application);
}
}

Expand Down Expand Up @@ -870,7 +869,7 @@ void MulticoreJitRecorder::PreRecordFirstMethod()
}


void MulticoreJitRecorder::RecordMethodJit(MethodDesc * pMethod, bool application, bool dojit)
void MulticoreJitRecorder::RecordMethodJitOrLoad(MethodDesc * pMethod, bool application)
{
STANDARD_VM_CONTRACT;

Expand All @@ -889,7 +888,7 @@ void MulticoreJitRecorder::RecordMethodJit(MethodDesc * pMethod, bool applicatio
return;
}

RecordMethodInfo(moduleIndex, pMethod, application, dojit);
RecordMethodInfo(moduleIndex, pMethod, application);
}


Expand Down Expand Up @@ -1104,7 +1103,7 @@ MulticoreJitCodeInfo MulticoreJitRecorder::RequestMethodCode(MethodDesc * pMetho

if (!codeInfo.IsNull() && pManager->IsRecorderActive()) // recorder may be off when player is on (e.g. for Appx)
{
RecordMethodJit(pMethod, false, true); // JITTed by background thread, returned to application
RecordMethodJitOrLoad(pMethod, false); // JITTed by background thread, returned to application
}

return codeInfo;
Expand Down Expand Up @@ -1427,32 +1426,15 @@ MulticoreJitCodeInfo MulticoreJitManager::RequestMethodCode(MethodDesc * pMethod
// Call back from MethodDesc::MakeJitWorker for
// Threading: protected by m_playerLock

void MulticoreJitManager::RecordMethodJit(MethodDesc * pMethod)
void MulticoreJitManager::RecordMethodJitOrLoad(MethodDesc * pMethod)
{
STANDARD_VM_CONTRACT;

CrstHolder hold(& m_playerLock);

if (m_pMulticoreJitRecorder != NULL)
{
m_pMulticoreJitRecorder->RecordMethodJit(pMethod, true, true);

if (m_pMulticoreJitRecorder->IsAtFullCapacity())
{
m_fRecorderActive = false;
}
}
}

void MulticoreJitManager::RecordMethodLoad(MethodDesc * pMethod)
{
STANDARD_VM_CONTRACT;

CrstHolder hold(& m_playerLock);

if (m_pMulticoreJitRecorder != NULL)
{
m_pMulticoreJitRecorder->RecordMethodJit(pMethod, true, false);
m_pMulticoreJitRecorder->RecordMethodJitOrLoad(pMethod, true);

if (m_pMulticoreJitRecorder->IsAtFullCapacity())
{
Expand Down
4 changes: 1 addition & 3 deletions src/coreclr/vm/multicorejit.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ struct MulticoreJitPlayerStat
unsigned short m_nTotalDelay;
unsigned short m_nDelayCount;
unsigned short m_nWalkBack;
unsigned short m_nNoJitCount;

HRESULT m_hr;

Expand Down Expand Up @@ -282,8 +281,7 @@ class MulticoreJitManager

MulticoreJitCodeInfo RequestMethodCode(MethodDesc * pMethod);

void RecordMethodJit(MethodDesc * pMethod);
void RecordMethodLoad(MethodDesc * pMethod);
void RecordMethodJitOrLoad(MethodDesc * pMethod);

MulticoreJitPlayerStat & GetStats()
{
Expand Down
20 changes: 7 additions & 13 deletions src/coreclr/vm/multicorejitimpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@
// Bits 0xff0000 are reserved method flags. Currently only first bit is used.
const unsigned METHOD_FLAGS_MASK = 0xff0000;
const unsigned JIT_BY_APP_THREAD_TAG = 0x10000; // tag, that indicates whether method is jitted by application thread(1) or background thread(0)
const unsigned NO_JIT_TAG = 0x20000; // tag, that indicates whether method should be jitted or simply loaded (i.e. related types are created, etc.)
// Tags 0xfc0000 are currently free
// Tags 0xfe0000 are currently free

const unsigned RECORD_TYPE_OFFSET = 24; // offset of type of record

Expand All @@ -46,7 +45,7 @@ const int MAX_WALKBACK = 128;

enum
{
MULTICOREJIT_PROFILE_VERSION = 103,
MULTICOREJIT_PROFILE_VERSION = 102,

MULTICOREJIT_HEADER_RECORD_ID = 1,
MULTICOREJIT_MODULE_RECORD_ID = 2,
Expand Down Expand Up @@ -298,8 +297,8 @@ friend class MulticoreJitRecorder;

HRESULT HandleModuleRecord(const ModuleRecord * pMod);
HRESULT HandleModuleInfoRecord(unsigned moduleTo, unsigned level);
HRESULT HandleNonGenericMethodInfoRecord(unsigned moduleIndex, unsigned token, bool dojit);
HRESULT HandleGenericMethodInfoRecord(unsigned moduleIndex, BYTE * signature, unsigned length, bool dojit);
HRESULT HandleNonGenericMethodInfoRecord(unsigned moduleIndex, unsigned token);
HRESULT HandleGenericMethodInfoRecord(unsigned moduleIndex, BYTE * signature, unsigned length);
void CompileMethodInfoRecord(Module *pModule, MethodDesc *pMethod, bool isGeneric);

bool CompileMethodDesc(Module * pModule, MethodDesc * pMD);
Expand Down Expand Up @@ -560,7 +559,7 @@ struct RecorderInfo
_ASSERTE(IsFullyInitialized());
}

void PackMethod(unsigned moduleIndex, MethodDesc * pMethod, bool application, bool dojit)
void PackMethod(unsigned moduleIndex, MethodDesc * pMethod, bool application)
{
LIMITED_METHOD_CONTRACT;

Expand All @@ -587,11 +586,6 @@ struct RecorderInfo
data1 |= JIT_BY_APP_THREAD_TAG;
}

if (!dojit)
{
data1 |= NO_JIT_TAG;
}

data2 = 0;
// To avoid recording overhead, records only pointer to MethodDesc.
ptr = (BYTE *) pMethod;
Expand Down Expand Up @@ -645,7 +639,7 @@ class MulticoreJitRecorder

HRESULT WriteModuleRecord(IStream * pStream, const RecorderModuleInfo & module);

void RecordMethodInfo(unsigned moduleIndex, MethodDesc * pMethod, bool application, bool dojit);
void RecordMethodInfo(unsigned moduleIndex, MethodDesc * pMethod, bool application);
unsigned RecordModuleInfo(Module * pModule);
void RecordOrUpdateModuleInfo(FileLoadLevel needLevel, unsigned moduleIndex);

Expand Down Expand Up @@ -711,7 +705,7 @@ class MulticoreJitRecorder
(m_ModuleCount >= MAX_MODULES);
}

void RecordMethodJit(MethodDesc * pMethod, bool application, bool dojit);
void RecordMethodJitOrLoad(MethodDesc * pMethod, bool application);

MulticoreJitCodeInfo RequestMethodCode(MethodDesc * pMethod, MulticoreJitManager * pManager);

Expand Down
34 changes: 8 additions & 26 deletions src/coreclr/vm/multicorejitplayer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ DomainAssembly * MulticoreJitProfilePlayer::LoadAssembly(SString & assemblyName)
FALSE); // Don't throw on FileNotFound.
}

HRESULT MulticoreJitProfilePlayer::HandleNonGenericMethodInfoRecord(unsigned moduleIndex, unsigned token, bool dojit)
HRESULT MulticoreJitProfilePlayer::HandleNonGenericMethodInfoRecord(unsigned moduleIndex, unsigned token)
{
STANDARD_VM_CONTRACT;

Expand All @@ -892,15 +892,7 @@ HRESULT MulticoreJitProfilePlayer::HandleNonGenericMethodInfoRecord(unsigned mod
// Similar to Module::FindMethod + Module::FindMethodThrowing,
// except it calls GetMethodDescFromMemberDefOrRefOrSpec with strictMetadataChecks=FALSE to allow generic instantiation
MethodDesc * pMethod = MemberLoader::GetMethodDescFromMemberDefOrRefOrSpec(pModule, token, NULL, FALSE, FALSE);

if (dojit)
{
CompileMethodInfoRecord(pModule, pMethod, false);
}
else
{
m_stats.m_nNoJitCount++;
}
CompileMethodInfoRecord(pModule, pMethod, false);
}
else
{
Expand All @@ -920,7 +912,7 @@ HRESULT MulticoreJitProfilePlayer::HandleNonGenericMethodInfoRecord(unsigned mod
return hr;
}

HRESULT MulticoreJitProfilePlayer::HandleGenericMethodInfoRecord(unsigned moduleIndex, BYTE * signature, unsigned length, bool dojit)
HRESULT MulticoreJitProfilePlayer::HandleGenericMethodInfoRecord(unsigned moduleIndex, BYTE * signature, unsigned length)
{
STANDARD_VM_CONTRACT;

Expand Down Expand Up @@ -954,14 +946,7 @@ HRESULT MulticoreJitProfilePlayer::HandleGenericMethodInfoRecord(unsigned module
}
EX_END_CATCH(SwallowAllExceptions);

if (dojit)
{
CompileMethodInfoRecord(pModule, pMethod, true);
}
else
{
m_stats.m_nNoJitCount++;
}
CompileMethodInfoRecord(pModule, pMethod, true);
}
else
{
Expand Down Expand Up @@ -1033,17 +1018,16 @@ void MulticoreJitProfilePlayer::TraceSummary()

unsigned compiled = curStorage.GetStored();

MulticoreJitTrace(("PlayerSummary: %d total, %d no jit: %d no mod, %d filtered out, %d had code, %d other, %d tried, %d compiled, %d returned, %d%% efficiency, %d mod loaded, %d ms delay(%d)",
MulticoreJitTrace(("PlayerSummary: %d total: %d no mod, %d filtered out, %d had code, %d other, %d tried, %d compiled, %d returned, %d%% efficiency, %d mod loaded, %d ms delay(%d)",
m_stats.m_nTotalMethod,
m_stats.m_nNoJitCount,
m_stats.m_nMissingModuleSkip,
m_stats.m_nFilteredMethods,
m_stats.m_nHasNativeCode,
m_stats.m_nTotalMethod - m_stats.m_nMissingModuleSkip - m_stats.m_nFilteredMethods - m_stats.m_nHasNativeCode - m_stats.m_nTryCompiling,
m_stats.m_nTryCompiling,
compiled,
returned,
(m_stats.m_nTotalMethod == 0) ? 100 : returned * 100 / (m_stats.m_nTotalMethod - m_stats.m_nNoJitCount),
(m_stats.m_nTotalMethod == 0) ? 100 : returned * 100 / m_stats.m_nTotalMethod,
m_nLoadedModuleCount,
m_stats.m_nTotalDelay,
m_stats.m_nDelayCount
Expand Down Expand Up @@ -1309,22 +1293,20 @@ HRESULT MulticoreJitProfilePlayer::PlayProfile()
unsigned curdata1 = * (const unsigned *) pCurBuf;
unsigned currcdTyp = curdata1 >> RECORD_TYPE_OFFSET;
unsigned curmoduleIndex = curdata1 & MODULE_MASK;
unsigned curflags = curdata1 & METHOD_FLAGS_MASK;
bool dojit = (curflags & NO_JIT_TAG) == 0;

if (currcdTyp == MULTICOREJIT_METHOD_RECORD_ID)
{
unsigned token = * (((const unsigned *) pCurBuf) + 1);

hr = HandleNonGenericMethodInfoRecord(curmoduleIndex, token, dojit);
hr = HandleNonGenericMethodInfoRecord(curmoduleIndex, token);
}
else
{
_ASSERTE(currcdTyp == MULTICOREJIT_GENERICMETHOD_RECORD_ID);

unsigned cursignatureLength = * (const unsigned short *) (((const unsigned *) pCurBuf) + 1);

hr = HandleGenericMethodInfoRecord(curmoduleIndex, (BYTE *) (pCurBuf + sizeof(unsigned) + sizeof(unsigned short)), cursignatureLength, dojit);
hr = HandleGenericMethodInfoRecord(curmoduleIndex, (BYTE *) (pCurBuf + sizeof(unsigned) + sizeof(unsigned short)), cursignatureLength);
}

if (SUCCEEDED(hr) && ShouldAbort(false))
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/prestub.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ PCODE MethodDesc::GetPrecompiledCode(PrepareCodeConfig* pConfig, bool shouldTier
{
if (MulticoreJitManager::IsMethodSupported(this))
{
mcJitManager.RecordMethodLoad(this);
mcJitManager.RecordMethodJitOrLoad(this);
}
}
}
Expand Down Expand Up @@ -1152,7 +1152,7 @@ PCODE MethodDesc::JitCompileCodeLocked(PrepareCodeConfig* pConfig, JitListLockEn
{
if (MulticoreJitManager::IsMethodSupported(this))
{
mcJitManager.RecordMethodJit(this); // Tell multi-core JIT manager to record method on successful JITting
mcJitManager.RecordMethodJitOrLoad(this); // Tell multi-core JIT manager to record method on successful JITting
}
}
}
Expand Down

0 comments on commit 2d7eede

Please sign in to comment.