Merge pull request #48 from MetanoKid/filtering-sooner

MetanoKid · web-flow · commit cc710258f156 · 2020-08-20T19:35:00.000+02:00
Refactors activity filtering to happen sooner
diff --git a/projects/AnalysisData/AnalysisData.vcxproj b/projects/AnalysisData/AnalysisData.vcxproj
@@ -20,7 +20,6 @@
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="..\..\src\AnalysisData\BuildTimeline\BuildTimeline.h" />
-    <ClInclude Include="..\..\src\AnalysisData\BuildTimeline\IgnoredEntries.h" />
     <ClInclude Include="..\..\src\AnalysisData\BuildTimeline\ProcessThreadRemapping\PackedProcessThreadRemapper.h" />
     <ClInclude Include="..\..\src\AnalysisData\BuildTimeline\ProcessThreadRemapping\ProcessThreadRemapping.h" />
     <ClInclude Include="..\..\src\AnalysisData\BuildTimeline\TimelineEntry.h" />
diff --git a/projects/AnalysisData/AnalysisData.vcxproj.filters b/projects/AnalysisData/AnalysisData.vcxproj.filters
@@ -35,9 +35,6 @@
     <ClInclude Include="..\..\src\AnalysisData\Utilities\CppBuildInsightsDataConversion.h">
       <Filter>Utilities</Filter>
     </ClInclude>
-    <ClInclude Include="..\..\src\AnalysisData\BuildTimeline\IgnoredEntries.h">
-      <Filter>BuildTimeline</Filter>
-    </ClInclude>
     <ClInclude Include="..\..\src\AnalysisData\TemplateInstantiationData.h" />
     <ClInclude Include="..\..\src\AnalysisData\SymbolKey.h" />
     <ClInclude Include="..\..\src\AnalysisData\SymbolNames.h" />
diff --git a/projects/BuildAnalyzer/BuildAnalyzer.vcxproj b/projects/BuildAnalyzer/BuildAnalyzer.vcxproj
@@ -20,7 +20,7 @@
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="..\..\src\BuildAnalyzer\Analyzers\BuildTimeline\BuildTimelineAnalyzer.h" />
-    <ClInclude Include="..\..\src\BuildAnalyzer\Analyzers\BuildTimeline\FilterTimelineAnalyzer.h" />
+    <ClInclude Include="..\..\src\BuildAnalyzer\Analyzers\BuildTimeline\FilterTimeline.h" />
     <ClInclude Include="..\..\src\BuildAnalyzer\Analyzers\FileCompilations\FileCompilationsAnalyzer.h" />
     <ClInclude Include="..\..\src\BuildAnalyzer\Analyzers\FileInclusions\FileInclusionsAnalyzer.h" />
     <ClInclude Include="..\..\src\BuildAnalyzer\Analyzers\FunctionCompilations\FunctionCompilationsAnalyzer.h" />
@@ -33,7 +33,7 @@
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="..\..\src\BuildAnalyzer\Analyzers\BuildTimeline\BuildTimelineAnalyzer.cpp" />
-    <ClCompile Include="..\..\src\BuildAnalyzer\Analyzers\BuildTimeline\FilterTimelineAnalyzer.cpp" />
+    <ClCompile Include="..\..\src\BuildAnalyzer\Analyzers\BuildTimeline\FilterTimeline.cpp" />
     <ClCompile Include="..\..\src\BuildAnalyzer\Analyzers\FileCompilations\FileCompilationsAnalyzer.cpp" />
     <ClCompile Include="..\..\src\BuildAnalyzer\Analyzers\FileInclusions\FileInclusionsAnalyzer.cpp" />
     <ClCompile Include="..\..\src\BuildAnalyzer\Analyzers\FunctionCompilations\FunctionCompilationsAnalyzer.cpp" />
diff --git a/projects/BuildAnalyzer/BuildAnalyzer.vcxproj.filters b/projects/BuildAnalyzer/BuildAnalyzer.vcxproj.filters
@@ -40,12 +40,12 @@
     <ClInclude Include="..\..\src\BuildAnalyzer\Analyzers\BuildTimeline\BuildTimelineAnalyzer.h">
       <Filter>Analyzers\BuildTimeline</Filter>
     </ClInclude>
