Convert HashRouter to use aged_unordered_map.

src/ripple/app/main/Application.cpp

@@ -456,7 +456,7 @@ public:
         , mFeeTrack (std::make_unique<LoadFeeTrack>(logs_->journal("LoadManager")))
 
         , mHashRouter (std::make_unique<HashRouter>(
-            HashRouter::getDefaultHoldTime ()))
+            stopwatch(), HashRouter::getDefaultHoldTime ()))
 
         , mValidations (make_Validations (*this))
 

src/ripple/app/misc/HashRouter.cpp

@@ -19,85 +19,73 @@
 
 #include <BeastConfig.h>
 #include <ripple/app/misc/HashRouter.h>
-#include <ripple/basics/UptimeTimer.h>
-#include <map>
-#include <mutex>
 
 namespace ripple {
 
 auto
-HashRouter::emplace (uint256 const& index)
+HashRouter::emplace (uint256 const& key)
     -> std::pair<Entry&, bool>
 {
-    auto fit = mSuppressionMap.find (index);
+    auto iter = mSuppressionMap.find (key);
 
-    if (fit != mSuppressionMap.end ())
+    if (iter != mSuppressionMap.end ())
     {
+        mSuppressionMap.touch(iter);
         return std::make_pair(
-            std::ref(fit->second), false);
+            std::ref(iter->second), false);
     }
 
-    int elapsed = UptimeTimer::getInstance ().getElapsedSeconds ();
-    int expireCutoff = elapsed - mHoldTime;
-
     // See if any supressions need to be expired
-    auto it = mSuppressionTimes.begin ();
+    expire(mSuppressionMap,
+        mHoldTime);
 
-    while ((it != mSuppressionTimes.end ()) && (it->first <= expireCutoff))
-    {
-        for(auto const& lit : it->second)
-            mSuppressionMap.erase (lit);
-        it = mSuppressionTimes.erase (it);
-    }
-
-    mSuppressionTimes[elapsed].push_back (index);
     return std::make_pair(std::ref(
         mSuppressionMap.emplace (
-            index, Entry ()).first->second),
+            key, Entry ()).first->second),
                 true);
 }
 
-void HashRouter::addSuppression (uint256 const& index)
+void HashRouter::addSuppression (uint256 const& key)
 {
     std::lock_guard <std::mutex> lock (mMutex);
 
-    emplace (index);
+    emplace (key);
 }
 
-bool HashRouter::addSuppressionPeer (uint256 const& index, PeerShortID peer)
+bool HashRouter::addSuppressionPeer (uint256 const& key, PeerShortID peer)
 {
     std::lock_guard <std::mutex> lock (mMutex);
 
-    auto result = emplace(index);
+    auto result = emplace(key);
     result.first.addPeer(peer);
     return result.second;
 }
 
-bool HashRouter::addSuppressionPeer (uint256 const& index, PeerShortID peer, int& flags)
+bool HashRouter::addSuppressionPeer (uint256 const& key, PeerShortID peer, int& flags)
 {
     std::lock_guard <std::mutex> lock (mMutex);
 
-    auto result = emplace(index);
+    auto result = emplace(key);
     auto& s = result.first;
     s.addPeer (peer);
     flags = s.getFlags ();
     return result.second;
 }
 
-int HashRouter::getFlags (uint256 const& index)
+int HashRouter::getFlags (uint256 const& key)
 {
     std::lock_guard <std::mutex> lock (mMutex);
 
-    return emplace(index).first.getFlags ();
+    return emplace(key).first.getFlags ();
 }
 
-bool HashRouter::setFlags (uint256 const& index, int flags)
+bool HashRouter::setFlags (uint256 const& key, int flags)
 {
     assert (flags != 0);
 
     std::lock_guard <std::mutex> lock (mMutex);
 
-    auto& s = emplace(index).first;
+    auto& s = emplace(key).first;
 
     if ((s.getFlags () & flags) == flags)
         return false;
@@ -106,11 +94,11 @@ bool HashRouter::setFlags (uint256 const& index, int flags)
     return true;
 }
 
