From 2da817615ed6452f5989f52bfd54778b43bc1d49 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Fri, 13 Aug 2021 21:55:14 +0200
Subject: [PATCH] Profiler: Store signposts in the main event stream

Instead of keeping a separate Vector<Event> for signposts, let them live
in the main event stream. For fast iteration, we instead keep a cache of
the signpost event indices.
---
 Userland/DevTools/Profiler/Profile.cpp       | 18 ++++++++----------
 Userland/DevTools/Profiler/Profile.h         | 17 +++++++++++++----
 Userland/DevTools/Profiler/TimelineTrack.cpp | 11 +++++------
 3 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/Userland/DevTools/Profiler/Profile.cpp b/Userland/DevTools/Profiler/Profile.cpp
index 0d9dbc6b66..f84b01b63f 100644
--- a/Userland/DevTools/Profiler/Profile.cpp
+++ b/Userland/DevTools/Profiler/Profile.cpp
@@ -31,11 +31,15 @@ static void sort_profile_nodes(Vector<NonnullRefPtr<ProfileNode>>& nodes)
         child->sort_children();
 }
 
-Profile::Profile(Vector<Process> processes, Vector<Event> events, Vector<Event> signposts)
+Profile::Profile(Vector<Process> processes, Vector<Event> events)
     : m_processes(move(processes))
     , m_events(move(events))
-    , m_signposts(move(signposts))
 {
+    for (size_t i = 0; i < m_events.size(); ++i) {
+        if (m_events[i].data.has<Event::SignpostData>())
+            m_signpost_indices.append(i);
+    }
+
     m_first_timestamp = m_events.first().timestamp;
     m_last_timestamp = m_events.last().timestamp;
 
@@ -231,7 +235,6 @@ Result<NonnullOwnPtr<Profile>, String> Profile::load_from_perfcore_file(const St
     NonnullOwnPtrVector<Process> all_processes;
     HashMap<pid_t, Process*> current_processes;
     Vector<Event> events;
-    Vector<Event> signposts;
     EventSerialNumber next_serial;
 
     for (auto& perf_event_value : perf_events.values()) {
@@ -393,12 +396,7 @@ Result<NonnullOwnPtr<Profile>, String> Profile::load_from_perfcore_file(const St
         FlatPtr innermost_frame_address = event.frames.at(1).address;
         event.in_kernel = maybe_kernel_base.has_value() && innermost_frame_address >= maybe_kernel_base.value();
 
-        dbgln("size: {}", sizeof(Event));
-
-        if (event.data.has<Event::SignpostData>())
-            signposts.append(move(event));
-        else
-            events.append(move(event));
+        events.append(move(event));
     }
 
     if (events.is_empty())
@@ -415,7 +413,7 @@ Result<NonnullOwnPtr<Profile>, String> Profile::load_from_perfcore_file(const St
     for (auto& it : all_processes)
         processes.append(move(it));
 
-    return adopt_own(*new Profile(move(processes), move(events), move(signposts)));
+    return adopt_own(*new Profile(move(processes), move(events)));
 }
 
 void ProfileNode::sort_children()
diff --git a/Userland/DevTools/Profiler/Profile.h b/Userland/DevTools/Profiler/Profile.h
index d7f07204d5..f665513797 100644
--- a/Userland/DevTools/Profiler/Profile.h
+++ b/Userland/DevTools/Profiler/Profile.h
@@ -217,8 +217,7 @@ public:
         Variant<std::nullptr_t, SampleData, MallocData, FreeData, SignpostData, MmapData, MunmapData, ProcessCreateData, ProcessExecData, ThreadCreateData> data { nullptr };
     };
 
-    const Vector<Event>& events() const { return m_events; }
-    Vector<Event> const& signposts() const { return m_signposts; }
+    Vector<Event> const& events() const { return m_events; }
     const Vector<size_t>& filtered_event_indices() const { return m_filtered_event_indices; }
 
     u64 length_in_ms() const { return m_last_timestamp - m_first_timestamp; }
@@ -258,8 +257,18 @@ public:
         }
     }
 
+    template<typename Callback>
+    void for_each_signpost(Callback callback) const
+    {
+        for (auto index : m_signpost_indices) {
+            auto& event = m_events[index];
+            if (callback(event) == IterationDecision::Break)
+                break;
+        }
+    }
+
 private:
-    Profile(Vector<Process>, Vector<Event> events, Vector<Event> signposts);
+    Profile(Vector<Process>, Vector<Event>);
 
     void rebuild_tree();
 
@@ -276,7 +285,7 @@ private:
 
     Vector<Process> m_processes;
     Vector<Event> m_events;
-    Vector<Event> m_signposts;
+    Vector<size_t> m_signpost_indices;
 
     bool m_has_timestamp_filter_range { false };
     u64 m_timestamp_filter_range_start { 0 };
diff --git a/Userland/DevTools/Profiler/TimelineTrack.cpp b/Userland/DevTools/Profiler/TimelineTrack.cpp
index aa87042124..def00478c9 100644
--- a/Userland/DevTools/Profiler/TimelineTrack.cpp
+++ b/Userland/DevTools/Profiler/TimelineTrack.cpp
@@ -113,16 +113,15 @@ void TimelineTrack::paint_event(GUI::PaintEvent& event)
 template<typename Callback>
 void TimelineTrack::for_each_signpost(Callback callback)
 {
-    for (auto& signpost : m_profile.signposts()) {
+    m_profile.for_each_signpost([&](auto& signpost) {
         if (signpost.pid != m_process.pid)
-            continue;
+            return IterationDecision::Continue;
 
         if (!m_process.valid_at(signpost.serial))
-            continue;
+            return IterationDecision::Continue;
 
-        if (callback(signpost) == IterationDecision::Break)
-            break;
-    }
+        return callback(signpost);
+    });
 }
 
 void TimelineTrack::mousemove_event(GUI::MouseEvent& event)