From 183dcd08d94e2fd087fed40f00ef088458808e04 Mon Sep 17 00:00:00 2001 From: seelabs Date: Thu, 10 Dec 2020 08:53:18 -0500 Subject: [PATCH] Remove ReadViewFwdRange `end` iterator cache: ReadViewFwdRange was storing a cached `end_` iterator that was lazily created in an iterators `end()` function. When the cache is empty, and the range it iterated from multiple threads, this creates a race condition. This change has performance consequences for "old style" for loops. For example: ``` // don't do this for(auto i = tx_range.begin(); i != tx_range.end(); ++i) ``` Can call the now expensive `end()` function more often than needed. Range-based for loop (I.e. `for(auto const& t : tx_range)`) should be used instead. --- src/ripple/ledger/ReadView.h | 4 ++-- src/ripple/ledger/detail/ReadViewFwdRange.h | 1 - src/ripple/ledger/impl/ReadView.cpp | 12 ++++-------- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/ripple/ledger/ReadView.h b/src/ripple/ledger/ReadView.h index a2b6f6f99..74fac105a 100644 --- a/src/ripple/ledger/ReadView.h +++ b/src/ripple/ledger/ReadView.h @@ -204,7 +204,7 @@ public: explicit sles_type(ReadView const& view); iterator begin() const; - iterator const& + iterator end() const; iterator upper_bound(key_type const& key) const; @@ -217,7 +217,7 @@ public: empty() const; iterator begin() const; - iterator const& + iterator end() const; }; diff --git a/src/ripple/ledger/detail/ReadViewFwdRange.h b/src/ripple/ledger/detail/ReadViewFwdRange.h index c37eab214..5b743678f 100644 --- a/src/ripple/ledger/detail/ReadViewFwdRange.h +++ b/src/ripple/ledger/detail/ReadViewFwdRange.h @@ -147,7 +147,6 @@ public: protected: ReadView const* view_; - boost::optional mutable end_; }; } // namespace detail diff --git a/src/ripple/ledger/impl/ReadView.cpp b/src/ripple/ledger/impl/ReadView.cpp index 69349e1df..96192e792 100644 --- a/src/ripple/ledger/impl/ReadView.cpp +++ b/src/ripple/ledger/impl/ReadView.cpp @@ -135,11 +135,9 @@ ReadView::sles_type::begin() const -> iterator } auto -ReadView::sles_type::end() const -> iterator const& +ReadView::sles_type::end() const -> iterator { - if (!end_) - end_ = iterator(view_, view_->slesEnd()); - return *end_; + return iterator(view_, view_->slesEnd()); } auto @@ -165,11 +163,9 @@ ReadView::txs_type::begin() const -> iterator } auto -ReadView::txs_type::end() const -> iterator const& +ReadView::txs_type::end() const -> iterator { - if (!end_) - end_ = iterator(view_, view_->txsEnd()); - return *end_; + return iterator(view_, view_->txsEnd()); } } // namespace ripple