fix bugs with inlining import thunks and saving stale profile info af…

…ter reparse
chakra-core · Sep 12, 2017 · b0b06fc · b0b06fc
1 parent ab1ab30
commit b0b06fc
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 14 deletions.
diff --git a/lib/Runtime/Base/FunctionBody.cpp b/lib/Runtime/Base/FunctionBody.cpp
@@ -1609,9 +1609,9 @@ namespace Js
       paramScopeSlotArraySize(0),
       m_reparsed(false),
       m_isAsmJsFunction(false),
-      m_tag21(true)
+      m_tag21(true),
+      m_wasEverAsmjsMode(false)
 #if DBG
-      ,m_wasEverAsmjsMode(false)
       ,scopeObjectSize(0)
 #endif
     {
@@ -1659,10 +1659,8 @@ namespace Js
       m_isStaticNameFunction(proxy->GetIsStaticNameFunction()),
       m_reportedInParamCount(proxy->GetReportedInParamsCount()),
       m_reparsed(proxy->IsReparsed()),
-      m_tag21(true)
-#if DBG
-      ,m_wasEverAsmjsMode(proxy->m_wasEverAsmjsMode)
-#endif
+      m_tag21(true),
+      m_wasEverAsmjsMode(proxy->m_wasEverAsmjsMode)
     {
         FunctionInfo * functionInfo = proxy->GetFunctionInfo();
         this->functionInfo = functionInfo;
@@ -4936,7 +4934,24 @@ namespace Js
         this->SetByteCodeInLoopCount(0);
 
 #if ENABLE_PROFILE_INFO
-        this->dynamicProfileInfo = nullptr;
+        if (this->dynamicProfileInfo != nullptr)
+        {
+            GetSourceContextInfo()->sourceDynamicProfileManager->RemoveDynamicProfileInfo(GetFunctionInfo()->GetLocalFunctionId());
+
+#ifdef DYNAMIC_PROFILE_STORAGE
+            DynamicProfileInfoList * profileInfoList = GetScriptContext()->GetProfileInfoList();
+            for (auto iter = profileInfoList->GetEditingIterator(); iter.IsValid(); iter.Next())
+            {
+                DynamicProfileInfo * info = iter.Data();
+                if (info->HasFunctionBody() && info->GetFunctionBody() == this)
+                {
+                    iter.UnlinkCurrent();
+                    break;
+                }
+            }
+#endif
+            this->dynamicProfileInfo = nullptr;
+        }
 #endif
         this->hasExecutionDynamicProfileInfo = false;
 

diff --git a/lib/Runtime/Base/FunctionBody.h b/lib/Runtime/Base/FunctionBody.h
@@ -1724,13 +1724,16 @@ namespace Js
         void SetIsAsmjsMode(bool value)
         {
             m_isAsmjsMode = value;
-    #if DBG
+#if DBG
             if (value)
             {
                 m_wasEverAsmjsMode = true;
             }
-    #endif
+#endif
         }
+#if DBG
+        bool WasEverAsmJsMode() const { return m_wasEverAsmjsMode; }
+#endif
 
         void SetIsWasmFunction(bool val)
         {
@@ -1965,6 +1968,9 @@ namespace Js
 
         // Indicates if the function has been reparsed for debug attach/detach scenario.
         FieldWithBarrier(bool) m_reparsed : 1;
+#if DBG
+        FieldWithBarrier(bool) m_wasEverAsmjsMode : 1; // has m_isAsmjsMode ever been true
+#endif
 
         // This field is not required for deferred parsing but because our thunks can't handle offsets > 128 bytes
         // yet, leaving this here for now. We can look at optimizing the function info and function proxy structures some
@@ -1993,7 +1999,6 @@ namespace Js
 
     public:
 #if DBG
-        FieldWithBarrier(bool) m_wasEverAsmjsMode; // has m_isAsmjsMode ever been true
         FieldWithBarrier(Js::LocalFunctionId) deferredParseNextFunctionId;
 #endif
 #if DBG

diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp
@@ -1803,7 +1803,7 @@ void ByteCodeGenerator::InitScopeSlotArray(FuncInfo * funcInfo)
     {
         Js::PropertyId *propertyIdsForScopeSlotArray = RecyclerNewArrayLeafZ(scriptContext->GetRecycler(), Js::PropertyId, scopeSlotCount);
         byteCodeFunction->SetPropertyIdsForScopeSlotArray(propertyIdsForScopeSlotArray, scopeSlotCount, scopeSlotCountForParamScope);
-        AssertMsg(!byteCodeFunction->IsReparsed() || byteCodeFunction->m_wasEverAsmjsMode || byteCodeFunction->scopeSlotArraySize == scopeSlotCount,
+        AssertMsg(!byteCodeFunction->IsReparsed() || byteCodeFunction->WasEverAsmJsMode() || byteCodeFunction->scopeSlotArraySize == scopeSlotCount,
             "The slot array size is different between debug and non-debug mode");
 #if DEBUG
         for (UINT i = 0; i < scopeSlotCount; i++)
@@ -1930,7 +1930,7 @@ void ByteCodeGenerator::LoadAllConstants(FuncInfo *funcInfo)
         // A reparse should result in the same size of the activation object.
         // Exclude functions which were created from the ByteCodeCache.
         AssertMsg(!byteCodeFunction->IsReparsed() || byteCodeFunction->HasGeneratedFromByteCodeCache() ||
-            byteCodeFunction->scopeObjectSize == count || byteCodeFunction->m_wasEverAsmjsMode,
+            byteCodeFunction->scopeObjectSize == count || byteCodeFunction->WasEverAsmJsMode(),
             "The activation object size is different between debug and non-debug mode");
         byteCodeFunction->scopeObjectSize = count;
 #endif

diff --git a/lib/Runtime/Language/DynamicProfileInfo.cpp b/lib/Runtime/Language/DynamicProfileInfo.cpp
@@ -1843,7 +1843,6 @@ namespace Js
 #if DBG_DUMP
         writer->Log(this);
 #endif
-
         FunctionBody * functionBody = this->GetFunctionBody();
         Js::ArgSlot paramInfoCount = functionBody->GetProfiledInParamsCount();
         if (!writer->Write(functionBody->GetLocalFunctionId())

diff --git a/lib/Runtime/Language/SourceDynamicProfileManager.cpp b/lib/Runtime/Language/SourceDynamicProfileManager.cpp
@@ -52,6 +52,14 @@ namespace Js
         dynamicProfileInfoMap.Item(functionId, dynamicProfileInfo);
     }
 
+    void SourceDynamicProfileManager::RemoveDynamicProfileInfo(LocalFunctionId functionId)
+    {
+        dynamicProfileInfoMap.Remove(functionId);
+#ifdef DYNAMIC_PROFILE_STORAGE
+        dynamicProfileInfoMapSaving.Remove(functionId);
+#endif
+    }
+
     void SourceDynamicProfileManager::MarkAsExecuted(LocalFunctionId functionId)
     {
         Assert(startupFunctions != nullptr);

diff --git a/lib/Runtime/Language/SourceDynamicProfileManager.h b/lib/Runtime/Language/SourceDynamicProfileManager.h
@@ -33,6 +33,7 @@ namespace Js
         DynamicProfileInfo * GetDynamicProfileInfo(FunctionBody * functionBody);
         Recycler* GetRecycler() { return recycler; }
         void UpdateDynamicProfileInfo(LocalFunctionId functionId, DynamicProfileInfo * dynamicProfileInfo);
+        void RemoveDynamicProfileInfo(LocalFunctionId functionId);
         void MarkAsExecuted(LocalFunctionId functionId);
         static SourceDynamicProfileManager * LoadFromDynamicProfileStorage(SourceContextInfo* info, ScriptContext* scriptContext, IActiveScriptDataCache* profileDataCache);
         void EnsureStartupFunctions(uint numberOfFunctions);

diff --git a/lib/WasmReader/WasmByteCodeGenerator.cpp b/lib/WasmReader/WasmByteCodeGenerator.cpp
@@ -963,7 +963,9 @@ EmitInfo WasmBytecodeGenerator::EmitCall()
         funcNum = GetReader()->m_currentNode.call.num;
         WasmFunctionInfo* calleeInfo = m_module->GetWasmFunctionInfo(funcNum);
         calleeSignature = calleeInfo->GetSignature();
-        if (!isImportCall)
+        // currently only handle inlining internal function calls
+        // in future we can expand to all calls by adding checks in inliner and falling back to call in case ScriptFunction doesn't match
+        if (GetReader()->m_currentNode.call.funcType == FunctionIndexTypes::Function)
         {
             profileId = GetNextProfileId();
         }