-    <ClInclude Include="..\..\src\BuildAnalyzer\Analyzers\BuildTimeline\FilterTimelineAnalyzer.h">
-      <Filter>Analyzers\BuildTimeline</Filter>
-    </ClInclude>
     <ClInclude Include="..\..\src\BuildAnalyzer\Analyzers\TemplateInstantiations\TemplateInstantiationsAnalyzer.h">
       <Filter>Analyzers\TemplateInstantiations</Filter>
     </ClInclude>
+    <ClInclude Include="..\..\src\BuildAnalyzer\Analyzers\BuildTimeline\FilterTimeline.h">
+      <Filter>Analyzers\BuildTimeline</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="..\..\src\BuildAnalyzer\BuildAnalyzer.cpp" />
@@ -64,12 +64,12 @@
     <ClCompile Include="..\..\src\BuildAnalyzer\Analyzers\BuildTimeline\BuildTimelineAnalyzer.cpp">
       <Filter>Analyzers\BuildTimeline</Filter>
     </ClCompile>
-    <ClCompile Include="..\..\src\BuildAnalyzer\Analyzers\BuildTimeline\FilterTimelineAnalyzer.cpp">
-      <Filter>Analyzers\BuildTimeline</Filter>
-    </ClCompile>
     <ClCompile Include="..\..\src\BuildAnalyzer\Analyzers\TemplateInstantiations\TemplateInstantiationsAnalyzer.cpp">
       <Filter>Analyzers\TemplateInstantiations</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\src\BuildAnalyzer\Analyzers\BuildTimeline\FilterTimeline.cpp">
+      <Filter>Analyzers\BuildTimeline</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <None Include="packages.config" />
diff --git a/src/AnalysisData/BuildTimeline/BuildTimeline.cpp b/src/AnalysisData/BuildTimeline/BuildTimeline.cpp
@@ -60,6 +60,32 @@ void BuildTimeline::FinishEntry(const CppBI::Activities::Activity& activity)
     m_remapper.CalculateLocalChildrenData(entry);
 }
 
+void BuildTimeline::RemoveHierarchy(const TEventInstanceId& parentId, const TEventInstanceId& id)
+{
+    TimelineEntry* parentEntry = GetEntry(parentId);
+    assert(parentEntry != nullptr);
+
+    // ensure parent no longer points to it
+    parentEntry->RemoveChild(id);
+
+    // remove entry from the hierarchy (no need to ensure intermediate parents don't point to intermediate children, because the full hierarchy will be deleted)
+    RemoveEntry(id);
+}
+
+void BuildTimeline::RemoveEntry(const TEventInstanceId& id)
+{
+    TimelineEntry* entry = GetEntry(id);
+    if (entry != nullptr)
+    {
+        for (TimelineEntry* child : entry->GetChildren())
+        {
+            RemoveEntry(child->GetId());
+        }
+
+        m_entries.erase(id);
+    }
+}
+
 void BuildTimeline::FinishTimeline()
 {
     assert(m_remapper.GetRemappings().empty());
diff --git a/src/AnalysisData/BuildTimeline/BuildTimeline.h b/src/AnalysisData/BuildTimeline/BuildTimeline.h
@@ -34,6 +34,7 @@ class BuildTimeline
     void AddNestedEntry(const CppBI::Activities::Activity& parent,
                         const CppBI::Activities::Activity& activity);
     void FinishEntry(const CppBI::Activities::Activity& activity);
+    void RemoveHierarchy(const TEventInstanceId& parentId, const TEventInstanceId& id);
     void FinishTimeline();
 
     void UpdateEntryName(const TEventInstanceId& id, const std::string& name);
@@ -48,5 +49,6 @@ class BuildTimeline
     PackedProcessThreadRemapper m_remapper;
 
     TimelineEntry* AddEntry(const CppBI::Activities::Activity& activity);
+    void RemoveEntry(const TEventInstanceId& id);
     TimelineEntry* GetEntry(const TEventInstanceId& id);
 };
