refactor: make tr_recentHistory a template class (#3012)

This gives more flexibility on the size of the count bins, so we can save space on smaller counts

libtransmission/announcer.cc

@@ -12,6 +12,7 @@
 #include <ctime>
 #include <deque>
 #include <map>
+#include <numeric>
 #include <set>
 #include <string>
 #include <string_view>

libtransmission/benc.h

@@ -9,7 +9,6 @@
 #include <cerrno>
 #include <cstddef> // size_t
 #include <cstdint> // int64_t
-#include <cstdlib>
 #include <optional>
 #include <string_view>
 #include <utility> // make_pair

libtransmission/file-posix.cc

@@ -12,7 +12,6 @@
 #include <climits> /* PATH_MAX */
 #include <cstdint> /* SIZE_MAX */
 #include <cstdio>
-#include <cstdlib>
 #include <cstring>
 #include <string_view>
 #include <string>

libtransmission/history.h

@@ -10,15 +10,16 @@
 #endif
 
 #include <array>
-#include <cstddef> // size_t
+#include <cstdint>
 #include <ctime> // time_t
-#include <numeric> // std::accumulate
 
 /**
  * A short-term memory object that remembers how many times something
- * happened over the last N seconds. tr_peer uses it to count how many
- * bytes transferred to estimate the speed over the last N seconds.
+ * happened over the last Seconds seconds. tr_peer uses it to count
+ * how many bytes transferred to estimate the speed over the last
+ * Seconds seconds.
  */
+template<typename SizeType, std::size_t Seconds = 60>
 class tr_recentHistory
 {
 public:
@@ -27,15 +28,16 @@ public:
      * @param when the current time in sec, such as from tr_time()
      * @param n how many items to add to the history's counter
      */
-    void add(time_t now, size_t n)
+    void add(time_t now, SizeType n)
     {
-        if (slices[newest].time != now)
+        if (timestamps_[newest_] != now)
         {
-            newest = (newest + 1) % TR_RECENT_HISTORY_PERIOD_SEC;
-            slices[newest].time = now;
+            newest_ = (newest_ + 1) % Seconds;
+            timestamps_[newest_] = now;
+            count_[newest_] = {};
         }
 
-        slices[newest].n += n;
+        count_[newest_] += n;
     }
 
     /**
@@ -43,27 +45,24 @@ public:
      * @param when the current time in sec, such as from tr_time()
      * @param seconds how many seconds to count back through.
      */
-    size_t count(time_t now, unsigned int age_sec) const
+    SizeType count(time_t now, unsigned int age_sec) const
     {
+        auto sum = SizeType{};
         time_t const oldest = now - age_sec;
 
-        return std::accumulate(
-            std::begin(slices),
-            std::end(slices),
-            size_t{ 0 },
-            [&oldest](size_t sum, auto const& slice) { return slice.time >= oldest ? sum + slice.n : sum; });
+        for (std::size_t i = 0; i < Seconds; ++i)
+        {
+            if (timestamps_[i] >= oldest)
+            {
+                sum += count_[i];
+            }
+        }
+
+        return sum;
     }
 
 private:
-    inline auto static constexpr TR_RECENT_HISTORY_PERIOD_SEC = size_t{ 60 };
-
-    int newest = 0;
-
-    struct slice_t
-    {
-        size_t n = 0;
-        time_t time = 0;
-    };
-
-    std::array<slice_t, TR_RECENT_HISTORY_PERIOD_SEC> slices = {};
+    std::array<time_t, Seconds> timestamps_ = {};
+    std::array<SizeType, Seconds> count_ = {};
+    uint32_t newest_ = 0;
 };

libtransmission/peer-common.h

@@ -103,11 +103,11 @@ public:
        For BitTorrent peers, this is the app name derived from the `v' string in LTEP's handshake dictionary */
     tr_interned_string client;
 
-    tr_recentHistory blocksSentToClient;
-    tr_recentHistory blocksSentToPeer;
+    tr_recentHistory<uint16_t> blocksSentToClient;
+    tr_recentHistory<uint16_t> blocksSentToPeer;
 
-    tr_recentHistory cancelsSentToClient;
-    tr_recentHistory cancelsSentToPeer;
+    tr_recentHistory<uint16_t> cancelsSentToClient;
+    tr_recentHistory<uint16_t> cancelsSentToPeer;
 };
 
 /** Update the tr_peer.progress field based on the 'have' bitset. */

libtransmission/peer-mgr.cc

@@ -10,8 +10,9 @@
 #include <cstdint>
 #include <cstdlib> /* qsort */
 #include <ctime> // time_t
-#include <tuple> // std::tie
 #include <iterator> // std::back_inserter
+#include <numeric> // std::accumulate
+#include <tuple> // std::tie
 #include <utility>
 #include <vector>
 
@@ -1921,7 +1922,6 @@ static void rechokeDownloads(tr_swarm* s)
         {
             auto const* const peer = static_cast<tr_peer const*>(tr_ptrArrayNth(&s->peers, i));
             auto const b = peer->blocksSentToClient.count(now, CancelHistorySec);
-            auto const c = peer->cancelsSentToPeer.count(now, CancelHistorySec);
 
             if (b == 0) /* ignore unresponsive peers, as described above */
             {
@@ -1929,7 +1929,7 @@ static void rechokeDownloads(tr_swarm* s)
             }
 
             blocks += b;
-            cancels += c;
+            cancels += peer->cancelsSentToPeer.count(now, CancelHistorySec);
         }
 
         if (cancels > 0)

libtransmission/session.cc

@@ -9,7 +9,6 @@
 #include <condition_variable>
 #include <csignal>
 #include <cstdint>
-#include <cstdlib>
 #include <ctime>
 #include <iterator> // std::back_inserter
 #include <list>

tests/libtransmission/history-test.cc

@@ -10,7 +10,7 @@
 
 TEST(History, recentHistory)
 {
-    auto h = tr_recentHistory{};
+    auto h = tr_recentHistory<size_t, 60>{};
 
     h.add(10000, 1);
     EXPECT_EQ(0U, h.count(12000, 1000));