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

fix scenario function table deletion is not transferred to background thread #4560

Merged
merged 1 commit into from
Jan 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion lib/Backend/CodeGenWorkItem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,22 @@ void CodeGenWorkItem::OnWorkItemProcessFail(NativeCodeGenerator* codeGen)
#if DBG
this->allocation->allocation->isNotExecutableBecauseOOM = true;
#endif
codeGen->FreeNativeCodeGenAllocation(this->allocation->allocation->address, nullptr);

#if PDATA_ENABLED & defined(_WIN32)
if (this->entryPointInfo && this->entryPointInfo->GetXDataInfo())
{
void* functionTable = this->entryPointInfo->GetXDataInfo()->functionTable;
if (functionTable)
{
if (!DelayDeletingFunctionTable::AddEntry(functionTable))
{
PHASE_PRINT_TESTTRACE1(Js::XDataPhase, _u("OnWorkItemProcessFail: Failed to add to slist, table: %llx\n"), functionTable);
DelayDeletingFunctionTable::DeleteFunctionTable(functionTable);
}
}
}
#endif
codeGen->FreeNativeCodeGenAllocation(this->allocation->allocation->address);
}
}

Expand Down
12 changes: 3 additions & 9 deletions lib/Backend/EmitBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ template <typename TAlloc, typename TPreReservedAlloc, class SyncObject>
void
EmitBufferManager<TAlloc, TPreReservedAlloc, SyncObject>::FreeAllocations(bool release)
{
DelayDeletingFunctionTable::Clear();
AutoRealOrFakeCriticalSection<SyncObject> autoCs(&this->criticalSection);

#if DBG_DUMP
Expand Down Expand Up @@ -191,9 +192,10 @@ EmitBufferManager<TAlloc, TPreReservedAlloc, SyncObject>::NewAllocation(size_t b

template <typename TAlloc, typename TPreReservedAlloc, class SyncObject>
bool
EmitBufferManager<TAlloc, TPreReservedAlloc, SyncObject>::FreeAllocation(void* address, void** functionTable)
EmitBufferManager<TAlloc, TPreReservedAlloc, SyncObject>::FreeAllocation(void* address)
{
AutoRealOrFakeCriticalSection<SyncObject> autoCs(&this->criticalSection);

#if _M_ARM
address = (void*)((uintptr_t)address & ~0x1); // clear the thumb bit
#endif
Expand Down Expand Up @@ -243,14 +245,6 @@ EmitBufferManager<TAlloc, TPreReservedAlloc, SyncObject>::FreeAllocation(void* a
}
VerboseHeapTrace(_u("Freeing 0x%p, allocation: 0x%p\n"), address, allocation->allocation->address);

#if PDATA_ENABLED && defined(_WIN32)
if (functionTable && *functionTable)
{
NtdllLibrary::Instance->DeleteGrowableFunctionTable(*functionTable);
*functionTable = nullptr;
}
#endif

this->allocationHeap.Free(allocation->allocation);
this->allocator->Free(allocation, sizeof(TEmitBufferAllocation));

Expand Down
2 changes: 1 addition & 1 deletion lib/Backend/EmitBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class EmitBufferManager
bool ProtectBufferWithExecuteReadWriteForInterpreter(TEmitBufferAllocation* allocation);
bool CommitBufferForInterpreter(TEmitBufferAllocation* allocation, _In_reads_bytes_(bufferSize) BYTE* pBuffer, _In_ size_t bufferSize);
void CompletePreviousAllocation(TEmitBufferAllocation* allocation);
bool FreeAllocation(void* address, void** functionTable);
bool FreeAllocation(void* address);
//Ends here

bool IsInHeap(void* address);
Expand Down
32 changes: 11 additions & 21 deletions lib/Backend/NativeCodeGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,7 @@ NativeCodeGenerator::CodeGen(PageAllocator * pageAllocator, CodeGenWorkItem* wor
}

#if defined(TARGET_64)
DelayDeletingFunctionTable::Clear();
XDataAllocation * xdataInfo = HeapNewZ(XDataAllocation);
xdataInfo->address = (byte*)jitWriteData.xdataAddr;
XDataAllocator::Register(xdataInfo, jitWriteData.codeAddress, jitWriteData.codeSize);
Expand Down Expand Up @@ -3206,14 +3207,14 @@ NativeCodeGenerator::EnterScriptStart()
}

void
FreeNativeCodeGenAllocation(Js::ScriptContext *scriptContext, Js::JavascriptMethod codeAddress, Js::JavascriptMethod thunkAddress, void** functionTable)
FreeNativeCodeGenAllocation(Js::ScriptContext *scriptContext, Js::JavascriptMethod codeAddress, Js::JavascriptMethod thunkAddress)
{
if (!scriptContext->GetNativeCodeGenerator())
{
return;
}

scriptContext->GetNativeCodeGenerator()->QueueFreeNativeCodeGenAllocation((void*)codeAddress, (void*)thunkAddress, functionTable);
scriptContext->GetNativeCodeGenerator()->QueueFreeNativeCodeGenAllocation((void*)codeAddress, (void*)thunkAddress);
}

bool TryReleaseNonHiPriWorkItem(Js::ScriptContext* scriptContext, CodeGenWorkItem* workItem)
Expand Down Expand Up @@ -3244,30 +3245,22 @@ bool NativeCodeGenerator::TryReleaseNonHiPriWorkItem(CodeGenWorkItem* workItem)
}

void
NativeCodeGenerator::FreeNativeCodeGenAllocation(void* codeAddress, void** functionTable)
NativeCodeGenerator::FreeNativeCodeGenAllocation(void* codeAddress)
{
if (JITManager::GetJITManager()->IsOOPJITEnabled())
{
// function table delete in content process
#if PDATA_ENABLED && defined(_WIN32)
if (functionTable && *functionTable)
{
NtdllLibrary::Instance->DeleteGrowableFunctionTable(*functionTable);
*functionTable = nullptr;
}
#endif
ThreadContext * context = this->scriptContext->GetThreadContext();
HRESULT hr = JITManager::GetJITManager()->FreeAllocation(context->GetRemoteThreadContextAddr(), (intptr_t)codeAddress);
JITManager::HandleServerCallResult(hr, RemoteCallType::MemFree);
}
else if(this->backgroundAllocators)
{
this->backgroundAllocators->emitBufferManager.FreeAllocation(codeAddress, functionTable);
this->backgroundAllocators->emitBufferManager.FreeAllocation(codeAddress);
}
}

void
NativeCodeGenerator::QueueFreeNativeCodeGenAllocation(void* codeAddress, void * thunkAddress, void** functionTable)
NativeCodeGenerator::QueueFreeNativeCodeGenAllocation(void* codeAddress, void * thunkAddress)
{
ASSERT_THREAD();

Expand Down Expand Up @@ -3297,24 +3290,24 @@ NativeCodeGenerator::QueueFreeNativeCodeGenAllocation(void* codeAddress, void *
// OOP JIT will always queue a job

// The foreground allocators may have been used
if (this->foregroundAllocators && this->foregroundAllocators->emitBufferManager.FreeAllocation(codeAddress, functionTable))
if (this->foregroundAllocators && this->foregroundAllocators->emitBufferManager.FreeAllocation(codeAddress))
{
return;
}

// The background allocators were used. Queue a job to free the allocation from the background thread.
this->freeLoopBodyManager.QueueFreeLoopBodyJob(codeAddress, thunkAddress, functionTable);
this->freeLoopBodyManager.QueueFreeLoopBodyJob(codeAddress, thunkAddress);
}

void NativeCodeGenerator::FreeLoopBodyJobManager::QueueFreeLoopBodyJob(void* codeAddress, void * thunkAddress, void** functionTable)
void NativeCodeGenerator::FreeLoopBodyJobManager::QueueFreeLoopBodyJob(void* codeAddress, void * thunkAddress)
{
Assert(!this->isClosed);

FreeLoopBodyJob* job = HeapNewNoThrow(FreeLoopBodyJob, this, codeAddress, thunkAddress, *functionTable);
FreeLoopBodyJob* job = HeapNewNoThrow(FreeLoopBodyJob, this, codeAddress, thunkAddress);

if (job == nullptr)
{
FreeLoopBodyJob stackJob(this, codeAddress, thunkAddress, *functionTable, false /* heapAllocated */);
FreeLoopBodyJob stackJob(this, codeAddress, thunkAddress, false /* heapAllocated */);

{
AutoOptionalCriticalSection lock(Processor()->GetCriticalSection());
Expand All @@ -3338,9 +3331,6 @@ void NativeCodeGenerator::FreeLoopBodyJobManager::QueueFreeLoopBodyJob(void* cod
HeapDelete(job);
}
}

// function table successfully transferred to background job
*functionTable = nullptr;
}

#ifdef PROFILE_EXEC
Expand Down
13 changes: 5 additions & 8 deletions lib/Backend/NativeCodeGenerator.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ void SetProfileMode(BOOL fSet);
void UpdateQueueForDebugMode();
bool IsBackgroundJIT() const;
void EnterScriptStart();
void FreeNativeCodeGenAllocation(void* codeAddress, void** functionTable);
void FreeNativeCodeGenAllocation(void* codeAddress);
bool TryReleaseNonHiPriWorkItem(CodeGenWorkItem* workItem);

void QueueFreeNativeCodeGenAllocation(void* codeAddress, void* thunkAddress, void** functionTable);
void QueueFreeNativeCodeGenAllocation(void* codeAddress, void* thunkAddress);

bool IsClosed() { return isClosed; }
void AddWorkItem(CodeGenWorkItem* workItem);
Expand Down Expand Up @@ -201,19 +201,17 @@ void SetProfileMode(BOOL fSet);
class FreeLoopBodyJob: public JsUtil::Job
{
public:
FreeLoopBodyJob(JsUtil::JobManager *const manager, void* codeAddress, void* thunkAddress, void* functionTable, bool isHeapAllocated = true):
FreeLoopBodyJob(JsUtil::JobManager *const manager, void* codeAddress, void* thunkAddress, bool isHeapAllocated = true):
JsUtil::Job(manager),
codeAddress(codeAddress),
thunkAddress(thunkAddress),
functionTable(functionTable),
heapAllocated(isHeapAllocated)
{
}

bool heapAllocated;
void* codeAddress;
void* thunkAddress;
void* functionTable;
};

class FreeLoopBodyJobManager sealed: public WaitableJobManager
Expand Down Expand Up @@ -281,9 +279,8 @@ void SetProfileMode(BOOL fSet);
{
FreeLoopBodyJob* freeLoopBodyJob = static_cast<FreeLoopBodyJob*>(job);

void* functionTable = freeLoopBodyJob->functionTable;
// Free Loop Body
nativeCodeGen->FreeNativeCodeGenAllocation(freeLoopBodyJob->codeAddress, &functionTable);
nativeCodeGen->FreeNativeCodeGenAllocation(freeLoopBodyJob->codeAddress);

return true;
}
Expand All @@ -306,7 +303,7 @@ void SetProfileMode(BOOL fSet);
}
}

void QueueFreeLoopBodyJob(void* codeAddress, void* thunkAddress, void** functionTable);
void QueueFreeLoopBodyJob(void* codeAddress, void* thunkAddress);

private:
NativeCodeGenerator* nativeCodeGen;
Expand Down
2 changes: 1 addition & 1 deletion lib/Backend/PDataManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void PDataManager::UnregisterPdata(RUNTIME_FUNCTION* pdata)
if (AutoSystemInfo::Data.IsWin8OrLater())
{
// TODO: need to move to background?
NtdllLibrary::Instance->DeleteGrowableFunctionTable(pdata);
DelayDeletingFunctionTable::DeleteFunctionTable(pdata);
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion lib/Common/BackendApi.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ void UpdateNativeCodeGeneratorForDebugMode(NativeCodeGenerator* nativeCodeGen);
CriticalSection *GetNativeCodeGenCriticalSection(NativeCodeGenerator *pNativeCodeGen);
bool TryReleaseNonHiPriWorkItem(Js::ScriptContext* scriptContext, CodeGenWorkItem* workItem);
void NativeCodeGenEnterScriptStart(NativeCodeGenerator * nativeCodeGen);
void FreeNativeCodeGenAllocation(Js::ScriptContext* scriptContext, Js::JavascriptMethod codeAddress, Js::JavascriptMethod thunkAddress, void** functionTable);
void FreeNativeCodeGenAllocation(Js::ScriptContext* scriptContext, Js::JavascriptMethod codeAddress, Js::JavascriptMethod thunkAddress);
InProcCodeGenAllocators* GetForegroundAllocator(NativeCodeGenerator * nativeCodeGen, PageAllocator* pageallocator);
void GenerateFunction(NativeCodeGenerator * nativeCodeGen, Js::FunctionBody * functionBody, Js::ScriptFunction * function = NULL);
void GenerateLoopBody(NativeCodeGenerator * nativeCodeGen, Js::FunctionBody * functionBody, Js::LoopHeader * loopHeader, Js::EntryPointInfo* entryPointInfo, uint localCount, Js::Var localSlots[]);
Expand Down
1 change: 1 addition & 0 deletions lib/Common/ConfigFlagsList.h
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ PHASE(All)
PHASE(DeferSourceLoad)
PHASE(ObjectMutationBreakpoint)
PHASE(NativeCodeData)
PHASE(XData)
#undef PHASE
#endif

Expand Down
9 changes: 8 additions & 1 deletion lib/Common/Core/DelayLoadLibrary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,17 @@ DWORD NtdllLibrary::AddGrowableFunctionTable( _Out_ PVOID * DynamicTable,
return 1;
}
}
return addGrowableFunctionTable(DynamicTable,
DWORD status = addGrowableFunctionTable(DynamicTable,
FunctionTable,
EntryCount,
MaximumEntryCount,
RangeBase,
RangeEnd);
#if _M_X64
PHASE_PRINT_TESTTRACE1(Js::XDataPhase, _u("Register: Begin: %llx, End: %x, Unwind: %llx, RangeBase: %llx, RangeEnd: %llx, table: %llx, Status: %x\n"),
FunctionTable->BeginAddress, FunctionTable->EndAddress, FunctionTable->UnwindInfoAddress, RangeBase, RangeEnd, *DynamicTable, status);
#endif
return status;
}
return 1;
}
Expand All @@ -107,6 +112,8 @@ VOID NtdllLibrary::DeleteGrowableFunctionTable( _In_ PVOID DynamicTable )
}
}
deleteGrowableFunctionTable(DynamicTable);