diff --git a/src/AnalysisData/BuildTimeline/IgnoredEntries.h b/src/AnalysisData/BuildTimeline/IgnoredEntries.h
diff --git a/src/AnalysisData/BuildTimeline/TimelineEntry.cpp b/src/AnalysisData/BuildTimeline/TimelineEntry.cpp
@@ -33,6 +33,17 @@ void TimelineEntry::AddChild(TimelineEntry* entry)
     m_children.push_back(entry);
 }
 
+void TimelineEntry::RemoveChild(const TEventInstanceId& id)
+{
+    auto it = std::find_if(m_children.begin(), m_children.end(), [&](const TimelineEntry* child)
+    {
+        return child->GetId() == id;
+    });
+    assert(it != m_children.end());
+
+    m_children.erase(it);
+}
+
 void TimelineEntry::SetName(const std::string& name)
 {
     m_name = name;
diff --git a/src/AnalysisData/BuildTimeline/TimelineEntry.h b/src/AnalysisData/BuildTimeline/TimelineEntry.h
@@ -20,6 +20,7 @@ class TimelineEntry
     ~TimelineEntry();
 
     void AddChild(TimelineEntry* entry);
+    void RemoveChild(const TEventInstanceId& id);
     bool AddProperty(const std::string& key, const std::string& value);
 
     void SetName(const std::string& name);
diff --git a/src/AnalysisExporter/BuildTimeline/BuildTimelineExporter.cpp b/src/AnalysisExporter/BuildTimeline/BuildTimelineExporter.cpp
@@ -13,14 +13,8 @@
 namespace
 {
     void AddEntry(const TimelineEntry* entry, rapidjson::Value& traceEvents, rapidjson::Document& document,
-                  const TProcessThreadRemappings& remappings, const TIgnoredEntries& ignoredEntries)
+                  const TProcessThreadRemappings& remappings)
     {
-        // apply filtering
-        if (ignoredEntries.find(entry->GetId()) != ignoredEntries.end())
-        {
-            return;
-        }
-
         auto itRemap = remappings.find(entry->GetId());
         const TProcessId& processId = itRemap != remappings.end() ? itRemap->second.ProcessId : entry->GetProcessId();
         const TThreadId& threadId = itRemap != remappings.end() ? itRemap->second.ThreadId : entry->GetThreadId();
@@ -84,7 +78,7 @@ namespace
             // write children entries
             for (auto&& child : entry->GetChildren())
             {
-                AddEntry(child, traceEvents, document, remappings, ignoredEntries);
+                AddEntry(child, traceEvents, document, remappings);
             }
 
             // write "end" event
@@ -103,9 +97,8 @@ namespace
     }
 }
 
-BuildTimelineExporter::BuildTimelineExporter(const BuildTimeline& timeline, const TIgnoredEntries& ignoredEntries)
+BuildTimelineExporter::BuildTimelineExporter(const BuildTimeline& timeline)
     : m_timeline(timeline)
-    , m_ignoredEntries(ignoredEntries)
 {
 }
 
@@ -134,7 +127,7 @@ bool BuildTimelineExporter::ExportTo(const std::string& path) const
         rapidjson::Value traceEvents(rapidjson::kArrayType);
         for (auto&& root : m_timeline.GetRoots())
         {
-            AddEntry(root, traceEvents, document, m_timeline.GetProcessThreadRemappings(), m_ignoredEntries);
+            AddEntry(root, traceEvents, document, m_timeline.GetProcessThreadRemappings());
         }
         document.AddMember("traceEvents", traceEvents, document.GetAllocator());
     }
diff --git a/src/AnalysisExporter/BuildTimeline/BuildTimelineExporter.h b/src/AnalysisExporter/BuildTimeline/BuildTimelineExporter.h
@@ -2,22 +2,19 @@
 
 #include <string>
 
