[MERGE #1975 @rajatd] A rejit/rethunk change

Merge pull request #1975 from rajatd:rejitrethunk

If we are about to rejit a function body and the last rejit was caused by a bailout at the same bytecode offset as the current one, then its a fair assumption that if there is a newer entrypoint available, the problem that is causing the rejit would have been fixed in that entry point. So, rethunk to that entry point instead of rejitting.

This specially helps with bailouts that don't disable an optimization on rejit but rely on updated profile info to fix the problem.

Author: rajatd

lib/Backend/BailOut.cpp

@@ -1150,7 +1150,7 @@ uint32 bailOutOffset, void * returnAddress, IR::BailOutKind bailOutKind, Js::Imp
         sizeof(registerSaves));
 
     Js::Var result = BailOutCommonNoCodeGen(layout, bailOutRecord, bailOutOffset, returnAddress, bailOutKind, branchValue, nullptr, bailOutReturnValue, argoutRestoreAddress);
-    ScheduleFunctionCodeGen(Js::ScriptFunction::FromVar(layout->functionObject), nullptr, bailOutRecord, bailOutKind, savedImplicitCallFlags, returnAddress);
+    ScheduleFunctionCodeGen(Js::ScriptFunction::FromVar(layout->functionObject), nullptr, bailOutRecord, bailOutKind, bailOutOffset, savedImplicitCallFlags, returnAddress);
     return result;
 }
 
@@ -1171,7 +1171,7 @@ BailOutRecord::BailOutInlinedCommon(Js::JavascriptCallStackLayout * layout, Bail
     BailOutInlinedHelper(layout, currentBailOutRecord, bailOutOffset, returnAddress, bailOutKind, registerSaves, &bailOutReturnValue, &innerMostInlinee, false, branchValue);
     Js::Var result = BailOutCommonNoCodeGen(layout, currentBailOutRecord, currentBailOutRecord->bailOutOffset, returnAddress, bailOutKind, branchValue,
         registerSaves, &bailOutReturnValue);
-    ScheduleFunctionCodeGen(Js::ScriptFunction::FromVar(layout->functionObject), innerMostInlinee, currentBailOutRecord, bailOutKind, savedImplicitCallFlags, returnAddress);
+    ScheduleFunctionCodeGen(Js::ScriptFunction::FromVar(layout->functionObject), innerMostInlinee, currentBailOutRecord, bailOutKind, bailOutOffset, savedImplicitCallFlags, returnAddress);
     return result;
 }
 
