Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT into scratch buffer #53173

Merged
merged 9 commits into from
May 26, 2021
Merged
86 changes: 56 additions & 30 deletions src/coreclr/vm/codeman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2592,18 +2592,20 @@ void* EEJitManager::allocCodeRaw(CodeHeapRequestInfo *pInfo,
RETURN(mem);
}

CodeHeader* EEJitManager::allocCode(MethodDesc* pMD, size_t blockSize, size_t reserveForJumpStubs, CorJitAllocMemFlag flag
void EEJitManager::allocCode(MethodDesc* pMD, size_t blockSize, size_t reserveForJumpStubs, CorJitAllocMemFlag flag, CodeHeader** ppCodeHeader, CodeHeader** ppCodeHeaderRW,
size_t* pAllocatedSize, HeapList** ppCodeHeap
#ifdef USE_INDIRECT_CODEHEADER
, BYTE** ppRealHeader
#endif
#ifdef FEATURE_EH_FUNCLETS
, UINT nUnwindInfos
, TADDR * pModuleBase
, UINT nUnwindInfos
#endif
)
)
{
CONTRACT(CodeHeader *) {
CONTRACTL {
THROWS;
GC_NOTRIGGER;
POSTCONDITION(CheckPointer(RETVAL));
} CONTRACT_END;
} CONTRACTL_END;

//
// Alignment
Expand Down Expand Up @@ -2636,6 +2638,7 @@ CodeHeader* EEJitManager::allocCode(MethodDesc* pMD, size_t blockSize, size_t re
SIZE_T totalSize = blockSize;

CodeHeader * pCodeHdr = NULL;
CodeHeader * pCodeHdrRW = NULL;

CodeHeapRequestInfo requestInfo(pMD);
#if defined(FEATURE_JIT_PITCHING)
Expand Down Expand Up @@ -2663,11 +2666,9 @@ CodeHeader* EEJitManager::allocCode(MethodDesc* pMD, size_t blockSize, size_t re
{
CrstHolder ch(&m_CodeHeapCritSec);

HeapList *pCodeHeap = NULL;

TADDR pCode = (TADDR) allocCodeRaw(&requestInfo, sizeof(CodeHeader), totalSize, alignment, &pCodeHeap);

_ASSERTE(pCodeHeap);
*ppCodeHeap = NULL;
TADDR pCode = (TADDR) allocCodeRaw(&requestInfo, sizeof(CodeHeader), totalSize, alignment, ppCodeHeap);
_ASSERTE(*ppCodeHeap);

if (pMD->IsLCGMethod())
{
Expand All @@ -2676,40 +2677,53 @@ CodeHeader* EEJitManager::allocCode(MethodDesc* pMD, size_t blockSize, size_t re

_ASSERTE(IS_ALIGNED(pCode, alignment));

// Initialize the CodeHeader *BEFORE* we publish this code range via the nibble
// map so that we don't have to harden readers against uninitialized data.
// However because we hold the lock, this initialization should be fast and cheap!

pCodeHdr = ((CodeHeader *)pCode) - 1;

*pAllocatedSize = sizeof(CodeHeader) + totalSize;
#ifdef FEATURE_WXORX
pCodeHdrRW = (CodeHeader *)new BYTE[*pAllocatedSize];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allocation has potential real cost. It may be nice to omit it in this change (together with the copy at the end) and only do it when W^X is turned on so that we can include it in the perf measurements.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have measured the perf with this change using ASPNet benchmarks in our lab on Windows and haven't seen any perf difference. Running 10 rounds of PlainText benchmark that I have found to be quite sensitive to the JIT performance with R2R disabled with and without my changes show the same average results.
I have actually intentionally made this change separate from the W^X to see its effect isolated from the other W^X changes and verify that it doesn't have possible negative impact on some other tests.
Here are the results of time to first request:

State
With this change 174 166 165 171 167 174 171 173 167 166
Before 166 171 165 174 165 166 168 165 169 165

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That depends on how much JITing in parallel is going on and how big the methods are. PlainText does not have too much parallel JITIng and the methods are not big either.

We have seen the JIT being sensitive to performance of unmanaged memory allocations and it is why we have cache of memory blocks to be used by the JIT in JitHost (slabAllocator).

#else
pCodeHdrRW = pCodeHdr;
#endif

#ifdef USE_INDIRECT_CODEHEADER
if (requestInfo.IsDynamicDomain())
{
pCodeHdr->SetRealCodeHeader((BYTE*)pCode + ALIGN_UP(blockSize, sizeof(void*)));
// Set the real code header to the writeable mapping so that we can set its members via the CodeHeader methods below
pCodeHdrRW->SetRealCodeHeader((BYTE *)(pCodeHdrRW + 1) + ALIGN_UP(blockSize, sizeof(void*)));
}
else
{
// TODO: think about the CodeHeap carrying around a RealCodeHeader chunking mechanism
//
// allocate the real header in the low frequency heap
BYTE* pRealHeader = (BYTE*)(void*)pMD->GetLoaderAllocator()->GetLowFrequencyHeap()->AllocMem(S_SIZE_T(realHeaderSize));
pCodeHdr->SetRealCodeHeader(pRealHeader);
pCodeHdrRW->SetRealCodeHeader(pRealHeader);
}
#endif

pCodeHdr->SetDebugInfo(NULL);
pCodeHdr->SetEHInfo(NULL);
pCodeHdr->SetGCInfo(NULL);
pCodeHdr->SetMethodDesc(pMD);
pCodeHdrRW->SetDebugInfo(NULL);
pCodeHdrRW->SetEHInfo(NULL);
pCodeHdrRW->SetGCInfo(NULL);
pCodeHdrRW->SetMethodDesc(pMD);
#ifdef FEATURE_EH_FUNCLETS
pCodeHdr->SetNumberOfUnwindInfos(nUnwindInfos);
*pModuleBase = pCodeHeap->GetModuleBase();
pCodeHdrRW->SetNumberOfUnwindInfos(nUnwindInfos);
#endif

NibbleMapSet(pCodeHeap, pCode, TRUE);
#ifdef USE_INDIRECT_CODEHEADER
if (requestInfo.IsDynamicDomain())
{
*ppRealHeader = (BYTE*)pCode + ALIGN_UP(blockSize, sizeof(void*));
}
else
{
*ppRealHeader = NULL;
}
#endif // USE_INDIRECT_CODEHEADER
}

RETURN(pCodeHdr);
*ppCodeHeader = pCodeHdr;
*ppCodeHeaderRW = pCodeHdrRW;
}

EEJitManager::DomainCodeHeapList *EEJitManager::GetCodeHeapList(CodeHeapRequestInfo *pInfo, LoaderAllocator *pAllocator, BOOL fDynamicOnly)
Expand Down Expand Up @@ -2995,7 +3009,7 @@ JumpStubBlockHeader * EEJitManager::allocJumpStubBlock(MethodDesc* pMD, DWORD n
CodeHeader * pCodeHdr = (CodeHeader *) (mem - sizeof(CodeHeader));
pCodeHdr->SetStubCodeBlockKind(STUB_CODE_BLOCK_JUMPSTUB);

NibbleMapSet(pCodeHeap, mem, TRUE);
NibbleMapSetUnlocked(pCodeHeap, mem, TRUE);

pBlock = (JumpStubBlockHeader *)mem;

Expand Down Expand Up @@ -3046,7 +3060,7 @@ void * EEJitManager::allocCodeFragmentBlock(size_t blockSize, unsigned alignment
CodeHeader * pCodeHdr = (CodeHeader *) (mem - sizeof(CodeHeader));
pCodeHdr->SetStubCodeBlockKind(kind);

NibbleMapSet(pCodeHeap, mem, TRUE);
NibbleMapSetUnlocked(pCodeHeap, mem, TRUE);

// Record the jump stub reservation
pCodeHeap->reserveForJumpStubs += requestInfo.getReserveForJumpStubs();
Expand Down Expand Up @@ -3224,7 +3238,7 @@ void EEJitManager::RemoveJitData (CodeHeader * pCHdr, size_t GCinfo_len, size_t
if (pHp == NULL)
return;

NibbleMapSet(pHp, (TADDR)(pCHdr + 1), FALSE);
NibbleMapSetUnlocked(pHp, (TADDR)(pCHdr + 1), FALSE);
}

// Backout the GCInfo
Expand Down Expand Up @@ -3386,7 +3400,7 @@ void EEJitManager::FreeCodeMemory(HostCodeHeap *pCodeHeap, void * codeStart)
// so pCodeHeap can only be a HostCodeHeap.

// clean up the NibbleMap
NibbleMapSet(pCodeHeap->m_pHeapList, (TADDR)codeStart, FALSE);
NibbleMapSetUnlocked(pCodeHeap->m_pHeapList, (TADDR)codeStart, FALSE);

// The caller of this method doesn't call HostCodeHeap->FreeMemForCode
// directly because the operation should be protected by m_CodeHeapCritSec.
Expand Down Expand Up @@ -3850,13 +3864,25 @@ TADDR EEJitManager::FindMethodCode(RangeSection * pRangeSection, PCODE currentPC
}

#if !defined(DACCESS_COMPILE)

void EEJitManager::NibbleMapSet(HeapList * pHp, TADDR pCode, BOOL bSet)
{
CONTRACTL {
NOTHROW;
GC_NOTRIGGER;
} CONTRACTL_END;

CrstHolder ch(&m_CodeHeapCritSec);
NibbleMapSetUnlocked(pHp, pCode, bSet);
}

void EEJitManager::NibbleMapSetUnlocked(HeapList * pHp, TADDR pCode, BOOL bSet)
{
CONTRACTL {
NOTHROW;
GC_NOTRIGGER;
} CONTRACTL_END;

// Currently all callers to this method ensure EEJitManager::m_CodeHeapCritSec
// is held.
_ASSERTE(m_CodeHeapCritSec.OwnedByCurrentThread());
Expand Down
18 changes: 11 additions & 7 deletions src/coreclr/vm/codeman.h
Original file line number Diff line number Diff line change
Expand Up @@ -461,9 +461,9 @@ class CodeHeap
// The number of code heaps at which we increase the size of new code heaps.
#define CODE_HEAP_SIZE_INCREASE_THRESHOLD 5

typedef DPTR(struct _HeapList) PTR_HeapList;
typedef DPTR(struct HeapList) PTR_HeapList;

typedef struct _HeapList
struct HeapList
{
PTR_HeapList hpNext;

Expand Down Expand Up @@ -497,7 +497,7 @@ typedef struct _HeapList
void SetNext(PTR_HeapList next)
{ hpNext = next; }

} HeapList;
};

//-----------------------------------------------------------------------------
// Implementation of the standard CodeHeap.
Expand Down Expand Up @@ -974,12 +974,15 @@ class EEJitManager : public IJitManager

BOOL LoadJIT();

CodeHeader* allocCode(MethodDesc* pFD, size_t blockSize, size_t reserveForJumpStubs, CorJitAllocMemFlag flag
void allocCode(MethodDesc* pFD, size_t blockSize, size_t reserveForJumpStubs, CorJitAllocMemFlag flag, CodeHeader** ppCodeHeader, CodeHeader** ppCodeHeaderRW,
size_t* pAllocatedSize, HeapList** ppCodeHeap
#ifdef USE_INDIRECT_CODEHEADER
, BYTE** ppRealHeader
#endif
#ifdef FEATURE_EH_FUNCLETS
, UINT nUnwindInfos
, TADDR * pModuleBase
, UINT nUnwindInfos
#endif
);
);
BYTE * allocGCInfo(CodeHeader* pCodeHeader, DWORD blockSize, size_t * pAllocationSize);
EE_ILEXCEPTION* allocEHInfo(CodeHeader* pCodeHeader, unsigned numClauses, size_t * pAllocationSize);
JumpStubBlockHeader* allocJumpStubBlock(MethodDesc* pMD, DWORD numJumps,
Expand Down Expand Up @@ -1024,6 +1027,7 @@ class EEJitManager : public IJitManager
#ifndef DACCESS_COMPILE
// Heap Management functions
void NibbleMapSet(HeapList * pHp, TADDR pCode, BOOL bSet);
void NibbleMapSetUnlocked(HeapList * pHp, TADDR pCode, BOOL bSet);
#endif // !DACCESS_COMPILE

static TADDR FindMethodCode(RangeSection * pRangeSection, PCODE currentPC);
Expand Down
Loading