-bool HashRouter::swapSet (uint256 const& index, std::set<PeerShortID>& peers, int flag)
+bool HashRouter::swapSet (uint256 const& key, std::set<PeerShortID>& peers, int flag)
 {
     std::lock_guard <std::mutex> lock (mMutex);
 
-    auto& s = emplace(index).first;
+    auto& s = emplace(key).first;
 
     if ((s.getFlags () & flag) == flag)
         return false;

src/ripple/app/misc/HashRouter.h

@@ -21,11 +21,10 @@
 #define RIPPLE_APP_MISC_HASHROUTER_H_INCLUDED
 
 #include <ripple/basics/base_uint.h>
+#include <ripple/basics/chrono.h>
 #include <ripple/basics/CountedObject.h>
 #include <ripple/basics/UnorderedContainers.h>
-#include <cstdint>
+#include <beast/container/aged_unordered_map.h>
-#include <functional>
-#include <set>
 
 namespace ripple {
 
@@ -116,16 +115,16 @@ private:
     };
 
 public:
-    // VFALCO NOTE this preferred alternative to default parameters makes
+    static inline std::chrono::seconds getDefaultHoldTime ()
-    //         behavior clear.
-    //
-    static inline int getDefaultHoldTime ()
     {
-        return 300;
+        using namespace std::chrono;
+
+        return 300s;
     }
 
-    explicit HashRouter (int entryHoldTimeInSeconds)
+    HashRouter (Stopwatch& clock, std::chrono::seconds entryHoldTimeInSeconds)
-        : mHoldTime (entryHoldTimeInSeconds)
+        : mSuppressionMap(clock)
+        , mHoldTime (entryHoldTimeInSeconds)
     {
     }
 
@@ -135,22 +134,22 @@ public:
 
     // VFALCO TODO Replace "Supression" terminology with something more
     // semantically meaningful.
-    void addSuppression(uint256 const& index);
+    void addSuppression(uint256 const& key);
 
-    bool addSuppressionPeer (uint256 const& index, PeerShortID peer);
+    bool addSuppressionPeer (uint256 const& key, PeerShortID peer);
 
-    bool addSuppressionPeer (uint256 const& index, PeerShortID peer,
+    bool addSuppressionPeer (uint256 const& key, PeerShortID peer,
                              int& flags);
 
     /** Set the flags on a hash.
 
         @return `true` if the flags were changed. `false` if unchanged.
     */
-    bool setFlags (uint256 const& index, int flags);
+    bool setFlags (uint256 const& key, int flags);
 
-    int getFlags (uint256 const& index);
+    int getFlags (uint256 const& key);
 
-    bool swapSet (uint256 const& index, std::set<PeerShortID>& peers, int flag);
+    bool swapSet (uint256 const& key, std::set<PeerShortID>& peers, int flag);
 
 private:
     // pair.second indicates whether the entry was created
@@ -159,12 +158,10 @@ private:
     std::mutex mutable mMutex;
 
     // Stores all suppressed hashes and their expiration time
-    hash_map <uint256, Entry> mSuppressionMap;
+    beast::aged_unordered_map<uint256, Entry, Stopwatch::clock_type,
+        hardened_hash<strong_hash>> mSuppressionMap;
 
-    // Stores all expiration times and the hashes indexed for them
+    std::chrono::seconds const mHoldTime;
-    std::map< int, std::list<uint256> > mSuppressionTimes;
-
-    int const mHoldTime;
 };
 
 } // ripple

src/ripple/app/tests/HashRouter_test.cpp

@@ -19,7 +19,7 @@
 
 #include <BeastConfig.h>
 #include <ripple/app/misc/HashRouter.h>