PHASE_PRINT_TESTTRACE1(Js::XDataPhase, _u("UnRegister: table: %llx\n"), DynamicTable);
}
}

Expand Down
9 changes: 8 additions & 1 deletion lib/Common/Memory/DelayDeletingFunctionTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void DelayDeletingFunctionTable::Clear()
while (entry)
{
FunctionTableNode* list = (FunctionTableNode*)entry;
NtdllLibrary::Instance->DeleteGrowableFunctionTable(list->functionTable);
DeleteFunctionTable(list->functionTable);
_aligned_free(entry);
entry = InterlockedPopEntrySList(Head);
}
Expand All @@ -79,4 +79,11 @@ bool DelayDeletingFunctionTable::IsEmpty()
#else
return true;
#endif
}

void DelayDeletingFunctionTable::DeleteFunctionTable(void* functionTable)
{
#if PDATA_ENABLED && defined(_WIN32)
NtdllLibrary::Instance->DeleteGrowableFunctionTable(functionTable);
#endif
}
1 change: 1 addition & 0 deletions lib/Common/Memory/XDataAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,5 @@ struct DelayDeletingFunctionTable
static bool AddEntry(FunctionTableHandle ft);
static void Clear();
static bool IsEmpty();
static void DeleteFunctionTable(void* functionTable);
};
5 changes: 1 addition & 4 deletions lib/Common/Memory/amd64/XDataAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,7 @@ void XDataAllocator::Register(XDataAllocation * xdataInfo, ULONG_PTR functionSta
/*RangeBase*/ functionStart,
/*RangeEnd*/ functionStart + functionSize);
success = NT_SUCCESS(status);
if (success)
{
Assert(xdataInfo->functionTable != nullptr);
}
Assert(success && xdataInfo->functionTable != nullptr);
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion lib/JITServer/JITServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ ServerFreeAllocation(

return ServerCallWrapper(context, [&]()->HRESULT
{
context->GetCodeGenAllocators()->emitBufferManager.FreeAllocation((void*)codeAddress, nullptr);
context->GetCodeGenAllocators()->emitBufferManager.FreeAllocation((void*)codeAddress);
return S_OK;
});
}
Expand Down
Loading