Skip to content

Move the thread alloc context off of Thread in CoreCLR #103055

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

Merged
merged 20 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
86e6837
Move the thread alloc context off of Thread in CoreCLR and NativeAOT
jkoritzinsky Jun 5, 2024
a96039a
fix typo
jkoritzinsky Jun 5, 2024
e511c4f
Remove extra indirection. use same thread-local model as in other cas…
jkoritzinsky Jun 10, 2024
d81012e
Fix ARM64 asm
jkoritzinsky Jun 10, 2024
c2cbeb6
Fix indexing on x86
jkoritzinsky Jun 11, 2024
71eb0f4
Fix root enumeration and ensure we're initializing/reading the right …
jkoritzinsky Jun 11, 2024
a87c3af
Remove the GetThread call in JIT_NewS_MP_FastPortable as it's unneces…
jkoritzinsky Jun 11, 2024
ce489a8
Remove most of the inline-assembly alloc helpers. These helpers now h…
jkoritzinsky Jun 12, 2024
0d40a74
Merge branch 'alloc-context-no-thread' of https://github.com/jkoritzi…
jkoritzinsky Jun 12, 2024
a4b5ba4
Fix contracts
jkoritzinsky Jun 13, 2024
a8eec35
Use a different reinit mechanism to ensure we're getting the behavior…
jkoritzinsky Jun 13, 2024
190abb5
Now that the alloc context is stored in TLS memory, we must clean it …
jkoritzinsky Jun 13, 2024
f090a19
More null checks and allow a thread to clean itself up in OnThreadTer…
jkoritzinsky Jun 13, 2024
5ed8f7a
Revert NativeAOT changes
jkoritzinsky Jun 14, 2024
6e74fda
Update comment
jkoritzinsky Jun 14, 2024
0ee2b52
Update dacdbiimpl.cpp
jkoritzinsky Jun 14, 2024
d895031
PR feedback
jkoritzinsky Jun 14, 2024
1929904
Merge branch 'alloc-context-no-thread' of https://github.com/jkoritzi…
jkoritzinsky Jun 14, 2024
af270e3
PR feedback
jkoritzinsky Jun 15, 2024
bad14bf
Revert instruction reduction (as it doesn't work with MASM SECTIONREL…
jkoritzinsky Jun 17, 2024
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
12 changes: 10 additions & 2 deletions src/coreclr/debug/daccess/dacdbiimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4713,8 +4713,16 @@ void DacDbiInterfaceImpl::GetThreadAllocInfo(VMPTR_Thread vmThread,

Thread * pThread = vmThread.GetDacPtr();
gc_alloc_context* allocContext = pThread->GetAllocContext();
threadAllocInfo->m_allocBytesSOH = allocContext->alloc_bytes - (allocContext->alloc_limit - allocContext->alloc_ptr);
threadAllocInfo->m_allocBytesUOH = allocContext->alloc_bytes_uoh;
if (allocContext != nullptr)
{
threadAllocInfo->m_allocBytesSOH = allocContext->alloc_bytes - (allocContext->alloc_limit - allocContext->alloc_ptr);
threadAllocInfo->m_allocBytesUOH = allocContext->alloc_bytes_uoh;
}
else
{
threadAllocInfo->m_allocBytesSOH = 0;
threadAllocInfo->m_allocBytesUOH = 0;
}
}

// Set and reset the TSNC_DebuggerUserSuspend bit on the state of the specified thread
Expand Down
28 changes: 24 additions & 4 deletions src/coreclr/debug/daccess/request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -720,8 +720,18 @@ ClrDataAccess::GetThreadAllocData(CLRDATA_ADDRESS addr, struct DacpAllocData *da

Thread* thread = PTR_Thread(TO_TADDR(addr));

data->allocBytes = TO_CDADDR(thread->m_alloc_context.alloc_bytes);
data->allocBytesLoh = TO_CDADDR(thread->m_alloc_context.alloc_bytes_uoh);
gc_alloc_context* pAllocContext = thread->GetAllocContext();

if (pAllocContext != NULL)
{
data->allocBytes = TO_CDADDR(pAllocContext->alloc_bytes);
data->allocBytesLoh = TO_CDADDR(pAllocContext->alloc_bytes_uoh);
}
else
{
data->allocBytes = TO_CDADDR(0);
data->allocBytesLoh = TO_CDADDR(0);
}

SOSDacLeave();
return hr;
Expand Down Expand Up @@ -816,8 +826,18 @@ HRESULT ClrDataAccess::GetThreadDataImpl(CLRDATA_ADDRESS threadAddr, struct Dacp
threadData->osThreadId = (DWORD)thread->m_OSThreadId;
threadData->state = thread->m_State;
threadData->preemptiveGCDisabled = thread->m_fPreemptiveGCDisabled;
threadData->allocContextPtr = TO_CDADDR(thread->m_alloc_context.alloc_ptr);
threadData->allocContextLimit = TO_CDADDR(thread->m_alloc_context.alloc_limit);

gc_alloc_context* allocContext = thread->GetAllocContext();
if (allocContext)
{
threadData->allocContextPtr = TO_CDADDR(allocContext->alloc_ptr);
threadData->allocContextLimit = TO_CDADDR(allocContext->alloc_limit);
}
else
{
threadData->allocContextPtr = TO_CDADDR(0);
threadData->allocContextLimit = TO_CDADDR(0);
}

threadData->fiberData = (CLRDATA_ADDRESS)NULL;

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -621,8 +621,8 @@ if(CLR_CMAKE_TARGET_ARCH_AMD64)
${ARCH_SOURCES_DIR}/GenericComPlusCallStubs.asm
${ARCH_SOURCES_DIR}/getstate.asm
${ARCH_SOURCES_DIR}/JitHelpers_Fast.asm
${ARCH_SOURCES_DIR}/JitHelpers_FastMP.asm
${ARCH_SOURCES_DIR}/JitHelpers_FastWriteBarriers.asm
${ARCH_SOURCES_DIR}/JitHelpers_InlineGetThread.asm
${ARCH_SOURCES_DIR}/JitHelpers_SingleAppDomain.asm
${ARCH_SOURCES_DIR}/JitHelpers_Slow.asm
${ARCH_SOURCES_DIR}/patchedcode.asm
Expand Down
20 changes: 20 additions & 0 deletions src/coreclr/vm/amd64/AsmMacros.inc
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,26 @@ INLINE_GETTHREAD macro Reg

endm

;
; Inlined macro to get the current thread's allocation context
; Trashes rax and r11
;

INLINE_GET_ALLOC_CONTEXT macro Reg

EXTERN _tls_index: DWORD
EXTERN t_thread_alloc_context: DWORD

mov r11d, [_tls_index]
mov rax, gs:[OFFSET__TEB__ThreadLocalStoragePointer]
mov rax, [rax + r11 * 8]
mov r11d, SECTIONREL t_thread_alloc_context
add rax, r11
mov Reg, rax

endm


; if you change this code there will be corresponding code in JITInterfaceGen.cpp which will need to be changed
;

Expand Down
75 changes: 75 additions & 0 deletions src/coreclr/vm/amd64/JitHelpers_FastMP.asm
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
; Licensed to the .NET Foundation under one or more agreements.
; The .NET Foundation licenses this file to you under the MIT license.

; ***********************************************************************
; File: JitHelpers_InlineGetThread.asm, see history in jithelp.asm
;
; ***********************************************************************

include AsmMacros.inc
include asmconstants.inc

CopyValueClassUnchecked equ ?CopyValueClassUnchecked@@YAXPEAX0PEAVMethodTable@@@Z
JIT_Box equ ?JIT_Box@@YAPEAVObject@@PEAUCORINFO_CLASS_STRUCT_@@PEAX@Z

extern CopyValueClassUnchecked:proc
extern JIT_Box:proc

; HCIMPL2(Object*, JIT_Box, CORINFO_CLASS_HANDLE type, void* unboxedData)
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to get rid of this asm implementation and replace it with the portable C++ one. (It can be done in separate change.)

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 was having trouble getting the exact same assembly out of this one with a C++ implementation, so I didn't want to do it in this PR. I'll move it over in another PR and we can review the assembly differences there.

Copy link
Member

@jkotas jkotas Jun 14, 2024

Choose a reason for hiding this comment

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

The box helper is not super perf critical. The JIT will inline it for all cases that matter. I think it would be fine to have something close enough - anything that does not raise HELPER_METHOD_FRAME on a hot path should be ok.

NESTED_ENTRY JIT_BoxFastMP, _TEXT

; m_BaseSize is guaranteed to be a multiple of 8.
mov r8d, [rcx + OFFSET__MethodTable__m_BaseSize]

INLINE_GET_ALLOC_CONTEXT r11
mov r10, [r11 + OFFSETOF__gc_alloc_context__alloc_limit]
mov rax, [r11 + OFFSETOF__gc_alloc_context__alloc_ptr]

add r8, rax

cmp r8, r10
ja AllocFailed

test rdx, rdx
je NullRef

mov [r11 + OFFSETOF__gc_alloc_context__alloc_ptr], r8
mov [rax], rcx

; Check whether the object contains pointers
test dword ptr [rcx + OFFSETOF__MethodTable__m_dwFlags], MethodTable__enum_flag_ContainsPointers
jnz ContainsPointers

; We have no pointers - emit a simple inline copy loop
; Copy the contents from the end
mov ecx, [rcx + OFFSET__MethodTable__m_BaseSize]
sub ecx, 18h ; sizeof(ObjHeader) + sizeof(Object) + last slot

align 16
CopyLoop:
mov r8, [rdx+rcx]
mov [rax+rcx+8], r8
sub ecx, 8
jge CopyLoop
REPRET

ContainsPointers:
; Do call to CopyValueClassUnchecked(object, data, pMT)
push_vol_reg rax
alloc_stack 20h
END_PROLOGUE

mov r8, rcx
lea rcx, [rax + 8]
call CopyValueClassUnchecked

add rsp, 20h
pop rax
ret

AllocFailed:
NullRef:
jmp JIT_Box
NESTED_END JIT_BoxFastMP, _TEXT

end
Loading