Use Boost ICL for RangeSet (RIPD-1473)

Builds/VisualStudio2015/RippleD.vcxproj

@@ -1463,10 +1463,6 @@
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
     </ClCompile>
-    <ClCompile Include="..\..\src\ripple\basics\impl\RangeSet.cpp">
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
-      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
-    </ClCompile>
     <ClCompile Include="..\..\src\ripple\basics\impl\ResolverAsio.cpp">
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>

Builds/VisualStudio2015/RippleD.vcxproj.filters

@@ -2007,9 +2007,6 @@
     <ClCompile Include="..\..\src\ripple\basics\impl\mulDiv.cpp">
       <Filter>ripple\basics\impl</Filter>
     </ClCompile>
-    <ClCompile Include="..\..\src\ripple\basics\impl\RangeSet.cpp">
-      <Filter>ripple\basics\impl</Filter>
-    </ClCompile>
     <ClCompile Include="..\..\src\ripple\basics\impl\ResolverAsio.cpp">
       <Filter>ripple\basics\impl</Filter>
     </ClCompile>

src/ripple/app/ledger/LedgerMaster.h

@@ -305,7 +305,7 @@ private:
     std::unique_ptr<LedgerReplay> replayData;
 
     std::recursive_mutex mCompleteLock;
-    RangeSet mCompleteLedgers;
+    RangeSet<std::uint32_t> mCompleteLedgers;
 
     std::unique_ptr <detail::LedgerCleaner> mLedgerCleaner;
 

src/ripple/app/ledger/impl/LedgerMaster.cpp

@@ -344,14 +344,14 @@ bool
 LedgerMaster::haveLedger (std::uint32_t seq)
 {
     ScopedLockType sl (mCompleteLock);
-    return mCompleteLedgers.hasValue (seq);
+    return boost::icl::contains(mCompleteLedgers, seq);
 }
 
 void
 LedgerMaster::clearLedger (std::uint32_t seq)
 {
     ScopedLockType sl (mCompleteLock);
-    return mCompleteLedgers.clearValue (seq);
+    mCompleteLedgers.erase (seq);
 }
 
 // returns Ledgers we have all the nodes for
@@ -365,15 +365,16 @@ LedgerMaster::getFullValidatedRange (std::uint32_t& minVal, std::uint32_t& maxVa
     if (!maxVal)
         return false;
 
+    boost::optional<std::uint32_t> maybeMin;
     {
         ScopedLockType sl (mCompleteLock);
-        minVal = mCompleteLedgers.prevMissing (maxVal);
+        maybeMin = prevMissing(mCompleteLedgers, maxVal);
     }
 
-    if (minVal == RangeSet::absent)
+    if (maybeMin == boost::none)
         minVal = maxVal;
     else
-        ++minVal;
+        minVal = 1 + *maybeMin;
 
     return true;
 }