-#include <ripple/basics/UptimeTimer.h>
+#include <ripple/basics/chrono.h>
 #include <beast/unit_test/suite.h>
 
 namespace ripple {
@@ -27,10 +27,47 @@ namespace test {
 
 class HashRouter_test : public beast::unit_test::suite
 {
+    void
+    testNonExpiration()
+    {
+        TestStopwatch stopwatch;
+        HashRouter router(stopwatch, std::chrono::seconds(2));
+
+        uint256 const key1(1);
+        uint256 const key2(2);
+        uint256 const key3(3);
+
+        // t=0
+        router.setFlags(key1, 11111);
+        expect(router.getFlags(key1) == 11111);
+        router.setFlags(key2, 22222);
+        expect(router.getFlags(key2) == 22222);
+        // key1 : 0
+        // key2 : 0
+        // key3: null
+
+        ++stopwatch;
+
+        // Because we are accessing key1 here, it
+        // will NOT be expired for another two ticks
+        expect(router.getFlags(key1) == 11111);
+        // key1 : 1
+        // key2 : 0
+        // key3 null
+
+        ++stopwatch;
+
+        // t=3
+        router.setFlags(key3,33333); // force expiration
+        expect(router.getFlags(key1) == 11111);
+        expect(router.getFlags(key2) == 0);
+    }
+
     void
     testExpiration()
     {
-        HashRouter router(2);
+        TestStopwatch stopwatch;
+        HashRouter router(stopwatch, std::chrono::seconds(2));
 
         uint256 const key1(1);
         uint256 const key2(2);
@@ -47,61 +84,68 @@ class HashRouter_test : public beast::unit_test::suite
         // key2 : null
         // key3 : null
 
-        UptimeTimer::getInstance().incrementElapsedTime();
+        ++stopwatch;
 
-        // Expiration is triggered by insertion, so
+        // Expiration is triggered by insertion,
-        // key1 will be expired after the second
+        // and timestamps are updated on access,
+        // so key1 will be expired after the second
         // call to setFlags.
         // t=1
         router.setFlags(key2, 9999);
         expect(router.getFlags(key1) == 12345);
         expect(router.getFlags(key2) == 9999);
-        // key1 : 0
+        // key1 : 1
         // key2 : 1
         // key3 : null
 
-        UptimeTimer::getInstance().incrementElapsedTime();
+        ++stopwatch;
-
         // t=2
+        expect(router.getFlags(key2) == 9999);
+        // key1 : 1
+        // key2 : 2
+        // key3 : null
+
+        ++stopwatch;
+        // t=3
         router.setFlags(key3, 2222);
         expect(router.getFlags(key1) == 0);
         expect(router.getFlags(key2) == 9999);
         expect(router.getFlags(key3) == 2222);
-        // key1 : 2
+        // key1 : 3
-        // key2 : 1
+        // key2 : 3
-        // key3 : 2
+        // key3 : 3
 
-        UptimeTimer::getInstance().incrementElapsedTime();
+        ++stopwatch;
-
+        // t=4
-        // t=3
         // No insertion, no expiration
         router.setFlags(key1, 7654);
         expect(router.getFlags(key1) == 7654);
         expect(router.getFlags(key2) == 9999);
         expect(router.getFlags(key3) == 2222);
-        // key1 : 2
+        // key1 : 4
-        // key2 : 1
+        // key2 : 4
-        // key3 : 2
+        // key3 : 4
 
-        UptimeTimer::getInstance().incrementElapsedTime();
+        ++stopwatch;
-        UptimeTimer::getInstance().incrementElapsedTime();
+        ++stopwatch;
 
-        // t=5
+        // t=6
         router.setFlags(key4, 7890);
         expect(router.getFlags(key1) == 0);
         expect(router.getFlags(key2) == 0);
         expect(router.getFlags(key3) == 0);
         expect(router.getFlags(key4) == 7890);
-        // key1 : 5
+        // key1 : 6
-        // key2 : 5
+        // key2 : 6
-        // key3 : 5
+        // key3 : 6
-        // key4 : 5
+        // key4 : 6
     }
 
     void testSuppression()
     {
         // Normal HashRouter
-        HashRouter router(2);
+        TestStopwatch stopwatch;
+        HashRouter router(stopwatch, std::chrono::seconds(2));
 
         uint256 const key1(1);
         uint256 const key2(2);
@@ -117,7 +161,7 @@ class HashRouter_test : public beast::unit_test::suite
         expect(router.addSuppressionPeer(key3, 20, flags));
         expect(flags == 0);
 
-        UptimeTimer::getInstance().incrementElapsedTime();
+        ++stopwatch;
 
         expect(!router.addSuppressionPeer(key1, 2));
         expect(!router.addSuppressionPeer(key2, 3));
@@ -129,7 +173,8 @@ class HashRouter_test : public beast::unit_test::suite
     void
     testSetFlags()
     {
-        HashRouter router(2);
+        TestStopwatch stopwatch;
+        HashRouter router(stopwatch, std::chrono::seconds(2));
 
         uint256 const key1(1);
         expect(router.setFlags(key1, 10));
@@ -140,7 +185,8 @@ class HashRouter_test : public beast::unit_test::suite
     void
     testSwapSet()
     {
-        HashRouter router(2);
+        TestStopwatch stopwatch;
+        HashRouter router(stopwatch, std::chrono::seconds(2));
 
         uint256 const key1(1);
 
@@ -171,18 +217,15 @@ public:
     void
     run()
     {
-        UptimeTimer::getInstance().beginManualUpdates();
+        testNonExpiration();
-
         testExpiration();
         testSuppression();
         testSetFlags();
         testSwapSet();
-
-        UptimeTimer::getInstance().endManualUpdates();
     }
 };
 
 BEAST_DEFINE_TESTSUITE(HashRouter, app, ripple)
 
 }
-}
+}

src/ripple/test/jtx/impl/Env.cpp

@@ -129,7 +129,6 @@ Env::close(NetClock::time_point const& closeTime,
         OpenView accum(&*next);
         OpenLedger::apply(app(), accum, *closed_,
             txs, retries, applyFlags(), journal);
-
         accum.apply(*next);
     }
     // To ensure that the close time is exact and not rounded, we don't