Rewrite & optimize LimitedQueue (Chatterino#3798)

* Use circular buffer for LimitedQueue * Reduce copying of snapshot * Small optimizations * Remove unneeded lock statements * Add LimitedQueue tests * Fix includes for limited queue benchmark * Update CHANGELOG.md * Use correct boost version iterators * Use a shared_mutex to clarify reads and writes * Update `find`/`rfind` to return the result as a boost::optional * Use `[[nodiscard]]` where applicable * Update comments * Add a couple more doc comments * Replace size with get get is a safe (locked & checked) version of at * Use std::vector in LimitedQueueSnapshot * Update LimitedQueue benchmarks * Add mutex guard to buffer accessors We do not know whether T is an atomic type or not so we can't safely say that we can copy the value at a certain address of the buffer. See https://stackoverflow.com/a/2252478 * Update doc comments, add first/last getters * Make limit_ const * Omit `else` if the if-case always returns * Title case category comments * Remove `at` * Fix `get` comment * Privatize/comment/lock property accessors - `limit` is now private - `space` is now private - `full` has been removed - `empty` now locks * Remove `front` function * Remove `back` method * Add comment to `first` * Add comment to `last` Co-authored-by: Rasmus Karlsson <[email protected]>
templateK · Jun 18, 2022 · 81caf1a · 81caf1a
1 parent f3f3403
commit 81caf1a
Show file tree

Hide file tree

Showing 12 changed files with 520 additions and 285 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -33,6 +33,7 @@
 - Bugfix: Fixed automod queue pubsub topic persisting after user change. (#3718)
 - Bugfix: Fixed viewer list not closing after pressing escape key. (#3734)
 - Bugfix: Fixed links with no thumbnail having previous link's thumbnail. (#3720)
+- Dev: Rewrite LimitedQueue (#3798)
 - Dev: Overhaul highlight system by moving all checks into a Controller allowing for easier tests. (#3399, #3801)
 - Dev: Use Game Name returned by Get Streams instead of querying it from the Get Games API. (#3662)
 - Dev: Batch checking live status for all channels after startup. (#3757, #3762, #3767)

diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt
@@ -6,6 +6,7 @@ set(benchmark_SOURCES
     ${CMAKE_CURRENT_LIST_DIR}/src/Highlights.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/FormatTime.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/Helpers.cpp
+    ${CMAKE_CURRENT_LIST_DIR}/src/LimitedQueue.cpp
     # Add your new file above this line!
     )
 

diff --git a/benchmarks/src/LimitedQueue.cpp b/benchmarks/src/LimitedQueue.cpp
@@ -0,0 +1,116 @@
+#include "messages/LimitedQueue.hpp"
+
+#include <benchmark/benchmark.h>
+
+#include <memory>
+#include <numeric>
+#include <vector>
+
+using namespace chatterino;
+
+void BM_LimitedQueue_PushBack(benchmark::State &state)
+{
+    LimitedQueue<int> queue(1000);
+    for (auto _ : state)
+    {
+        queue.pushBack(1);
+    }
+}
+
+void BM_LimitedQueue_PushFront_One(benchmark::State &state)
+{
+    std::vector<int> items = {1};
+    LimitedQueue<int> queue(2);
+
+    for (auto _ : state)
+    {
+        state.PauseTiming();
+        queue.clear();
+        state.ResumeTiming();
+        queue.pushFront(items);
+    }
+}
+
+void BM_LimitedQueue_PushFront_Many(benchmark::State &state)
+{
+    std::vector<int> items;
+    items.resize(10000);
+    std::iota(items.begin(), items.end(), 0);
+
+    for (auto _ : state)
+    {
+        state.PauseTiming();
+        LimitedQueue<int> queue(1000);
+        state.ResumeTiming();
+        queue.pushFront(items);
+    }
+}
+
+void BM_LimitedQueue_Replace(benchmark::State &state)
+{
+    LimitedQueue<int> queue(1000);
+    for (int i = 0; i < 1000; ++i)
+    {
+        queue.pushBack(i);
+    }
+
+    for (auto _ : state)
+    {
+        queue.replaceItem(500, 500);
+    }
+}
+
+void BM_LimitedQueue_Snapshot(benchmark::State &state)
+{
+    LimitedQueue<int> queue(1000);
+    for (int i = 0; i < 1000; ++i)
+    {
+        queue.pushBack(i);
+    }
+
+    for (auto _ : state)
+    {
+        auto snapshot = queue.getSnapshot();
+        benchmark::DoNotOptimize(snapshot);
+    }
+}
+
+void BM_LimitedQueue_Snapshot_ExpensiveCopy(benchmark::State &state)
+{
+    LimitedQueue<std::shared_ptr<int>> queue(1000);
+    for (int i = 0; i < 1000; ++i)
+    {
+        queue.pushBack(std::make_shared<int>(i));
+    }
+
+    for (auto _ : state)
+    {
+        auto snapshot = queue.getSnapshot();
+        benchmark::DoNotOptimize(snapshot);
+    }
+}
+
+void BM_LimitedQueue_Find(benchmark::State &state)
+{
+    LimitedQueue<int> queue(1000);
+    for (int i = 0; i < 10000; ++i)
+    {
+        queue.pushBack(i);
+    }
+
+    for (auto _ : state)
+    {
+        auto res = queue.find([](const auto &val) {
+            return val == 500;
+        });
+        benchmark::DoNotOptimize(res);
+    }
+}
+
+BENCHMARK(BM_LimitedQueue_PushBack);
+BENCHMARK(BM_LimitedQueue_PushFront_One);
+BENCHMARK(BM_LimitedQueue_PushFront_Many);
+BENCHMARK(BM_LimitedQueue_Replace);
+BENCHMARK(BM_LimitedQueue_Snapshot);
+BENCHMARK(BM_LimitedQueue_Snapshot_ExpensiveCopy);
+BENCHMARK(BM_LimitedQueue_Find);
diff --git a/src/common/Channel.cpp b/src/common/Channel.cpp
@@ -246,23 +246,20 @@ void Channel::deleteMessage(QString messageID)
         msg->flags.set(MessageFlag::Disabled);
     }
 }
+
 MessagePtr Channel::findMessage(QString messageID)
 {
-    LimitedQueueSnapshot<MessagePtr> snapshot = this->getMessageSnapshot();
-    int snapshotLength = snapshot.size();
+    MessagePtr res;
 
-    int end = std::max(0, snapshotLength - 200);
-
-    for (int i = snapshotLength - 1; i >= end; --i)
+    if (auto msg = this->messages_.rfind([&messageID](const MessagePtr &msg) {
+            return msg->id == messageID;
+        });
+        msg)
     {
-        auto &s = snapshot[i];
-
-        if (s->id == messageID)
-        {
-            return s;
-        }
+        res = *msg;
     }
-    return nullptr;
+
+    return res;
 }
 
 bool Channel::canSendMessage() const