@@ -382,23 +383,9 @@ LedgerMaster::getFullValidatedRange (std::uint32_t& minVal, std::uint32_t& maxVa
 bool
 LedgerMaster::getValidatedRange (std::uint32_t& minVal, std::uint32_t& maxVal)
 {
-    // Validated ledger is likely not stored in the DB yet so we use the
+    if (!getFullValidatedRange(minVal, maxVal))
-    // published ledger which is.
-    maxVal = mPubLedgerSeq.load();
-
-    if (!maxVal)
         return false;
 
-    {
-        ScopedLockType sl (mCompleteLock);
-        minVal = mCompleteLedgers.prevMissing (maxVal);
-    }
-
-    if (minVal == RangeSet::absent)
-        minVal = maxVal;
-    else
-        ++minVal;
-
     // Remove from the validated range any ledger sequences that may not be
     // fully updated in the database yet
 
@@ -481,8 +468,8 @@ LedgerMaster::tryFill (
                 return;
 
             {
-                ScopedLockType ml (mCompleteLock);
+                ScopedLockType ml(mCompleteLock);
-                mCompleteLedgers.setRange (minHas, maxHas);
+                mCompleteLedgers.insert(range(minHas, maxHas));
             }
             maxHas = minHas;
             ledgerHashes = getHashesByIndex ((seq < 500)
@@ -502,7 +489,7 @@ LedgerMaster::tryFill (
 
     {
         ScopedLockType ml (mCompleteLock);
-        mCompleteLedgers.setRange (minHas, maxHas);
+        mCompleteLedgers.insert(range(minHas, maxHas));
     }
     {
         ScopedLockType ml (m_mutex);
@@ -647,7 +634,7 @@ LedgerMaster::setFullLedger (
 
     {
         ScopedLockType ml (mCompleteLock);
-        mCompleteLedgers.setValue (ledger->info().seq);
+        mCompleteLedgers.insert (ledger->info().seq);
     }
 
     {
@@ -1282,7 +1269,7 @@ std::string
 LedgerMaster::getCompleteLedgers ()
 {
     ScopedLockType sl (mCompleteLock);
-    return mCompleteLedgers.toString ();
+    return to_string(mCompleteLedgers);
 }
 
 boost::optional <NetClock::time_point>
@@ -1446,7 +1433,7 @@ void
 LedgerMaster::setLedgerRangePresent (std::uint32_t minV, std::uint32_t maxV)
 {
     ScopedLockType sl (mCompleteLock);
-    mCompleteLedgers.setRange (minV, maxV);
+    mCompleteLedgers.insert(range(minV, maxV));
 }
 
 void
@@ -1477,12 +1464,9 @@ LedgerMaster::getPropertySource ()
 void
 LedgerMaster::clearPriorLedgers (LedgerIndex seq)
 {
-    ScopedLockType sl (mCompleteLock);
+    ScopedLockType sl(mCompleteLock);
-    for (LedgerIndex i = mCompleteLedgers.getFirst(); i < seq; ++i)
+    if (seq > 0)
-    {
+        mCompleteLedgers.erase(range(0u, seq - 1));
-        if (haveLedger (i))
-            clearLedger (i);
-    }
 }
 
 void
@@ -1546,137 +1530,141 @@ void LedgerMaster::doAdvance ()
                 (mValidLedgerSeq == mPubLedgerSeq) &&
                 (getValidatedLedgerAge() < MAX_LEDGER_AGE_ACQUIRE))
             { // We are in sync, so can acquire
-                std::uint32_t missing;
+                boost::optional<std::uint32_t> maybeMissing;
                 {
                     ScopedLockType sl (mCompleteLock);
-                    missing = mCompleteLedgers.prevMissing(
+                    maybeMissing =
-                        mPubLedger->info().seq);
+                        prevMissing(mCompleteLedgers, mPubLedger->info().seq);
                 }
-                JLOG (m_journal.trace())
+                if (maybeMissing)
-                    << "tryAdvance discovered missing " << missing;
-                if ((missing != RangeSet::absent) && (missing > 0) &&
-                    shouldAcquire (mValidLedgerSeq, ledger_history_,
-                        app_.getSHAMapStore ().getCanDelete (), missing) &&
-                    ((mFillInProgress == 0) || (missing > mFillInProgress)))
                 {
-                    JLOG (m_journal.trace())
+                    std::uint32_t missing = *maybeMissing;
-                        << "advanceThread should acquire";
+                    JLOG(m_journal.trace())
+                        << "tryAdvance discovered missing " << missing;
+                    if ((missing > 0) &&
+                        shouldAcquire(mValidLedgerSeq, ledger_history_,
+                            app_.getSHAMapStore().getCanDelete(), missing) &&
+                            ((mFillInProgress == 0) || (missing > mFillInProgress)))
                     {
-                        ScopedUnlockType sl(m_mutex);
+                        JLOG(m_journal.trace())
-                        auto hash = getLedgerHashForHistory (missing);
+                            << "advanceThread should acquire";
-                        if (hash)
                         {
-                            assert(hash->isNonZero());
+                            ScopedUnlockType sl(m_mutex);
-                            auto ledger = getLedgerByHash (*hash);
+                            auto hash = getLedgerHashForHistory(missing);
-                            if (!ledger)
+                            if (hash)
                             {
-                                if (!app_.getInboundLedgers().isFailure (
+                                assert(hash->isNonZero());
-                                        *hash))
+                                auto ledger = getLedgerByHash(*hash);
+                                if (!ledger)
                                 {
-                                    ledger =
+                                    if (!app_.getInboundLedgers().isFailure(
-                                        app_.getInboundLedgers().acquire(
+                                        *hash))
-                                            *hash, missing,
-                                            InboundLedger::fcHISTORY);
-                                    if (! ledger && (missing > 32600) &&
-                                        shouldFetchPack (missing))
                                     {
-                                        JLOG (m_journal.trace()) <<
+                                        ledger =
-                                        "tryAdvance want fetch pack " <<
+                                            app_.getInboundLedgers().acquire(
-                                        missing;
+                                                *hash, missing,
-                                        fetch_seq_ = missing;
+                                                InboundLedger::fcHISTORY);
-                                        getFetchPack(*hash, missing);
+                                        if (!ledger && (missing > 32600) &&
+                                            shouldFetchPack(missing))
+                                        {
+                                            JLOG(m_journal.trace()) <<
+                                                "tryAdvance want fetch pack " <<
+                                                missing;
+                                            fetch_seq_ = missing;
+                                            getFetchPack(*hash, missing);
+                                        }
+                                        else
+                                            JLOG(m_journal.trace()) <<
+                                            "tryAdvance no fetch pack for " <<
+                                            missing;
                                     }
                                     else
-                                        JLOG (m_journal.trace()) <<
+                                        JLOG(m_journal.debug()) <<
-                                        "tryAdvance no fetch pack for " <<
+                                        "tryAdvance found failed acquire";
-                                        missing;
                                 }
-                                else
+                                if (ledger)
-                                    JLOG (m_journal.debug()) <<
+                                {
-                                    "tryAdvance found failed acquire";
+                                    auto seq = ledger->info().seq;
-                            }
+                                    assert(seq == missing);
-                            if (ledger)
+                                    JLOG(m_journal.trace())
-                            {
-                                auto seq = ledger->info().seq;
-                                assert(seq == missing);
-                                JLOG (m_journal.trace())
                                         << "tryAdvance acquired "
                                         << ledger->info().seq;
-                                setFullLedger(
+                                    setFullLedger(
-                                    ledger,
+                                        ledger,
-                                    false,
+                                        false,
-                                    false);
+                                        false);
-                                auto const& parent = ledger->info().parentHash;
+                                    auto const& parent = ledger->info().parentHash;
 
-                                int fillInProgress;
+                                    int fillInProgress;
-                                {
-                                    ScopedLockType lock(m_mutex);
-                                    mHistLedger = ledger;
-                                    fillInProgress = mFillInProgress;
-                                }
-
-                                if (fillInProgress == 0 &&
-                                    getHashByIndex(seq - 1, app_) == parent)
-                                {
                                     {
-                                        // Previous ledger is in DB
                                         ScopedLockType lock(m_mutex);
-                                        mFillInProgress = ledger->info().seq;
+                                        mHistLedger = ledger;
+                                        fillInProgress = mFillInProgress;
                                     }
 
-                                    app_.getJobQueue().addJob(
+                                    if (fillInProgress == 0 &&
-                                        jtADVANCE, "tryFill",
+                                        getHashByIndex(seq - 1, app_) == parent)
-                                        [this, ledger] (Job& j) {
+                                    {
+                                        {
+                                            // Previous ledger is in DB
+                                            ScopedLockType lock(m_mutex);
+                                            mFillInProgress = ledger->info().seq;
+                                        }
+
+                                        app_.getJobQueue().addJob(
+                                            jtADVANCE, "tryFill",
+                                            [this, ledger](Job& j) {
                                             tryFill(j, ledger);
                                         });
-                                }
+                                    }
 
-                                progress = true;
+                                    progress = true;
+                                }
+                                else
+                                {
+                                    try
+                                    {
+                                        for (int i = 0; i < ledger_fetch_size_; ++i)
+                                        {
+                                            std::uint32_t seq = missing - i;
+                                            auto hash2 =
+                                                getLedgerHashForHistory(seq);
+                                            if (hash2)
+                                            {
+                                                assert(hash2->isNonZero());
+                                                app_.getInboundLedgers().acquire
+                                                (*hash2, seq,
+                                                    InboundLedger::fcHISTORY);
+                                            }
+                                        }
+                                    }
+                                    catch (std::exception const&)
+                                    {
+                                        JLOG(m_journal.warn()) <<
+                                            "Threw while prefetching";
+                                    }
+                                }
                             }
                             else
                             {
-                                try
+                                JLOG(m_journal.fatal()) <<
-                                {
+                                    "Can't find ledger following prevMissing " <<
-                                    for (int i = 0; i < ledger_fetch_size_; ++i)
+                                    missing;
-                                    {
+                                JLOG(m_journal.fatal()) << "Pub:" <<
-                                        std::uint32_t seq = missing - i;
+                                    mPubLedgerSeq << " Val:" << mValidLedgerSeq;
-                                        auto hash2 =
+                                JLOG(m_journal.fatal()) << "Ledgers: " <<
-                                                getLedgerHashForHistory(seq);
+                                    app_.getLedgerMaster().getCompleteLedgers();
-                                        if (hash2)
+                                clearLedger(missing + 1);
-                                        {
+                                progress = true;
-                                            assert(hash2->isNonZero());
-                                            app_.getInboundLedgers().acquire
-                                                (*hash2, seq,
-                                                    InboundLedger::fcHISTORY);
-                                        }
-                                    }
-                                }
-                                catch (std::exception const&)
-                                {
-                                    JLOG (m_journal.warn()) <<
-                                    "Threw while prefetching";
-                                }
                             }
                         }
-                        else
+                        if (mValidLedgerSeq != mPubLedgerSeq)
                         {
-                            JLOG (m_journal.fatal()) <<
+                            JLOG(m_journal.debug()) <<
-                                "Can't find ledger following prevMissing " <<
+                                "tryAdvance found last valid changed";
-                                missing;
-                            JLOG (m_journal.fatal()) << "Pub:" <<
-                                mPubLedgerSeq << " Val:" << mValidLedgerSeq;
-                            JLOG (m_journal.fatal()) << "Ledgers: " <<
-                                app_.getLedgerMaster().getCompleteLedgers();
-                            clearLedger (missing + 1);
                             progress = true;
                         }
                     }
-                    if (mValidLedgerSeq != mPubLedgerSeq)
-                    {
-                        JLOG (m_journal.debug()) <<
-                            "tryAdvance found last valid changed";
-                        progress = true;
-                    }
                 }
             }
             else

src/ripple/basics/RangeSet.h

@@ -20,72 +20,113 @@
 #ifndef RIPPLE_BASICS_RANGESET_H_INCLUDED
 #define RIPPLE_BASICS_RANGESET_H_INCLUDED
 
-#include <cstdint>
-#include <map>
 #include <string>
+#include <boost/optional.hpp>
+#include <boost/icl/closed_interval.hpp>
+#include <boost/icl/interval_set.hpp>
 
-namespace ripple {
+namespace ripple
-
-/** A sparse set of integers. */
-// VFALCO TODO Replace with juce::SparseSet
-class RangeSet
 {
-public:
-    static const std::uint32_t absent = static_cast <std::uint32_t> (-1);
 
-public:
+/** A closed interval over the domain T.
-    RangeSet () { }
 
-    bool hasValue (std::uint32_t) const;
+    For an instance ClosedInterval c, this represents the closed interval
+    (c.first(), c.last()).  A single element interval has c.first() == c.last().
 
-    std::uint32_t getFirst () const;
+    This is simply a type-alias for boost interval container library interval
-    std::uint32_t getNext (std::uint32_t) const;
+    set, so users should consult that documentation for available supporting
-    std::uint32_t getLast () const;
+    member and free functions.
-    std::uint32_t getPrev (std::uint32_t) const;
+*/
+template <class T>
+using ClosedInterval = boost::icl::closed_interval<T>;
 
-    // largest number not in the set that is less than the given number
+/** Create a closed range interval
-    std::uint32_t prevMissing (std::uint32_t) const;
 
-    // Add an item to the set
+    Helper function to create a closed range interval without having to qualify
-    void setValue (std::uint32_t);
+    the template argument.
+*/
+template <class T>
+ClosedInterval<T>
+range(T low, T high)
+{
+    return ClosedInterval<T>(low, high);
+}
 
-    // Add the closed interval to the set
+/** A set of closed intervals over the domain T.
-    void setRange (std::uint32_t, std::uint32_t);
 
-    void clearValue (std::uint32_t);
+    Represents a set of values of the domain T using the minimum number
+    of disjoint ClosedInterval<T>.  This is useful to represent ranges of
+    T where a few instances are missing, e.g. the set 1-5,8-9,11-14.
 
-    std::string toString () const;
+    This is simply a type-alias for boost interval container library interval
+    set, so users should consult that documentation for available supporting
+    member and free functions.
+*/
+template <class T>
+using RangeSet = boost::icl::interval_set<T, std::less, ClosedInterval<T>>;
 
-    /** Returns the sum of the Lebesgue measures of all sub-ranges. */
-    std::size_t
-    lebesgue_sum() const;
 
-    /** Check invariants of the data.
+/** Convert a ClosedInterval to a styled string
 
-        This is for diagnostics, and does nothing in release builds.
+    The styled string is
-    */
+        "c.first()-c.last()"  if c.first() != c.last()
-    void checkInternalConsistency () const noexcept;
+        "c.first()" if c.first() == c.last()
 
-private:
+    @param ci The closed interval to convert
-    void simplify ();
+    @return The style string
+*/
+template <class T>
+std::string to_string(ClosedInterval<T> const & ci)
+{
+    if (ci.first() == ci.last())
+        return std::to_string(ci.first());
+    return std::to_string(ci.first()) + "-" + std::to_string(ci.last());
+}
 
-private:
+/** Convert the given RangeSet to a styled string.
-    using Map = std::map <std::uint32_t, std::uint32_t>;
 
-    using const_iterator         = Map::const_iterator;
+    The styled string represention is the set of disjoint intervals joined by
-    using const_reverse_iterator = Map::const_reverse_iterator;
+    commas.  The string "empty" is returned if the set is empty.
-    using value_type             = Map::value_type;
-    using iterator               = Map::iterator;
 
-    static bool contains (value_type const& it, std::uint32_t v)
+    @param rs The rangeset to convert
+    @return The styled string
+*/
+template <class T>
+std::string to_string(RangeSet<T> const & rs)
+{
+    using ripple::to_string;
+
+    if (rs.empty())
+        return "empty";
+    std::string res = "";
+    for (auto const & interval : rs)
     {
-        return (it.first <= v) && (it.second >= v);
+        if (!res.empty())
+            res += ",";
+        res += to_string(interval);
     }
+    return res;
+}
 
-    // First is lowest value in range, last is highest value in range
+/** Find the largest value not in the set that is less than a given value.
-    Map mRanges;
-};
-
-} // ripple
 
+    @param rs The set of interest
+    @param t The value that must be larger than the result
+    @param minVal (Default is 0) The smallest allowed value
+    @return The largest v such that minV <= v < t and !contains(rs, v) or
+            boost::none if no such v exists.
+*/
+template <class T>
+boost::optional<T>
+prevMissing(RangeSet<T> const & rs, T t, T minVal = 0)
+{
+    if (rs.empty() || t == minVal)
+        return boost::none;
+    RangeSet<T> tgt{ ClosedInterval<T>{minVal, t - 1} };
+    tgt -= rs;
+    if (tgt.empty())
+        return boost::none;
+    return boost::icl::last(tgt);
+}
+ }  // namespace ripple
 #endif

src/ripple/basics/impl/RangeSet.cpp

@@ -1,270 +0,0 @@
-//------------------------------------------------------------------------------
-/*
-    This file is part of rippled: https://github.com/ripple/rippled
-    Copyright (c) 2012, 2013 Ripple Labs Inc.
-
-    Permission to use, copy, modify, and/or distribute this software for any
-    purpose  with  or without fee is hereby granted, provided that the above
-    copyright notice and this permission notice appear in all copies.
-
-    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
-    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
-    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
-    ANY  SPECIAL ,  DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
-    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
-    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
-    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
-*/
-//==============================================================================
-
-#include <BeastConfig.h>
-#include <ripple/basics/Log.h>
-#include <ripple/basics/RangeSet.h>
-#include <ripple/beast/core/LexicalCast.h>
-#include <boost/foreach.hpp>
-#include <cassert>
-#include <cstdint>
-
-namespace ripple {
-
-// VFALCO NOTE std::min and std::max not good enough?
-//        NOTE Why isn't this written as a template?
-//        TODO Replace this with std calls.
-//
-inline std::uint32_t min (std::uint32_t x, std::uint32_t y)
-{
-    return (x < y) ? x : y;
-}
-inline std::uint32_t max (std::uint32_t x, std::uint32_t y)
-{
-    return (x > y) ? x : y;
-}
-
-bool RangeSet::hasValue (std::uint32_t v) const
-{
-    for (auto const& it : mRanges)
-    {
-        if (contains (it, v))
-            return true;
-    }
-    return false;
-}
-
-std::uint32_t RangeSet::getFirst () const
-{
-    const_iterator it = mRanges.begin ();
-
-    if (it == mRanges.end ())
-        return absent;
-
-    return it->first;
-}
-
-std::uint32_t RangeSet::getNext (std::uint32_t v) const
-{
-    for (auto const& it : mRanges)
-    {
-        if (it.first > v)
-            return it.first;
-
-        if (contains (it, v + 1))
-            return v + 1;
-    }
-    return absent;
-}
-
-std::uint32_t RangeSet::getLast () const
-{
-    const_reverse_iterator it = mRanges.rbegin ();
-
-    if (it == mRanges.rend ())
-        return absent;
-
-    return it->second;
-}
-
-std::uint32_t RangeSet::getPrev (std::uint32_t v) const
-{
-    BOOST_REVERSE_FOREACH (const value_type & it, mRanges)
-    {
-        if (it.second < v)
-            return it.second;
-
-        if (contains (it, v + 1))
-            return v - 1;
-    }
-    return absent;
-}
-
-// Return the largest number not in the set that is less than the given number
-//
-std::uint32_t RangeSet::prevMissing (std::uint32_t v) const
-{
-    std::uint32_t result = absent;
-
-    if (v != 0)
-    {
-        checkInternalConsistency ();
-
-        // Handle the case where the loop reaches the terminating condition
-        //
-        result = v - 1;
-
-        for (const_reverse_iterator cur = mRanges.rbegin (); cur != mRanges.rend (); ++cur)
-        {
-            // See if v-1 is in the range
-            if (contains (*cur, result))
-            {
-                result = cur->first - 1;
-                break;
-            }
-        }
-    }
-
-    assert (result == absent || !hasValue (result));
-
-    return result;
-}
-
-void RangeSet::setValue (std::uint32_t v)
-{
-    if (!hasValue (v))
-    {
-        mRanges[v] = v;
-
-        simplify ();
-    }
-}
-
-void RangeSet::setRange (std::uint32_t minV, std::uint32_t maxV)
-{
-    while (hasValue (minV))
-    {
-        ++minV;
-
-        if (minV >= maxV)
-            return;
-    }
-
-    mRanges[minV] = maxV;
-
-    simplify ();
-}
-
-void RangeSet::clearValue (std::uint32_t v)
-{
-    for (iterator it = mRanges.begin (); it != mRanges.end (); ++it)
-    {
-        if (contains (*it, v))
-        {
-            if (it->first == v)
-            {
-                if (it->second == v)
-                {
-                    mRanges.erase (it);
-                }
-                else
-                {
-                    std::uint32_t oldEnd = it->second;
-                    mRanges.erase(it);
-                    mRanges[v + 1] = oldEnd;
-                }
-            }
-            else if (it->second == v)
-            {
-                -- (it->second);
-            }
-            else
-            {
-                std::uint32_t oldEnd = it->second;
-                it->second = v - 1;
-                mRanges[v + 1] = oldEnd;
-            }
-
-            checkInternalConsistency();
-            return;
-        }
-    }
-}
-
-std::string RangeSet::toString () const
-{
-    std::string ret;
-    for (auto const& it : mRanges)
-    {
-        if (!ret.empty ())
-            ret += ",";
-
-        if (it.first == it.second)
-            ret += beast::lexicalCastThrow <std::string> ((it.first));
-        else
-            ret += beast::lexicalCastThrow <std::string> (it.first) + "-"
-                   + beast::lexicalCastThrow <std::string> (it.second);
-    }
-
-    if (ret.empty ())
-        return "empty";
-
-    return ret;
-}
-
-void RangeSet::simplify ()
-{
-    iterator it = mRanges.begin ();
-
-    while (1)
-    {
-        iterator nit = it;
-
-        if (++nit == mRanges.end ())
-        {
-            checkInternalConsistency();
-            return;
-        }
-
-        if (it->second >= (nit->first - 1))
-        {
-            // ranges overlap
-            it->second = std::max(it->second, nit->second);
-            mRanges.erase (nit);
-        }
-        else
-        {
-            it = nit;
-        }
-    }
-}
-
-std::size_t
-RangeSet::lebesgue_sum() const
-{
-    std::size_t sum = mRanges.size();
-    for (auto const& e : mRanges)
-        sum += e.second - e.first;
-    return sum;
-}
-
-void RangeSet::checkInternalConsistency () const noexcept
-{
-#ifndef NDEBUG
-    if (mRanges.size () > 1)
-    {
-        const_iterator const last = std::prev (mRanges.end ());
-
-        for (const_iterator cur = mRanges.begin (); cur != last; ++cur)
-        {
-            const_iterator const next = std::next (cur);
-            assert (cur->first <= cur->second);
-            assert (next->first <= next->second);
-            assert (cur->second + 1 < next->first);
-        }
-    }
-    else if (mRanges.size () == 1)
-    {
-        const_iterator const iter = mRanges.begin ();
-        assert (iter->first <= iter->second);
-    }
-#endif
-}
-
-} // ripple

src/ripple/unity/basics.cpp

@@ -26,7 +26,6 @@
 #include <ripple/basics/impl/Log.cpp>
 #include <ripple/basics/impl/make_SSLContext.cpp>
 #include <ripple/basics/impl/mulDiv.cpp>
-#include <ripple/basics/impl/RangeSet.cpp>
 #include <ripple/basics/impl/ResolverAsio.cpp>
 #include <ripple/basics/impl/strHex.cpp>
 #include <ripple/basics/impl/StringUtilities.cpp>

src/test/basics/RangeSet_test.cpp

@@ -21,14 +21,15 @@
 #include <ripple/basics/RangeSet.h>
 #include <ripple/beast/unit_test.h>
 
-namespace ripple {
+namespace ripple
-
+{
 class RangeSet_test : public beast::unit_test::suite
 {
 public:
-    RangeSet createPredefinedSet ()
+    void
+    testPrevMissing()
     {
-        RangeSet set;
+        testcase("prevMissing");
 
         // Set will include:
         // [ 0, 5]
@@ -36,59 +37,52 @@ public:
         // [20,25]
         // etc...
 
-        for (int i = 0; i < 10; ++i)
+        RangeSet<std::uint32_t> set;
-            set.setRange (10 * i, 10 * i + 5);
+        for (std::uint32_t i = 0; i < 10; ++i)
+            set.insert(range(10 * i, 10 * i + 5));
 
-        return set;
+        for (std::uint32_t i = 1; i < 100; ++i)
-    }
-
-    void testMembership ()
-    {
-        testcase ("membership");
-
-        RangeSet r1, r2;
-
-        r1.setRange (1, 10);
-        r1.clearValue (5);
-        r1.setRange (11, 20);
-
-        r2.setRange (1, 4);
-        r2.setRange (6, 10);
-        r2.setRange (10, 20);
-
-        BEAST_EXPECT(!r1.hasValue (5));
-
-        BEAST_EXPECT(r2.hasValue (9));
-    }
-
-    void testPrevMissing ()
-    {
-        testcase ("prevMissing");
-
-        RangeSet const set = createPredefinedSet ();
-
-        for (int i = 0; i < 100; ++i)
         {
-            int const oneBelowRange = (10*(i/10))-1;
+            boost::optional<std::uint32_t> expected;
+            // no prev missing in domain for i <= 6
+            if (i > 6)
+            {
+                std::uint32_t const oneBelowRange = (10 * (i / 10)) - 1;
 
-            int const expectedPrevMissing =
+                expected = ((i % 10) > 6) ? (i - 1) : oneBelowRange;
-                ((i % 10) > 6) ? (i-1) : oneBelowRange;
+            }
-
+            BEAST_EXPECT(prevMissing(set, i) == expected);
-            BEAST_EXPECT(set.prevMissing (i) == expectedPrevMissing);
         }
     }
 
-    void run ()
+    void
+    testToString()
     {
-        testMembership ();
+        testcase("toString");
 
-        testPrevMissing ();
+        RangeSet<std::uint32_t> set;
+        BEAST_EXPECT(to_string(set) == "empty");
 
-        // TODO: Traverse functions must be tested
+        set.insert(1);
+        BEAST_EXPECT(to_string(set) == "1");
+
+        set.insert(range(4u, 6u));
+        BEAST_EXPECT(to_string(set) == "1,4-6");
+
+        set.insert(2);
+        BEAST_EXPECT(to_string(set) == "1-2,4-6");
+
+        set.erase(range(4u, 5u));
+        BEAST_EXPECT(to_string(set) == "1-2,6");
+    }
+    void
+    run()
+    {
+        testPrevMissing();
+        testToString();
     }
 };
 
-BEAST_DEFINE_TESTSUITE(RangeSet,ripple_basics,ripple);
+BEAST_DEFINE_TESTSUITE(RangeSet, ripple_basics, ripple);
-
-} // ripple
 
+}  // namespace ripple