Fix inconsistency in F12 rundown/reparse after redeferral. F12 rundown/reparse counts on having function bodies stay around so that top-level functions can be discovered and reparsed. With the changes in redeferral, and making the function body dictionary into a LeafValueDictionary, function bodies can be discarded, so the functions that appear to be top-level at reparse time are missing information about enclosing scopes. Fix this by detecting top-level functions at byte code gen time and reparsing them even if they've been kicked out of the dictionary.

pleath · pleath · commit 14b97eb01542 · 2017-02-08T17:49:26.000-08:00
diff --git a/lib/Runtime/Base/FunctionBody.cpp b/lib/Runtime/Base/FunctionBody.cpp
@@ -203,7 +203,7 @@ namespace Js
     // Given an offset into the source buffer, determine if the end of this SourceInfo
     // lies after the given offset.
     bool
-    FunctionBody::EndsAfter(size_t offset) const
+    ParseableFunctionInfo::EndsAfter(size_t offset) const
     {
         return offset < this->StartOffset() + this->LengthInBytes();
     }
diff --git a/lib/Runtime/Base/FunctionBody.h b/lib/Runtime/Base/FunctionBody.h
@@ -1980,6 +1980,8 @@ namespace Js
         void SetReparsed(bool set) { m_reparsed = set; }
         bool GetExternalDisplaySourceName(BSTR* sourceName);
 
+        bool EndsAfter(size_t offset) const;
+
         void SetDoBackendArgumentsOptimization(bool set)
         {
             m_doBackendArgumentsOptimization = set;
@@ -2875,8 +2877,6 @@ namespace Js
 
         bool HasGeneratedFromByteCodeCache() const { return this->byteCodeCache != nullptr; }
 
-        bool EndsAfter(size_t offset) const;
-
         void TrackLoad(int ichMin);
 
         SmallSpanSequence* GetStatementMapSpanSequence() const { return m_sourceInfo.pSpanSequence; }
diff --git a/lib/Runtime/Base/Utf8SourceInfo.cpp b/lib/Runtime/Base/Utf8SourceInfo.cpp
@@ -32,6 +32,7 @@ namespace Js
         m_lineOffsetCache(nullptr),
         m_deferredFunctionsDictionary(nullptr),
         m_deferredFunctionsInitialized(false),
+        topLevelFunctionInfoList(nullptr),
         debugModeSource(nullptr),
         debugModeSourceIsEmpty(false),
         debugModeSourceLength(0),
@@ -140,6 +141,31 @@ namespace Js
         functionBody->SetIsFuncRegistered(true);
     }
 
+    void Utf8SourceInfo::AddTopLevelFunctionInfo(FunctionInfo * functionInfo, Recycler * recycler)
+    {
+        JsUtil::List<FunctionInfo *, Recycler> * list = EnsureTopLevelFunctionInfoList(recycler);
+        Assert(!list->Contains(functionInfo));
+        list->Add(functionInfo);
+    }
+
+    void Utf8SourceInfo::ClearTopLevelFunctionInfoList()
+    {
+        if (this->topLevelFunctionInfoList)
+        {
+            this->topLevelFunctionInfoList->Clear();
+        }
+    }
+
+    JsUtil::List<FunctionInfo *, Recycler> *
+    Utf8SourceInfo::EnsureTopLevelFunctionInfoList(Recycler * recycler)
+    {
+        if (this->topLevelFunctionInfoList == nullptr)
+        {
+            this->topLevelFunctionInfoList = JsUtil::List<FunctionInfo *, Recycler>::New(recycler);
+        }
+        return this->topLevelFunctionInfoList;
+    }
+
     void Utf8SourceInfo::EnsureInitialized(int initialFunctionCount)
     {
         ThreadContext* threadContext = ThreadContext::GetContextForCurrentThread();
diff --git a/lib/Runtime/Base/Utf8SourceInfo.h b/lib/Runtime/Base/Utf8SourceInfo.h
@@ -130,6 +130,14 @@ namespace Js
         void SetFunctionBody(FunctionBody * functionBody);
         void RemoveFunctionBody(FunctionBody* functionBodyBeingRemoved);
 
+        void AddTopLevelFunctionInfo(Js::FunctionInfo * functionInfo, Recycler * recycler);
+        void ClearTopLevelFunctionInfoList();
+        JsUtil::List<Js::FunctionInfo *, Recycler> * EnsureTopLevelFunctionInfoList(Recycler * recycler);
+        JsUtil::List<Js::FunctionInfo *, Recycler> * GetTopLevelFunctionInfoList() const
+        {
+            return this->topLevelFunctionInfoList;
+        }
+
         // The following functions could get called even if EnsureInitialized hadn't gotten called
         // (Namely in the OOM scenario), so we simply guard against that condition rather than
         // asserting
@@ -376,6 +384,7 @@ namespace Js
 
         FunctionBodyDictionary* functionBodyDictionary;
         DeferredFunctionsDictionary* m_deferredFunctionsDictionary;
+        JsUtil::List<Js::FunctionInfo *, Recycler> *topLevelFunctionInfoList;
 
         DebugDocument* m_debugDocument;
 
diff --git a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp
@@ -775,6 +775,7 @@ void ByteCodeGenerator::SetRootFuncInfo(FuncInfo* func)
     }
 
     this->pRootFunc = func->byteCodeFunction->GetParseableFunctionInfo();
