From 7d0d89faae97b335c343f5a82221a2bdbf9eda3a Mon Sep 17 00:00:00 2001 From: Edward Hennis Date: Wed, 21 Oct 2015 16:22:19 -0400 Subject: [PATCH] Convert HashRouter to use aged_unordered_map. --- src/ripple/app/main/Application.cpp | 2 +- src/ripple/app/misc/HashRouter.cpp | 52 +++++------ src/ripple/app/misc/HashRouter.h | 39 ++++---- src/ripple/app/tests/HashRouter_test.cpp | 109 ++++++++++++++++------- src/ripple/test/jtx/impl/Env.cpp | 1 - 5 files changed, 115 insertions(+), 88 deletions(-) diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index ac29a04189..2e12919626 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -456,7 +456,7 @@ public: , mFeeTrack (std::make_unique(logs_->journal("LoadManager"))) , mHashRouter (std::make_unique( - HashRouter::getDefaultHoldTime ())) + stopwatch(), HashRouter::getDefaultHoldTime ())) , mValidations (make_Validations (*this)) diff --git a/src/ripple/app/misc/HashRouter.cpp b/src/ripple/app/misc/HashRouter.cpp index 609704a0ef..83a9c07a06 100644 --- a/src/ripple/app/misc/HashRouter.cpp +++ b/src/ripple/app/misc/HashRouter.cpp @@ -19,85 +19,73 @@ #include #include -#include -#include -#include namespace ripple { auto -HashRouter::emplace (uint256 const& index) +HashRouter::emplace (uint256 const& key) -> std::pair { - 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 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 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 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 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 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& peers, int flag) +bool HashRouter::swapSet (uint256 const& key, std::set& peers, int flag) { std::lock_guard lock (mMutex); - auto& s = emplace(index).first; + auto& s = emplace(key).first; if ((s.getFlags () & flag) == flag) return false; diff --git a/src/ripple/app/misc/HashRouter.h b/src/ripple/app/misc/HashRouter.h index e7abdb5daa..1690446a08 100644 --- a/src/ripple/app/misc/HashRouter.h +++ b/src/ripple/app/misc/HashRouter.h @@ -21,11 +21,10 @@ #define RIPPLE_APP_MISC_HASHROUTER_H_INCLUDED #include +#include #include #include -#include -#include -#include +#include namespace ripple { @@ -116,16 +115,16 @@ private: }; public: - // VFALCO NOTE this preferred alternative to default parameters makes - // behavior clear. - // - static inline int getDefaultHoldTime () + static inline std::chrono::seconds getDefaultHoldTime () { - return 300; + using namespace std::chrono; + + return 300s; } - explicit HashRouter (int entryHoldTimeInSeconds) - : mHoldTime (entryHoldTimeInSeconds) + HashRouter (Stopwatch& clock, std::chrono::seconds 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& peers, int flag); + bool swapSet (uint256 const& key, std::set& 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 mSuppressionMap; + beast::aged_unordered_map> mSuppressionMap; - // Stores all expiration times and the hashes indexed for them - std::map< int, std::list > mSuppressionTimes; - - int const mHoldTime; + std::chrono::seconds const mHoldTime; }; } // ripple diff --git a/src/ripple/app/tests/HashRouter_test.cpp b/src/ripple/app/tests/HashRouter_test.cpp index 95c9df759a..825f25ab1f 100644 --- a/src/ripple/app/tests/HashRouter_test.cpp +++ b/src/ripple/app/tests/HashRouter_test.cpp @@ -19,7 +19,7 @@ #include #include -#include +#include #include 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 - // key1 will be expired after the second + // Expiration is triggered by insertion, + // 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 - // key2 : 1 - // key3 : 2 + // key1 : 3 + // key2 : 3 + // key3 : 3 - UptimeTimer::getInstance().incrementElapsedTime(); - - // t=3 + ++stopwatch; + // t=4 // 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 - // key2 : 1 - // key3 : 2 + // key1 : 4 + // key2 : 4 + // key3 : 4 - UptimeTimer::getInstance().incrementElapsedTime(); - UptimeTimer::getInstance().incrementElapsedTime(); + ++stopwatch; + ++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 - // key2 : 5 - // key3 : 5 - // key4 : 5 + // key1 : 6 + // key2 : 6 + // key3 : 6 + // 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) } -} \ No newline at end of file +} diff --git a/src/ripple/test/jtx/impl/Env.cpp b/src/ripple/test/jtx/impl/Env.cpp index 74d772345e..81bb39becd 100644 --- a/src/ripple/test/jtx/impl/Env.cpp +++ b/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