From 2e25836cf398f6472c425e7d3bd0fdc70eecbe2b Mon Sep 17 00:00:00 2001 From: Hidenori Shinohara Date: Fri, 23 Oct 2020 11:53:42 -0400 Subject: [PATCH 1/3] Minor optimization of contractToMaximalQuorum --- src/herder/QuorumIntersectionCheckerImpl.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/herder/QuorumIntersectionCheckerImpl.cpp b/src/herder/QuorumIntersectionCheckerImpl.cpp index 367254d32d..3b7391c801 100644 --- a/src/herder/QuorumIntersectionCheckerImpl.cpp +++ b/src/herder/QuorumIntersectionCheckerImpl.cpp @@ -478,16 +478,15 @@ QuorumIntersectionCheckerImpl::contractToMaximalQuorum(BitSet nodes) const } while (true) { - BitSet filtered(nodes.count()); + BitSet filtered(nodes); for (size_t i = 0; nodes.nextSet(i); ++i) { - if (containsQuorumSliceForNode(nodes, i)) + if (containsQuorumSliceForNode(filtered, i)) { if (mLogTrace) { CLOG(TRACE, "SCP") << "Have qslice for " << i; } - filtered.set(i); } else { @@ -495,6 +494,7 @@ QuorumIntersectionCheckerImpl::contractToMaximalQuorum(BitSet nodes) const { CLOG(TRACE, "SCP") << "Missing qslice for " << i; } + filtered.unset(i); } } if (filtered.count() == nodes.count() || filtered.empty()) From afde58305e3b655b67831ce23d2fb50e78fd54b0 Mon Sep 17 00:00:00 2001 From: Hidenori Shinohara Date: Wed, 11 Nov 2020 16:09:15 -0500 Subject: [PATCH 2/3] Add maybeGet to RandomEvictionCache --- src/util/RandomEvictionCache.h | 23 +++++++-- src/util/test/CacheTests.cpp | 89 +++++++++++++++++++++++++++++++--- 2 files changed, 101 insertions(+), 11 deletions(-) diff --git a/src/util/RandomEvictionCache.h b/src/util/RandomEvictionCache.h index 3122c63542..f1aafc83bb 100644 --- a/src/util/RandomEvictionCache.h +++ b/src/util/RandomEvictionCache.h @@ -181,9 +181,11 @@ class RandomEvictionCache : public NonMovableOrCopyable } } - // `get` offers strong exception safety guarantee. - V& - get(K const& k) + // `maybeGet` offers basic exception safety guarantee. + // Returns a pointer to the value if the key exists, + // and returns a nullptr otherwise. + V* + maybeGet(K const& k) { auto it = mValueMap.find(k); if (it != mValueMap.end()) @@ -191,12 +193,25 @@ class RandomEvictionCache : public NonMovableOrCopyable auto& cacheVal = it->second; ++mCounters.mHits; cacheVal.mLastAccess = ++mGeneration; - return cacheVal.mValue; + return &cacheVal.mValue; } else + { + ++mCounters.mMisses; + return nullptr; + } + } + + // `get` offers basic exception safety guarantee. + V& + get(K const& k) + { + V* result = maybeGet(k); + if (result == nullptr) { throw std::range_error("There is no such key in cache"); } + return *result; } }; } diff --git a/src/util/test/CacheTests.cpp b/src/util/test/CacheTests.cpp index 3d1cc8519a..9fa6d2e1e7 100644 --- a/src/util/test/CacheTests.cpp +++ b/src/util/test/CacheTests.cpp @@ -25,9 +25,14 @@ TEST_CASE("cachetable works as a cache", "[randomevictioncache]") auto p = cache.get(i); REQUIRE(p == i * 100); } + for (int i = 0; i < sz; ++i) + { + auto p = *cache.maybeGet(i); + REQUIRE(p == i * 100); + } REQUIRE(ctrs.mInserts == sz); REQUIRE(ctrs.mUpdates == 0); - REQUIRE(ctrs.mHits == sz); + REQUIRE(ctrs.mHits == 2 * sz); REQUIRE(ctrs.mMisses == 0); REQUIRE(ctrs.mEvicts == 0); @@ -41,9 +46,14 @@ TEST_CASE("cachetable works as a cache", "[randomevictioncache]") int p = cache.get(i); REQUIRE(p == i * 200); } + for (int i = 0; i < sz; ++i) + { + int p = *cache.maybeGet(i); + REQUIRE(p == i * 200); + } REQUIRE(ctrs.mInserts == sz); REQUIRE(ctrs.mUpdates == sz); - REQUIRE(ctrs.mHits == 2 * sz); + REQUIRE(ctrs.mHits == 4 * sz); REQUIRE(ctrs.mMisses == 0); REQUIRE(ctrs.mEvicts == 0); @@ -54,7 +64,7 @@ TEST_CASE("cachetable works as a cache", "[randomevictioncache]") } REQUIRE(ctrs.mInserts == sz + sz / 2); REQUIRE(ctrs.mUpdates == sz); - REQUIRE(ctrs.mHits == 2 * sz); + REQUIRE(ctrs.mHits == 4 * sz); REQUIRE(ctrs.mMisses == 0); REQUIRE(ctrs.mEvicts == sz / 2); @@ -68,9 +78,27 @@ TEST_CASE("cachetable works as a cache", "[randomevictioncache]") } REQUIRE(ctrs.mInserts == sz + sz / 2); REQUIRE(ctrs.mUpdates == sz); - REQUIRE(ctrs.mHits >= 2 * sz + sz / 2); + REQUIRE(ctrs.mHits >= 4 * sz + sz / 2); REQUIRE(ctrs.mMisses <= sz / 2); REQUIRE(ctrs.mEvicts == sz / 2); + + // Check that enough un-evicted entries are still there. + for (int i = 0; i < sz; ++i) + { + cache.maybeGet(i); + } + REQUIRE(ctrs.mInserts == sz + sz / 2); + REQUIRE(ctrs.mUpdates == sz); + REQUIRE(ctrs.mHits >= 5 * sz); + REQUIRE(ctrs.mMisses <= sz); + REQUIRE(ctrs.mEvicts == sz / 2); + + // Ensure that maybeGet returns nullptr if and only if + // cache.exists return false. + for (int i = 0; i < sz; i++) + { + REQUIRE(cache.exists(i) == (cache.maybeGet(i) != nullptr)); + } } TEST_CASE("cachetable does not thrash", "[randomevictioncachethrash][!hide]") @@ -91,13 +119,24 @@ TEST_CASE("cachetable does not thrash", "[randomevictioncachethrash][!hide]") // LRU this would be the bad case. for (int i = 0; i <= sz; ++i) { - if (cache.exists(i)) + if (i % 2 == 0) { - cache.get(i); + if (cache.exists(i)) + { + cache.get(i); + } + else + { + cache.put(i, i * 100); + } } else { - cache.put(i, i * 100); + auto p = cache.maybeGet(i); + if (p == nullptr) + { + cache.put(i, i * 100); + } } } // These are statistical: it's theoretically possible that a terrible stroke @@ -172,6 +211,30 @@ TEMPLATE_TEST_CASE("cache keeps last read items", "[cache][template]", REQUIRE(existing == 5); } +TEMPLATE_TEST_CASE("cache keeps last read items with maybeGet", + "[cache][template]", RandCache) +{ + TestType c{5}; + c.put(0, 0); + c.put(1, 1); + c.put(2, 2); + c.put(3, 3); + c.put(4, 4); + c.maybeGet(0); + c.put(5, 5); + + REQUIRE(c.size() == 5); + size_t existing = 0; + for (int i = 0; i <= 5; ++i) + { + if (c.exists(i)) + { + ++existing; + } + } + REQUIRE(existing == 5); +} + TEMPLATE_TEST_CASE("cache replace element", "[cache][template]", RandCache) { TestType c{5}; @@ -185,6 +248,18 @@ TEMPLATE_TEST_CASE("cache replace element", "[cache][template]", RandCache) REQUIRE(c.get(0) == 3); c.put(0, 4); REQUIRE(c.get(0) == 4); + + TestType d{5}; + d.put(0, 0); + REQUIRE(*d.maybeGet(0) == 0); + d.put(0, 1); + REQUIRE(*d.maybeGet(0) == 1); + d.put(0, 2); + REQUIRE(*d.maybeGet(0) == 2); + d.put(0, 3); + REQUIRE(*d.maybeGet(0) == 3); + d.put(0, 4); + REQUIRE(*d.maybeGet(0) == 4); } TEMPLATE_TEST_CASE("cache erase_if removes some nodes", "[cache][template]", From fbb4f2dfa09c7bc4e091e29b201c0b94e6e53045 Mon Sep 17 00:00:00 2001 From: Hidenori Shinohara Date: Wed, 11 Nov 2020 16:09:24 -0500 Subject: [PATCH 3/3] Cache the results of isAQuorum --- src/herder/QuorumIntersectionCheckerImpl.cpp | 13 ++++++++++++- src/herder/QuorumIntersectionCheckerImpl.h | 5 +++++ src/util/BitSet.h | 19 +++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/herder/QuorumIntersectionCheckerImpl.cpp b/src/herder/QuorumIntersectionCheckerImpl.cpp index 3b7391c801..d53db8a11f 100644 --- a/src/herder/QuorumIntersectionCheckerImpl.cpp +++ b/src/herder/QuorumIntersectionCheckerImpl.cpp @@ -346,6 +346,7 @@ QuorumIntersectionCheckerImpl::QuorumIntersectionCheckerImpl( , mQuiet(quiet) , mTSC(mGraph) , mInterruptFlag(interruptFlag) + , mCachedQuorums(MAX_CACHED_QUORUMS_SIZE) { buildGraph(qmap); buildSCCs(); @@ -464,7 +465,17 @@ QuorumIntersectionCheckerImpl::containsQuorumSliceForNode(BitSet const& bs, bool QuorumIntersectionCheckerImpl::isAQuorum(BitSet const& nodes) const { - return (bool)contractToMaximalQuorum(nodes); + bool* pRes = mCachedQuorums.maybeGet(nodes); + if (pRes == nullptr) + { + bool result = (bool)contractToMaximalQuorum(nodes); + mCachedQuorums.put(nodes, result); + return result; + } + else + { + return *pRes; + } } BitSet diff --git a/src/herder/QuorumIntersectionCheckerImpl.h b/src/herder/QuorumIntersectionCheckerImpl.h index 7f35f720cd..e22efc4319 100644 --- a/src/herder/QuorumIntersectionCheckerImpl.h +++ b/src/herder/QuorumIntersectionCheckerImpl.h @@ -353,6 +353,7 @@ #include "QuorumIntersectionChecker.h" #include "main/Config.h" #include "util/BitSet.h" +#include "util/RandomEvictionCache.h" #include "xdr/Stellar-SCP.h" #include "xdr/Stellar-types.h" @@ -530,6 +531,10 @@ class QuorumIntersectionCheckerImpl : public stellar::QuorumIntersectionChecker bool containsQuorumSlice(BitSet const& bs, QBitSet const& qbs) const; bool containsQuorumSliceForNode(BitSet const& bs, size_t node) const; BitSet contractToMaximalQuorum(BitSet nodes) const; + + const int MAX_CACHED_QUORUMS_SIZE = 0xffff; + mutable stellar::RandomEvictionCache + mCachedQuorums; bool isAQuorum(BitSet const& nodes) const; bool isMinimalQuorum(BitSet const& nodes) const; void noteFoundDisjointQuorums(BitSet const& nodes, diff --git a/src/util/BitSet.h b/src/util/BitSet.h index 0976a3dea0..41a0a67835 100644 --- a/src/util/BitSet.h +++ b/src/util/BitSet.h @@ -351,6 +351,25 @@ class BitSet { streamWith(out, [](std::ostream& out, size_t i) { out << i; }); } + class HashFunction + { + std::hash mHasher; + + public: + size_t + operator()(BitSet const& bitset) const noexcept + { + // Implementation taken from Boost. + // https://www.boost.org/doc/libs/1_35_0/doc/html/boost/hash_combine_id241013.html + size_t seed = 0; + for (size_t i = 0; i < bitset.mPtr->arraysize; i++) + { + seed ^= mHasher(bitset.mPtr->array[i]) + 0x9e3779b9 + + (seed << 6) + (seed >> 2); + } + return seed; + } + }; }; inline std::ostream&