-#include "AnalysisData\BuildTimeline\IgnoredEntries.h"
-
 class BuildTimeline;
 class TimelineEntry;
 class ProcessIdThreadIdRecalculation;
 
 class BuildTimelineExporter
 {
 public:
-    BuildTimelineExporter(const BuildTimeline& timeline, const TIgnoredEntries& ignoredEntries);
+    BuildTimelineExporter(const BuildTimeline& timeline);
     ~BuildTimelineExporter();
 
     // exports to Google Chrome trace format
     bool ExportTo(const std::string& path) const;
 
 private:
     const BuildTimeline& m_timeline;
-    const TIgnoredEntries& m_ignoredEntries;
 };
diff --git a/src/BuildAnalyzer/Analyzers/BuildTimeline/BuildTimelineAnalyzer.cpp b/src/BuildAnalyzer/Analyzers/BuildTimeline/BuildTimelineAnalyzer.cpp
@@ -1,12 +1,14 @@
 #include "BuildTimelineAnalyzer.h"
 
 #include <cassert>
+#include <algorithm>
 
 #include "AnalysisData\Utilities\CppBuildInsightsDataConversion.h"
 
-BuildTimelineAnalyzer::BuildTimelineAnalyzer()
+BuildTimelineAnalyzer::BuildTimelineAnalyzer(const FilterTimeline& filter)
     : CppBI::IAnalyzer()
     , m_buildTimeline()