@@ -1750,8 +1750,10 @@ BailOutRecord::BailOutHelper(Js::JavascriptCallStackLayout * layout, Js::ScriptF
 // code we avoid a rejit by checking if the offending optimization has been disabled in the default code and if so
 // we "rethunk" the bailing out function rather that incurring a rejit.
 
+// actualBailOutOffset - bail out offset in the function, inlinee or otherwise, that had the bailout.
+
 void BailOutRecord::ScheduleFunctionCodeGen(Js::ScriptFunction * function, Js::ScriptFunction * innerMostInlinee,
-    BailOutRecord const * bailOutRecord, IR::BailOutKind bailOutKind, Js::ImplicitCallFlags savedImplicitCallFlags, void * returnAddress)
+    BailOutRecord const * bailOutRecord, IR::BailOutKind bailOutKind, uint32 actualBailOutOffset, Js::ImplicitCallFlags savedImplicitCallFlags, void * returnAddress)
 {
     if (bailOutKind == IR::BailOnSimpleJitToFullJitLoopBody ||
         bailOutKind == IR::BailOutForGeneratorYield ||
@@ -2216,18 +2218,21 @@ void BailOutRecord::ScheduleFunctionCodeGen(Js::ScriptFunction * function, Js::S
         rejitReason = RejitReason::Forced;
     }
 
-    // REVIEW: Temporary fix for RS1.  Disable Rejiting if it looks like it is not fixing the problem.
-    //         For RS2, turn this into an assert and let's fix all these issues.
     if (!reThunk && rejitReason != RejitReason::None)
     {
-        if (executeFunction->GetDynamicProfileInfo()->GetRejitCount() >= 100)
+        Js::DynamicProfileInfo * profileInfo = executeFunction->GetAnyDynamicProfileInfo();
+        // REVIEW: Temporary fix for RS1.  Disable Rejiting if it looks like it is not fixing the problem.
+        //         For RS2, turn the rejitCount check into an assert and let's fix all these issues.
+        if (profileInfo->GetRejitCount() >= 100 ||
+            (profileInfo->GetBailOutOffsetForLastRejit() == actualBailOutOffset && function->IsNewEntryPointAvailable()))
         {
             reThunk = true;
             rejitReason = RejitReason::None;
         }
         else
         {
-            executeFunction->GetDynamicProfileInfo()->IncRejitCount();
+            profileInfo->IncRejitCount();
+            profileInfo->SetBailOutOffsetForLastRejit(actualBailOutOffset);
         }
     }
 

lib/Backend/BailOut.h

@@ -253,7 +253,8 @@ class BailOutRecord
 
     static void UpdatePolymorphicFieldAccess(Js::JavascriptFunction *  function, BailOutRecord const * bailOutRecord);
 
-    static void ScheduleFunctionCodeGen(Js::ScriptFunction * function, Js::ScriptFunction * innerMostInlinee, BailOutRecord const * bailOutRecord, IR::BailOutKind bailOutKind, Js::ImplicitCallFlags savedImplicitCallFlags, void * returnAddress);
+    static void ScheduleFunctionCodeGen(Js::ScriptFunction * function, Js::ScriptFunction * innerMostInlinee, BailOutRecord const * bailOutRecord, IR::BailOutKind bailOutKind, 
+                                        uint32 actualBailOutOffset, Js::ImplicitCallFlags savedImplicitCallFlags, void * returnAddress);
     static void ScheduleLoopBodyCodeGen(Js::ScriptFunction * function, Js::ScriptFunction * innerMostInlinee, BailOutRecord const * bailOutRecord, IR::BailOutKind bailOutKind);
     static void CheckPreemptiveRejit(Js::FunctionBody* executeFunction, IR::BailOutKind bailOutKind, BailOutRecord* bailoutRecord, uint8& callsOrIterationsCount, int loopNumber);
     void RestoreValues(IR::BailOutKind bailOutKind, Js::JavascriptCallStackLayout * layout, Js::InterpreterStackFrame * newInstance, Js::ScriptContext * scriptContext,

lib/Runtime/Language/DynamicProfileInfo.cpp

@@ -189,6 +189,7 @@ namespace Js
         }
 
         this->rejitCount = 0;
+        this->bailOutOffsetForLastRejit = Js::Constants::NoByteCodeOffset;
 #if DBG
         for (ProfileId i = 0; i < functionBody->GetProfiledArrayCallSiteCount(); ++i)
         {

lib/Runtime/Language/DynamicProfileInfo.h

@@ -517,6 +517,7 @@ namespace Js
 
         uint32 m_recursiveInlineInfo; // Bit is set for each callsites where the function is called recursively
         uint32 polymorphicCacheState;
+        uint32 bailOutOffsetForLastRejit;
         uint16 rejitCount;
         BYTE currentInlinerVersion; // Used to detect when inlining profile changes
         bool hasFunctionBody;
@@ -810,6 +811,8 @@ namespace Js
         static bool IsCallSiteNoInfo(Js::LocalFunctionId functionId) { return functionId == CallSiteNoInfo; }
         int IncRejitCount() { return this->rejitCount++; }
         int GetRejitCount() { return this->rejitCount; }
+        void SetBailOutOffsetForLastRejit(uint32 offset) { this->bailOutOffsetForLastRejit = offset; }
+        uint32 GetBailOutOffsetForLastRejit() { return this->bailOutOffsetForLastRejit; }
 
 #if DBG_DUMP
         void Dump(FunctionBody* functionBody, ArenaAllocator * dynamicProfileInfoAllocator = nullptr);

lib/Runtime/Library/ScriptFunction.cpp

@@ -323,6 +323,14 @@ namespace Js
         return functionBody->GetOriginalEntryPoint();
     }
 
+    bool ScriptFunction::IsNewEntryPointAvailable()
+    {
+        Js::FunctionEntryPointInfo *const defaultEntryPointInfo = this->GetFunctionBody()->GetDefaultFunctionEntryPointInfo();
+        JavascriptMethod defaultEntryPoint = this->GetFunctionBody()->GetDirectEntryPoint(defaultEntryPointInfo);
+
+        return this->GetEntryPoint() != defaultEntryPoint;
+    }
+
     Var ScriptFunction::GetSourceString() const
     {
         return this->GetFunctionProxy()->EnsureDeserialized()->GetCachedSourceString();

lib/Runtime/Library/ScriptFunction.h

@@ -72,6 +72,7 @@ namespace Js
 
         void ChangeEntryPoint(ProxyEntryPointInfo* entryPointInfo, JavascriptMethod entryPoint);
         JavascriptMethod UpdateThunkEntryPoint(FunctionEntryPointInfo* entryPointInfo, JavascriptMethod entryPoint);
+        bool IsNewEntryPointAvailable();
         JavascriptMethod UpdateUndeferredBody(FunctionBody* newFunctionInfo);
 
         virtual ScriptFunctionType * DuplicateType() override;