From 986d44873a464fe97d09a26260f55e7e7f3f106c Mon Sep 17 00:00:00 2001
From: Audun Foyen <audunf@gnokk.no>
Date: Tue, 20 Jul 2021 21:19:35 +0200
Subject: [PATCH] Issue-108. Track recent packets in unordered_set

Check individual packets seen recently for expiry - and purge.
Otherwise - only scan all of recentPackets for expired once
fill > 75% (of MAX_NUM_NODES).
---
 src/mesh/PacketHistory.cpp | 74 ++++++++++++++++++++++++--------------
 src/mesh/PacketHistory.h   | 29 ++-------------
 2 files changed, 51 insertions(+), 52 deletions(-)

diff --git a/src/mesh/PacketHistory.cpp b/src/mesh/PacketHistory.cpp
index 6a2cb2c9b..c8008689d 100644
--- a/src/mesh/PacketHistory.cpp
+++ b/src/mesh/PacketHistory.cpp
@@ -19,35 +19,57 @@ bool PacketHistory::wasSeenRecently(const MeshPacket *p, bool withUpdate)
     }
 
     uint32_t now = millis();
-    for (size_t i = 0; i < recentPackets.size();) {
-        PacketRecord &r = recentPackets[i];
 
-        if ((now - r.rxTimeMsec) >= FLOOD_EXPIRE_TIME) {
-            // DEBUG_MSG("Deleting old broadcast record %d\n", i);
-            recentPackets.erase(recentPackets.begin() + i); // delete old record
+    PacketRecord r;
+    r.id = p->id;
+    r.sender = getFrom(p);
+    r.rxTimeMsec = now;
+
+    auto found = recentPackets.find(r);
+    bool seenRecently = (found != recentPackets.end());  // found not equal to .end() means packet was seen recently
+
+    if (seenRecently && (now - found->rxTimeMsec) >= FLOOD_EXPIRE_TIME) { // Check whether found packet has already expired
+        recentPackets.erase(found);  // Erase and pretend packet has not been seen recently
+        found = recentPackets.end();
+        seenRecently = false;
+    }
+
+    if (seenRecently) {
+        DEBUG_MSG("Found existing packet record for fr=0x%x,to=0x%x,id=0x%x\n", p->from, p->to, p->id);
+    }
+
+    if (withUpdate) {
+        if (found != recentPackets.end()) { // delete existing to updated timestamp (re-insert)
+            recentPackets.erase(found);     // as unsorted_set::iterator is const (can't update timestamp - so re-insert..)
+        }
+        recentPackets.insert(r);
+        printPacket("Add packet record", p);
+    }
+
+    // Capacity is reerved, so only purge expired packets if recentPackets fills past 90% capacity
+    // Expiry is normally dealt with after having searched/found a packet (above)
+    if (recentPackets.size() > (MAX_NUM_NODES * 0.9)) {
+        clearExpiredRecentPackets();
+    }
+
+    return seenRecently;
+}
+
+/**
+ * Iterate through all recent packets, and remove all older than FLOOD_EXPIRE_TIME
+ */
+void PacketHistory::clearExpiredRecentPackets() {
+    uint32_t now = millis();
+
+    DEBUG_MSG("recentPackets size=%ld\n", recentPackets.size());
+
+    for (auto it = recentPackets.begin(); it != recentPackets.end(); ) {
+        if ((now - it->rxTimeMsec) >= FLOOD_EXPIRE_TIME) {
+            it = recentPackets.erase(it); // erase returns iterator pointing to element immediately following the one erased
         } else {
-            if (r.id == p->id && r.sender == getFrom(p)) {
-                DEBUG_MSG("Found existing packet record for fr=0x%x,to=0x%x,id=0x%x\n", p->from, p->to, p->id);
-
-                // Update the time on this record to now
-                if (withUpdate)
-                    r.rxTimeMsec = now;
-                return true;
-            }
-
-            i++;
+            ++it;
         }
     }
 
-    // Didn't find an existing record, make one
-    if (withUpdate) {
-        PacketRecord r;
-        r.id = p->id;
-        r.sender = getFrom(p);
-        r.rxTimeMsec = now;
-        recentPackets.push_back(r);
-        printPacket("Adding packet record", p);
-    }
-
-    return false;
+    DEBUG_MSG("recentPackets size=%ld (after clearing expired packets)\n", recentPackets.size());
 }
\ No newline at end of file
diff --git a/src/mesh/PacketHistory.h b/src/mesh/PacketHistory.h
index 8deee092e..d44a3d369 100644
--- a/src/mesh/PacketHistory.h
+++ b/src/mesh/PacketHistory.h
@@ -1,7 +1,6 @@
 #pragma once
 
 #include "Router.h"
-#include <queue>
 #include <unordered_set>
 
 /// We clear our old flood record five minute after we see the last of it
@@ -24,37 +23,15 @@ class PacketRecordHashFunction
     size_t operator()(const PacketRecord &p) const { return (std::hash<NodeNum>()(p.sender)) ^ (std::hash<PacketId>()(p.id)); }
 };
 
-/// Order packet records by arrival time, we want the oldest packets to be in the front of our heap
-class PacketRecordOrderFunction
-{
-  public:
-    size_t operator()(const PacketRecord &p1, const PacketRecord &p2) const
-    {
-        // If the timer ticks have rolled over the difference between times will be _enormous_.  Handle that case specially
-        uint32_t t1 = p1.rxTimeMsec, t2 = p2.rxTimeMsec;
-
-        if (t1 - t2 > UINT32_MAX / 2) {
-            // time must have rolled over, swap them because the new little number is 'bigger' than the old big number
-            t1 = t2;
-            t2 = p1.rxTimeMsec;
-        }
-
-        return t1 > t2;
-    }
-};
-
 /**
  * This is a mixin that adds a record of past packets we have seen
  */
 class PacketHistory
 {
   private:
-    /** FIXME: really should be a std::unordered_set with the key being sender,id.
-     * This would make checking packets in wasSeenRecently faster.
-     */
-    std::vector<PacketRecord> recentPackets;
-    // priority_queue<PacketRecord, vector<PacketRecord>, PacketRecordOrderFunction> arrivalTimes;
-    // unordered_set<PacketRecord, PacketRecordHashFunction> recentPackets;
+    std::unordered_set<PacketRecord, PacketRecordHashFunction> recentPackets;
+
+    void clearExpiredRecentPackets(); // clear all recentPackets older than FLOOD_EXPIRE_TIME
 
   public:
     PacketHistory();