+    , m_filter(filter)
     , m_symbolNames()
     , m_unresolvedTemplateInstantiationsPerSymbol()
 {
@@ -36,6 +38,8 @@ CppBI::AnalysisControl BuildTimelineAnalyzer::OnStartActivity(const CppBI::Event
 CppBI::AnalysisControl BuildTimelineAnalyzer::OnStopActivity(const CppBI::EventStack& eventStack)
 {
     bool processed =
+        CppBI::MatchEventStackInMemberFunction(eventStack,   this, &BuildTimelineAnalyzer::OnFunctionFinished) ||
+        CppBI::MatchEventStackInMemberFunction(eventStack,   this, &BuildTimelineAnalyzer::OnTemplateInstantiationFinished) ||
         CppBI::MatchEventInMemberFunction(eventStack.Back(), this, &BuildTimelineAnalyzer::OnActivityFinished);
     assert(processed);
 
@@ -101,6 +105,19 @@ void BuildTimelineAnalyzer::OnFunction(const CppBI::Activities::Function& functi
     m_buildTimeline.UpdateEntryName(function.EventInstanceId(), Utilities::CppBuildInsightsDataConversion::UndecorateFunction(function.Name()));
 }
 
+void BuildTimelineAnalyzer::OnFunctionFinished(const CppBI::Activities::Activity& parent, const CppBI::Activities::Function& function)
+{
+    if (m_filter.Passes(function))
+    {
+        OnActivityFinished(function);
+    }
+    else
+    {
+        // remove from timeline
+        m_buildTimeline.RemoveHierarchy(parent.EventInstanceId(), function.EventInstanceId());
+    }
+}
+
 void BuildTimelineAnalyzer::OnTemplateInstantiation(const CppBI::Activities::TemplateInstantiation& templateInstantiation)
 {
     // from the docs: "Type keys are unique within the trace being analyzed. [...]
@@ -114,6 +131,28 @@ void BuildTimelineAnalyzer::OnTemplateInstantiation(const CppBI::Activities::Tem
     result.first->second.push_back(templateInstantiation.EventInstanceId());
 }
 
+void BuildTimelineAnalyzer::OnTemplateInstantiationFinished(const CppBI::Activities::Activity& parent, const CppBI::Activities::TemplateInstantiation& templateInstantiation)
+{
+    if (m_filter.Passes(templateInstantiation))
+    {
+        OnActivityFinished(templateInstantiation);
+    }
+    else
+    {
+        // remove subscription
+        auto itSubscriptionsForCurrentSpecialization = m_unresolvedTemplateInstantiationsPerSymbol.find(templateInstantiation.SpecializationSymbolKey());
+        assert(itSubscriptionsForCurrentSpecialization != m_unresolvedTemplateInstantiationsPerSymbol.end());
+
+        auto itSubscription = std::find(itSubscriptionsForCurrentSpecialization->second.begin(),
+                                        itSubscriptionsForCurrentSpecialization->second.end(),
+                                        templateInstantiation.EventInstanceId());
+        itSubscriptionsForCurrentSpecialization->second.erase(itSubscription);
+
+        // remove from timeline
+        m_buildTimeline.RemoveHierarchy(parent.EventInstanceId(), templateInstantiation.EventInstanceId());
+    }
+}
+
 void BuildTimelineAnalyzer::OnThread(const CppBI::Activities::Activity& parent, const CppBI::Activities::Thread& thread)
 {
     m_buildTimeline.UpdateEntryName(thread.EventInstanceId(), std::string(parent.EventName()) + std::string(thread.EventName()));
diff --git a/src/BuildAnalyzer/Analyzers/BuildTimeline/BuildTimelineAnalyzer.h b/src/BuildAnalyzer/Analyzers/BuildTimeline/BuildTimelineAnalyzer.h
@@ -5,12 +5,13 @@
 #include <CppBuildInsights.hpp>
 namespace CppBI = Microsoft::Cpp::BuildInsights;
 
+#include "BuildAnalyzer\Analyzers\BuildTimeline\FilterTimeline.h"
 #include "AnalysisData\BuildTimeline\BuildTimeline.h"
 
 class BuildTimelineAnalyzer : public CppBI::IAnalyzer
 {
 public:
-    BuildTimelineAnalyzer();
+    BuildTimelineAnalyzer(const FilterTimeline& filter);
     virtual ~BuildTimelineAnalyzer();
 
     CppBI::AnalysisControl OnStartActivity(const CppBI::EventStack& eventStack) override;
@@ -22,6 +23,7 @@ class BuildTimelineAnalyzer : public CppBI::IAnalyzer
 
 private:
     BuildTimeline m_buildTimeline;
+    FilterTimeline m_filter;
     
     typedef unsigned long long TSymbolKey;
     std::unordered_map<TSymbolKey, std::string> m_symbolNames;
@@ -38,7 +40,9 @@ class BuildTimelineAnalyzer : public CppBI::IAnalyzer
     void OnInvocation(const CppBI::Activities::Invocation& invocation);
     void OnFrontEndFile(const CppBI::Activities::FrontEndFile& frontEndFile);
     void OnFunction(const CppBI::Activities::Function& function);
+    void OnFunctionFinished(const CppBI::Activities::Activity& parent, const CppBI::Activities::Function& function);
     void OnTemplateInstantiation(const CppBI::Activities::TemplateInstantiation& templateInstantiation);
+    void OnTemplateInstantiationFinished(const CppBI::Activities::Activity& parent, const CppBI::Activities::TemplateInstantiation& templateInstantiation);
     void OnThread(const CppBI::Activities::Activity& parent, const CppBI::Activities::Thread& thread);
 
     // specific event handling
diff --git a/src/BuildAnalyzer/Analyzers/BuildTimeline/FilterTimeline.cpp b/src/BuildAnalyzer/Analyzers/BuildTimeline/FilterTimeline.cpp
@@ -0,0 +1,20 @@
+#include "FilterTimeline.h"
+
+FilterTimeline::FilterTimeline(const Options &options)
+    : m_options(options)
+{
+}
+
+FilterTimeline::~FilterTimeline()
+{
+}
+
+bool FilterTimeline::Passes(const CppBI::Activities::Function& function) const
+{
+    return function.Duration() > m_options.IgnoreFunctionsUnder;
+}
+
+bool FilterTimeline::Passes(const CppBI::Activities::TemplateInstantiation& templateInstantiation) const
+{
+    return templateInstantiation.Duration() > m_options.IgnoreTemplatesUnder;
+}
diff --git a/src/BuildAnalyzer/Analyzers/BuildTimeline/FilterTimeline.h b/src/BuildAnalyzer/Analyzers/BuildTimeline/FilterTimeline.h
@@ -0,0 +1,26 @@
+#pragma once
+
+#include <chrono>
+
+#include <CppBuildInsights.hpp>
+namespace CppBI = Microsoft::Cpp::BuildInsights;
+
+class FilterTimeline
+{
+public:
+    struct Options
+    {
+        std::chrono::nanoseconds IgnoreFunctionsUnder = std::chrono::nanoseconds(0);
+        std::chrono::nanoseconds IgnoreTemplatesUnder = std::chrono::nanoseconds(0);
+    };
+
+public:
+    FilterTimeline(const Options& options);
+    ~FilterTimeline();
+
+    bool Passes(const CppBI::Activities::Function& function) const;
+    bool Passes(const CppBI::Activities::TemplateInstantiation& templateInstantiation) const;
+
+private:
+    Options m_options;
+};
diff --git a/src/BuildAnalyzer/Analyzers/BuildTimeline/FilterTimelineAnalyzer.cpp b/src/BuildAnalyzer/Analyzers/BuildTimeline/FilterTimelineAnalyzer.cpp
diff --git a/src/BuildAnalyzer/Analyzers/BuildTimeline/FilterTimelineAnalyzer.h b/src/BuildAnalyzer/Analyzers/BuildTimeline/FilterTimelineAnalyzer.h
diff --git a/src/BuildAnalyzer/BuildAnalyzer.cpp b/src/BuildAnalyzer/BuildAnalyzer.cpp
diff --git a/src/BuildAnalyzer/BuildAnalyzer.h b/src/BuildAnalyzer/BuildAnalyzer.h

Original file line number	Diff line number	Diff line change
`@@ -13,14 +13,8 @@`
`13`	`13`	`namespace`
`14`	`14`	`{`
`15`	`15`	`void AddEntry(const TimelineEntry* entry, rapidjson::Value& traceEvents, rapidjson::Document& document,`
`16`		`- const TProcessThreadRemappings& remappings, const TIgnoredEntries& ignoredEntries)`
	`16`	`+ const TProcessThreadRemappings& remappings)`
`17`	`17`	`{`
`18`		`- // apply filtering`
`19`		`- if (ignoredEntries.find(entry->GetId()) != ignoredEntries.end())`
`20`		`- {`
`21`		`- return;`
`22`		`- }`
`23`		`-`
`24`	`18`	`auto itRemap = remappings.find(entry->GetId());`
`25`	`19`	`const TProcessId& processId = itRemap != remappings.end() ? itRemap->second.ProcessId : entry->GetProcessId();`
`26`	`20`	`const TThreadId& threadId = itRemap != remappings.end() ? itRemap->second.ThreadId : entry->GetThreadId();`
`@@ -84,7 +78,7 @@ namespace`
`84`	`78`	`// write children entries`
`85`	`79`	`for (auto&& child : entry->GetChildren())`
`86`	`80`	`{`
`87`		`- AddEntry(child, traceEvents, document, remappings, ignoredEntries);`
	`81`	`+ AddEntry(child, traceEvents, document, remappings);`
`88`	`82`	`}`
`89`	`83`
`90`	`84`	`// write "end" event`
`@@ -103,9 +97,8 @@ namespace`
`103`	`97`	`}`
`104`	`98`	`}`
`105`	`99`
`106`		`-BuildTimelineExporter::BuildTimelineExporter(const BuildTimeline& timeline, const TIgnoredEntries& ignoredEntries)`
	`100`	`+BuildTimelineExporter::BuildTimelineExporter(const BuildTimeline& timeline)`
`107`	`101`	`: m_timeline(timeline)`
`108`		`- , m_ignoredEntries(ignoredEntries)`
`109`	`102`	`{`
`110`	`103`	`}`
`111`	`104`
`@@ -134,7 +127,7 @@ bool BuildTimelineExporter::ExportTo(const std::string& path) const`
`134`	`127`	`rapidjson::Value traceEvents(rapidjson::kArrayType);`
`135`	`128`	`for (auto&& root : m_timeline.GetRoots())`
`136`	`129`	`{`
`137`		`- AddEntry(root, traceEvents, document, m_timeline.GetProcessThreadRemappings(), m_ignoredEntries);`
	`130`	`+ AddEntry(root, traceEvents, document, m_timeline.GetProcessThreadRemappings());`
`138`	`131`	`}`
`139`	`132`	`document.AddMember("traceEvents", traceEvents, document.GetAllocator());`
`140`	`133`	`}`