+    this->m_utf8SourceInfo->AddTopLevelFunctionInfo(func->byteCodeFunction->GetFunctionInfo(), scriptContext->GetRecycler());
 }
 
 Js::RegSlot ByteCodeGenerator::NextVarRegister()
diff --git a/lib/Runtime/ByteCode/ByteCodeSerializer.cpp b/lib/Runtime/ByteCode/ByteCodeSerializer.cpp
@@ -3858,6 +3858,9 @@ class ByteCodeBufferReader
 
         (*function) = functionBody;
 
+        sourceInfo->ClearTopLevelFunctionInfoList();
+        sourceInfo->AddTopLevelFunctionInfo(functionBody->GetFunctionInfo(), this->scriptContext->GetRecycler());
+
 #if ENABLE_NATIVE_CODEGEN && defined(ENABLE_PREJIT)
         if (prejit)
         {
diff --git a/lib/Runtime/Debug/DebugContext.cpp b/lib/Runtime/Debug/DebugContext.cpp
@@ -72,16 +72,38 @@ namespace Js
             return;
         }
 
-        FunctionBody * functionBody;
+        this->RegisterFunction(func, func->GetHostSourceContext(), title);
+    }
+
+    void DebugContext::RegisterFunction(Js::ParseableFunctionInfo * func, DWORD_PTR dwDebugSourceContext, LPCWSTR title)
+    {
+        if (!this->CanRegisterFunction())
+        {
+            return;
+        }
+
+        FunctionBody * functionBody = nullptr;
         if (func->IsDeferredParseFunction())
         {
-            functionBody = func->Parse();
+            HRESULT hr = S_OK;
+            Assert(!this->scriptContext->GetThreadContext()->IsScriptActive());
+
+            BEGIN_JS_RUNTIME_CALL_EX_AND_TRANSLATE_EXCEPTION_AND_ERROROBJECT_TO_HRESULT_NESTED(this->scriptContext, false)
+            {
+                functionBody = func->Parse();
+            }
+            END_JS_RUNTIME_CALL_AND_TRANSLATE_EXCEPTION_AND_ERROROBJECT_TO_HRESULT(hr);
+
+            if (FAILED(hr))
+            {
+                return;
+            }
         }
         else
         {
             functionBody = func->GetFunctionBody();
         }
-        this->RegisterFunction(functionBody, functionBody->GetHostSourceContext(), title);
+        this->RegisterFunction(functionBody, dwDebugSourceContext, title);
     }
 
     void DebugContext::RegisterFunction(Js::FunctionBody * functionBody, DWORD_PTR dwDebugSourceContext, LPCWSTR title)
@@ -123,7 +145,7 @@ namespace Js
             this->scriptContext, shouldPerformSourceRundown, shouldReparseFunctions);
 
         Js::TempArenaAllocatorObject *tempAllocator = nullptr;
-        JsUtil::List<Js::FunctionBody *, ArenaAllocator>* pFunctionsToRegister = nullptr;
+        JsUtil::List<Js::FunctionInfo *, Recycler>* pFunctionsToRegister = nullptr;
         JsUtil::List<Js::Utf8SourceInfo *, Recycler, false, Js::CopyRemovePolicy, RecyclerPointerComparer>* utf8SourceInfoList = nullptr;
 
         HRESULT hr = S_OK;
@@ -132,7 +154,6 @@ namespace Js
         BEGIN_TRANSLATE_OOM_TO_HRESULT_NESTED
         tempAllocator = threadContext->GetTemporaryAllocator(_u("debuggerAlloc"));
 
