[clr-interp] Improve the performance of interpreter calls (#120067)

* [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.

* Add a tweak so that CallPreStub is able to succeed with methods which were not quite ready to execute.

* Fix build breaks

* Change NULL check to PCODE comparison

Author: davidwrighton

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;

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 != (PCODE)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 == (PCODE)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 != (PCODE)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, (PCODE)NULL);
     }
 
     // CallStubHeaders encode their destination addresses in the Routines array, so they need to be
@@ -627,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
@@ -2440,6 +2449,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 +2463,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, (PCODE)NULL);
                             break;
                         }
                     }

src/coreclr/vm/method.hpp

@@ -1805,18 +1805,37 @@ 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<TADDR>(interpreterCode) == INTERPRETER_CODE_POISON)
+            return NULL;
+        return interpreterCode;
     }
     void SetInterpreterCode(PTR_InterpByteCodeStart interpreterCode)
     {
         LIMITED_METHOD_CONTRACT;
+        _ASSERTE(dac_cast<TADDR>(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<PTR_InterpByteCodeStart>((TADDR)INTERPRETER_CODE_POISON));
+    }
+
+    bool IsInterpreterCodePoisoned() const
+    {
+        LIMITED_METHOD_DAC_CONTRACT;
+        return dac_cast<TADDR>(VolatileLoadWithoutBarrier(&m_interpreterCode)) == INTERPRETER_CODE_POISON;
+    }
+
     void ClearInterpreterCodePointer()
     {
         LIMITED_METHOD_CONTRACT;

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)