From cfe5911eb1bafc6c68421df9e23867576bf1d95e Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 24 Sep 2025 13:27:22 -0700 Subject: [PATCH 1/4] [clr-interp] Improve the performance of interpreter calls - Instead of setting the target in the CallStubHeader at each invocation, store the target in the CallStubHeader immediately. - Avoid invoking the prestub unnecessarily. Instead, detect that we've already run the prestub to completion at least once before poisoning the m_interpreterCode field on MethodDesc. This may be enough to make some of our tests which are currently timing out pass. --- src/coreclr/vm/callstubgenerator.h | 7 +++++++ src/coreclr/vm/interpexec.cpp | 25 +++++++++++++++++++------ src/coreclr/vm/method.hpp | 20 +++++++++++++++++++- src/coreclr/vm/wasm/helpers.cpp | 2 +- 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/coreclr/vm/callstubgenerator.h b/src/coreclr/vm/callstubgenerator.h index 55ef6911480cdb..b32c6cfa83331b 100644 --- a/src/coreclr/vm/callstubgenerator.h +++ b/src/coreclr/vm/callstubgenerator.h @@ -45,6 +45,13 @@ struct CallStubHeader Routines[NumRoutines - 1] = target; } + PCODE GetTarget() + { + LIMITED_METHOD_CONTRACT; + + return Routines[NumRoutines - 1]; + } + size_t GetSize() { LIMITED_METHOD_CONTRACT; diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 059c34d201652c..d48d2d6662223c 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -68,7 +68,7 @@ void InvokeUnmanagedCalliWithTransition(PCODE ftn, void *cookie, int8_t *stack, #ifndef TARGET_WASM #include "callstubgenerator.h" -static CallStubHeader *UpdateCallStubForMethod(MethodDesc *pMD) +static CallStubHeader *UpdateCallStubForMethod(MethodDesc *pMD, PCODE target) { CONTRACTL { @@ -85,6 +85,11 @@ static CallStubHeader *UpdateCallStubForMethod(MethodDesc *pMD) AllocMemTracker amTracker; CallStubHeader *header = callStubGenerator.GenerateCallStub(pMD, &amTracker, true /* interpreterToNative */); + if (target != NULL) + { + header->SetTarget(target); + } + if (pMD->SetCallStub(header)) { amTracker.SuppressRelease(); @@ -114,11 +119,13 @@ void InvokeManagedMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE tar CallStubHeader *pHeader = pMD->GetCallStub(); if (pHeader == NULL) { - pHeader = UpdateCallStubForMethod(pMD); + pHeader = UpdateCallStubForMethod(pMD, target == NULL ? pMD->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY) : target); } - // Interpreter-FIXME: Potential race condition if a single CallStubHeader is reused for multiple targets. - pHeader->SetTarget(target); // The method to call + if (target != NULL) + { + _ASSERTE(pHeader->GetTarget() == target); + } pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); } @@ -143,7 +150,7 @@ void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, in CallStubHeader *stubHeaderTemplate = pMDDelegateInvoke->GetCallStub(); if (stubHeaderTemplate == NULL) { - stubHeaderTemplate = UpdateCallStubForMethod(pMDDelegateInvoke); + stubHeaderTemplate = UpdateCallStubForMethod(pMDDelegateInvoke, NULL); } // CallStubHeaders encode their destination addresses in the Routines array, so they need to be @@ -2440,6 +2447,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr InterpByteCodeStart* targetIp = targetMethod->GetInterpreterCode(); if (targetIp == NULL) { + if (!targetMethod->IsInterpreterCodePoisoned()) { // This is an optimization to ensure that the stack walk will not have to search // for the topmost frame in the current InterpExecMethod. It is not required @@ -2453,12 +2461,17 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr CallPreStub(targetMethod); targetIp = targetMethod->GetInterpreterCode(); + if (targetIp == NULL) + { + // The prestub wasn't able to setup an interpreter code, so it will never be able to. + targetMethod->PoisonInterpreterCode(); + } } if (targetIp == NULL) { // If we didn't get the interpreter code pointer setup, then this is a method we need to invoke as a compiled method. // Interpreter-FIXME: Implement tailcall via helpers, see https://github.com/dotnet/runtime/blob/main/docs/design/features/tailcalls-with-helpers.md - InvokeManagedMethod(targetMethod, callArgsAddress, returnValueAddress, targetMethod->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY)); + InvokeManagedMethod(targetMethod, callArgsAddress, returnValueAddress, NULL); break; } } diff --git a/src/coreclr/vm/method.hpp b/src/coreclr/vm/method.hpp index 9e272f64457e54..bb715ac0035528 100644 --- a/src/coreclr/vm/method.hpp +++ b/src/coreclr/vm/method.hpp @@ -1805,18 +1805,36 @@ class MethodDesc WORD m_wFlags; // See MethodDescFlags PTR_MethodDescCodeData m_codeData; #ifdef FEATURE_INTERPRETER +#define INTERPRETER_CODE_POISON 1 PTR_InterpByteCodeStart m_interpreterCode; public: const PTR_InterpByteCodeStart GetInterpreterCode() const { LIMITED_METHOD_DAC_CONTRACT; - return m_interpreterCode; + PTR_InterpByteCodeStart interpreterCode = VolatileLoadWithoutBarrier(&m_interpreterCode); + if (dac_cast(interpreterCode) == INTERPRETER_CODE_POISON) + return NULL; + return interpreterCode; } void SetInterpreterCode(PTR_InterpByteCodeStart interpreterCode) { LIMITED_METHOD_CONTRACT; + _ASSERTE(dac_cast(m_interpreterCode) != INTERPRETER_CODE_POISON); VolatileStore(&m_interpreterCode, interpreterCode); } + + // Call this if the m_interpreterCode will never be set to a valid value + void PoisonInterpreterCode() + { + LIMITED_METHOD_CONTRACT; + VolatileStore(&m_interpreterCode, dac_cast((TADDR)INTERPRETER_CODE_POISON)); + } + + bool IsInterpreterCodePoisoned() const + { + LIMITED_METHOD_DAC_CONTRACT; + return dac_cast(VolatileLoadWithoutBarrier(&m_interpreterCode)) == INTERPRETER_CODE_POISON; + } #endif // FEATURE_INTERPRETER #ifdef _DEBUG diff --git a/src/coreclr/vm/wasm/helpers.cpp b/src/coreclr/vm/wasm/helpers.cpp index 3208a2c16b1c56..45f5c05b38aae4 100644 --- a/src/coreclr/vm/wasm/helpers.cpp +++ b/src/coreclr/vm/wasm/helpers.cpp @@ -802,7 +802,7 @@ void InvokeManagedMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE tar _ASSERTE(cookie != NULL); - InvokeCalliStub(target, cookie, pArgs, pRet); + InvokeCalliStub(target == NULL ? pMD->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY) : target, cookie, pArgs, pRet); } void InvokeUnmanagedMethod(MethodDesc *targetMethod, int8_t *pArgs, int8_t *pRet, PCODE callTarget) From bd431c2090c30b27d7e0d5e435a09c6e65ab9a57 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 25 Sep 2025 11:50:23 -0700 Subject: [PATCH 2/4] Add a tweak so that CallPreStub is able to succeed with methods which were not quite ready to execute. --- src/coreclr/vm/interpexec.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index d48d2d6662223c..c238e5dab7bf61 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -634,7 +634,9 @@ static void CallPreStub(MethodDesc *pMD) STATIC_STANDARD_VM_CONTRACT; _ASSERTE(pMD != NULL); - if (!pMD->IsPointingToPrestub()) + if (!pMD->IsPointingToPrestub() && + pMD->GetTemporaryEntryPoint() && // The prestub may not yet be ready to be used, so force temporary entry point creation, and check again. + !pMD->IsPointingToPrestub()) return; struct Param From 918c24c5c128df14f9de8ae89d1387883863d281 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 25 Sep 2025 11:58:34 -0700 Subject: [PATCH 3/4] Fix build breaks --- src/coreclr/vm/interpexec.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index c238e5dab7bf61..6263e973afd239 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -85,7 +85,7 @@ static CallStubHeader *UpdateCallStubForMethod(MethodDesc *pMD, PCODE target) AllocMemTracker amTracker; CallStubHeader *header = callStubGenerator.GenerateCallStub(pMD, &amTracker, true /* interpreterToNative */); - if (target != NULL) + if (target != (PCODE)NULL) { header->SetTarget(target); } @@ -119,7 +119,7 @@ void InvokeManagedMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE tar CallStubHeader *pHeader = pMD->GetCallStub(); if (pHeader == NULL) { - pHeader = UpdateCallStubForMethod(pMD, target == NULL ? pMD->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY) : target); + pHeader = UpdateCallStubForMethod(pMD, target == (PCODE)NULL ? pMD->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY) : target); } if (target != NULL) @@ -150,7 +150,7 @@ void InvokeDelegateInvokeMethod(MethodDesc *pMDDelegateInvoke, int8_t *pArgs, in CallStubHeader *stubHeaderTemplate = pMDDelegateInvoke->GetCallStub(); if (stubHeaderTemplate == NULL) { - stubHeaderTemplate = UpdateCallStubForMethod(pMDDelegateInvoke, NULL); + stubHeaderTemplate = UpdateCallStubForMethod(pMDDelegateInvoke, (PCODE)NULL); } // CallStubHeaders encode their destination addresses in the Routines array, so they need to be @@ -2473,7 +2473,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr { // If we didn't get the interpreter code pointer setup, then this is a method we need to invoke as a compiled method. // Interpreter-FIXME: Implement tailcall via helpers, see https://github.com/dotnet/runtime/blob/main/docs/design/features/tailcalls-with-helpers.md - InvokeManagedMethod(targetMethod, callArgsAddress, returnValueAddress, NULL); + InvokeManagedMethod(targetMethod, callArgsAddress, returnValueAddress, (PCODE)NULL); break; } } From f551adaa7ab9389d67562e183c364cc270bf98f5 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 25 Sep 2025 16:11:18 -0700 Subject: [PATCH 4/4] Change NULL check to PCODE comparison --- src/coreclr/vm/interpexec.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 6263e973afd239..ef41586dc02176 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -122,7 +122,7 @@ void InvokeManagedMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE tar pHeader = UpdateCallStubForMethod(pMD, target == (PCODE)NULL ? pMD->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY) : target); } - if (target != NULL) + if (target != (PCODE)NULL) { _ASSERTE(pHeader->GetTarget() == target); }