-        pFunctionsToRegister = JsUtil::List<Js::FunctionBody*, ArenaAllocator>::New(tempAllocator->GetAllocator());
         utf8SourceInfoList = JsUtil::List<Js::Utf8SourceInfo *, Recycler, false, Js::CopyRemovePolicy, RecyclerPointerComparer>::New(this->scriptContext->GetRecycler());
 
         this->MapUTF8SourceInfoUntil([&](Js::Utf8SourceInfo * sourceInfo) -> bool
@@ -197,9 +218,9 @@ namespace Js
                 dwDebugHostSourceContext = this->hostDebugContext->GetHostSourceContext(sourceInfo);
             }
 
-            this->FetchTopLevelFunction(pFunctionsToRegister, sourceInfo);
+            pFunctionsToRegister = sourceInfo->GetTopLevelFunctionInfoList();
 
-            if (pFunctionsToRegister->Count() == 0)
+            if (pFunctionsToRegister == nullptr || pFunctionsToRegister->Count() == 0)
             {
                 // This could happen if there are no functions to re-compile.
                 return false;
@@ -221,21 +242,20 @@ namespace Js
                     return true;
                 }
 
-                Js::FunctionBody* pFuncBody = pFunctionsToRegister->Item(i);
-                if (pFuncBody == nullptr)
+                Js::FunctionInfo *functionInfo = pFunctionsToRegister->Item(i);
+                if (functionInfo == nullptr)
                 {
                     continue;
                 }
 
                 if (shouldReparseFunctions)
                 {
-
                     BEGIN_JS_RUNTIME_CALL_EX_AND_TRANSLATE_EXCEPTION_AND_ERROROBJECT_TO_HRESULT_NESTED(cachedScriptContext, false)
                     {
-                        pFuncBody->Parse();
+                        functionInfo->GetParseableFunctionInfo()->Parse();
                         // This is the first call to the function, ensure dynamic profile info
 #if ENABLE_PROFILE_INFO
-                        pFuncBody->EnsureDynamicProfileInfo();
+                        functionInfo->GetFunctionBody()->EnsureDynamicProfileInfo();
 #endif
                     }
                     END_JS_RUNTIME_CALL_AND_TRANSLATE_EXCEPTION_AND_ERROROBJECT_TO_HRESULT(hr);
@@ -244,11 +264,14 @@ namespace Js
                     DEBUGGER_ATTACHDETACH_FATAL_ERROR_IF_FAILED(hr);
                 }
 
+                // Parsing the function may change its FunctionProxy.
+                Js::ParseableFunctionInfo *parseableFunctionInfo = functionInfo->GetParseableFunctionInfo();
+
                 if (!fHasDoneSourceRundown && shouldPerformSourceRundown && !cachedScriptContext->IsClosed())
                 {
                     BEGIN_TRANSLATE_OOM_TO_HRESULT_NESTED
                     {
-                        this->RegisterFunction(pFuncBody, dwDebugHostSourceContext, pFuncBody->GetSourceName());
+                        this->RegisterFunction(parseableFunctionInfo, dwDebugHostSourceContext, parseableFunctionInfo->GetSourceName());
                     }
                     END_TRANSLATE_OOM_TO_HRESULT(hr);
 
@@ -294,85 +317,6 @@ namespace Js
         return hr;
     }
 
-    void DebugContext::FetchTopLevelFunction(JsUtil::List<Js::FunctionBody *, ArenaAllocator>* pFunctions, Js::Utf8SourceInfo * sourceInfo)
-    {
-        Assert(pFunctions != nullptr);
-        Assert(sourceInfo != nullptr);
-
-        HRESULT hr = S_OK;
-
-        // Get FunctionBodys which are distinctly parseable, i.e. they are not enclosed in any other function (finding
-        // out root node of the sub-tree, in which root node is not enclosed in any other available function) this is
-        // by walking over all function and comparing their range.
-
-        BEGIN_TRANSLATE_OOM_TO_HRESULT_NESTED
-        {
-            pFunctions->Clear();
-
-            sourceInfo->MapFunctionUntil([&](Js::FunctionBody* pFuncBody) -> bool
-            {
-                if (pFuncBody->GetIsGlobalFunc())
-                {
-                    if (pFuncBody->IsFakeGlobalFunc(pFuncBody->GetGrfscr()))
-                    {
-                        // This is created due to 'Function' code or deferred parsed functions, there is nothing to
-                        // re-compile in this function as this is just a place-holder/fake function.
-
-                        Assert(pFuncBody->GetByteCode() == NULL);
-
-                        return false;
-                    }
-
-                    if (!pFuncBody->GetIsTopLevel())
-                    {
-                        return false;
-                    }
-
-                    // If global function, there is no need to find out any other functions.
-
-                    pFunctions->Clear();
-                    pFunctions->Add(pFuncBody);
-                    return true;
-                }
-
-                if (pFuncBody->IsFunctionParsed())
-                {
-                    bool isNeedToAdd = true;
-                    for (int i = 0; i < pFunctions->Count(); i++)
-                    {
-                        Js::FunctionBody *currentFunction = pFunctions->Item(i);
-                        if (currentFunction != nullptr)
-                        {
-                            if (currentFunction->StartInDocument() > pFuncBody->StartInDocument() || !currentFunction->EndsAfter(pFuncBody->StartInDocument()))
-                            {
-                                if (pFuncBody->StartInDocument() <= currentFunction->StartInDocument() && pFuncBody->EndsAfter(currentFunction->StartInDocument()))
-                                {
-                                    // The stored item has got the parent, remove current Item
-                                    pFunctions->Item(i, nullptr);
-                                }
-                            }
-                            else
-                            {
-                                // Parent (the enclosing function) is already in the list
-                                isNeedToAdd = false;
-                                break;
-                            }
-                        }
-                    }
-
-                    if (isNeedToAdd)
-                    {
-                        pFunctions->Add(pFuncBody);
-                    }
-                }
-                return false;
-            });
-        }
-        END_TRANSLATE_OOM_TO_HRESULT(hr);
-
-        Assert(hr == S_OK);
-    }
-
     // Create an ordered flat list of sources to reparse. Caller of a source should be added to the list before we add the source itself.
     void DebugContext::WalkAndAddUtf8SourceInfo(Js::Utf8SourceInfo* sourceInfo, JsUtil::List<Js::Utf8SourceInfo *, Recycler, false, Js::CopyRemovePolicy, RecyclerPointerComparer> *utf8SourceInfoList)
     {
diff --git a/lib/Runtime/Debug/DebugContext.h b/lib/Runtime/Debug/DebugContext.h
@@ -67,9 +67,9 @@ namespace Js
         ProbeContainer* diagProbesContainer;
 
         // Private Functions
-        void FetchTopLevelFunction(JsUtil::List<Js::FunctionBody *, ArenaAllocator>* pFunctions, Js::Utf8SourceInfo * sourceInfo);
         void WalkAndAddUtf8SourceInfo(Js::Utf8SourceInfo* sourceInfo, JsUtil::List<Js::Utf8SourceInfo *, Recycler, false, Js::CopyRemovePolicy, RecyclerPointerComparer> *utf8SourceInfoList);
         bool CanRegisterFunction() const;
+        void RegisterFunction(Js::ParseableFunctionInfo * func, DWORD_PTR dwDebugSourceContext, LPCWSTR title);
         void RegisterFunction(Js::FunctionBody * functionBody, DWORD_PTR dwDebugSourceContext, LPCWSTR title);
 
         template<class TMapFunction>

Original file line number	Diff line number	Diff line change
`@@ -203,7 +203,7 @@ namespace Js`
`203`	`203`	`// Given an offset into the source buffer, determine if the end of this SourceInfo`
`204`	`204`	`// lies after the given offset.`
`205`	`205`	`bool`
`206`		`- FunctionBody::EndsAfter(size_t offset) const`
	`206`	`+ ParseableFunctionInfo::EndsAfter(size_t offset) const`
`207`	`207`	`{`
`208`	`208`	`return offset < this->StartOffset() + this->LengthInBytes();`
`209`	`209`	`}`
Original file line number	Diff line number	Diff line change
`@@ -775,6 +775,7 @@ void ByteCodeGenerator::SetRootFuncInfo(FuncInfo* func)`
`775`	`775`	`}`
`776`	`776`
`777`	`777`	`this->pRootFunc = func->byteCodeFunction->GetParseableFunctionInfo();`
	`778`	`+ this->m_utf8SourceInfo->AddTopLevelFunctionInfo(func->byteCodeFunction->GetFunctionInfo(), scriptContext->GetRecycler());`
`778`	`779`	`}`
`779`	`780`
`780`	`781`	`Js::RegSlot ByteCodeGenerator::NextVarRegister()`
Original file line number	Diff line number	Diff line change
`@@ -3858,6 +3858,9 @@ class ByteCodeBufferReader`
`3858`	`3858`
`3859`	`3859`	`(*function) = functionBody;`
`3860`	`3860`
	`3861`	`+ sourceInfo->ClearTopLevelFunctionInfoList();`
	`3862`	`+ sourceInfo->AddTopLevelFunctionInfo(functionBody->GetFunctionInfo(), this->scriptContext->GetRecycler());`
	`3863`	`+`
`3861`	`3864`	`#if ENABLE_NATIVE_CODEGEN && defined(ENABLE_PREJIT)`
`3862`	`3865`	`if (prejit)`
`3863`	`3866`	`{`