[FOLD] Review feedback from @gregtatcam:

* Convert Features to use boost::multi_index_container.

src/ripple/protocol/impl/Feature.cpp

@@ -17,13 +17,31 @@
 */
 //==============================================================================
 
+#include <ripple/protocol/Feature.h>
+
 #include <ripple/basics/Slice.h>
 #include <ripple/basics/contract.h>
-#include <ripple/protocol/Feature.h>
 #include <ripple/protocol/digest.h>
-
+#include <boost/container_hash/hash.hpp>
+#include <boost/multi_index/hashed_index.hpp>
+#include <boost/multi_index/key_extractors.hpp>
+#include <boost/multi_index/random_access_index.hpp>
+#include <boost/multi_index_container.hpp>
 #include <cstring>
 
+namespace boost {
+
+inline std::size_t
+hash_value(ripple::uint256 const& feature)
+{
+ std::size_t seed = 0;
+ using namespace boost;
+ for (auto const& n : feature)
+ hash_combine(seed, n);
+ return seed;
+}
+
+} // namespace boost
 namespace ripple {
 
 enum class Supported : bool { no = false, yes };
@@ -52,29 +70,73 @@ namespace detail {
 
 class FeatureCollections
 {
+ struct ByName
+ {
+ };
+ struct ByFeature
+ {
+ };
  struct Feature
  {
  std::string name;
  uint256 feature;
- std::size_t index;
-
- explicit Feature() = default;
- explicit Feature(
- std::string const& name_,
- uint256 const& feature_,
- std::size_t index_)
- : name(name_), feature(feature_), index(index_)
+
+ Feature() = delete;
+ explicit Feature(std::string const& name_, uint256 const& feature_)
+ : name(name_), feature(feature_)
  {
  }
  };
- std::vector<Feature> features;
- boost::container::flat_map<uint256, std::size_t> featureToIndex;
- boost::container::flat_map<std::string, uint256> nameToFeature;
+
+ // Intermediate types to help with readability
+ template <class tag, typename Type, Type Feature::*PtrToMember>
+ using feature_hashed_unique = boost::multi_index::hashed_unique<
+ boost::multi_index::tag<tag>,
+ boost::multi_index::member<Feature, Type, PtrToMember>>;
+
+ // Intermediate types to help with readability
+ using feature_indexing = boost::multi_index::indexed_by<
+ boost::multi_index::random_access<>,
+ feature_hashed_unique<ByFeature, uint256, &Feature::feature>,
+ feature_hashed_unique<ByName, std::string, &Feature::name>>;
+
+ boost::multi_index::multi_index_container<Feature, feature_indexing>
+ features;
  std::map<std::string, DefaultVote> supported;
  std::size_t upVotes = 0;
  std::size_t downVotes = 0;
  mutable std::atomic<bool> readOnly = false;
 
+ Feature const&
+ getByIndex(size_t i) const
+ {
+ if (i >= features.size())
+ LogicError("Invalid FeatureBitset index");
+ const auto& sequence = features.get<0>();
+ return sequence[i];
+ }
+ size_t
+ getIndex(Feature const& feature) const
+ {
+ const auto& sequence = features.get<0>();
+ auto it_to = sequence.iterator_to(feature);
+ return it_to - sequence.begin();
+ }
+ Feature const*
+ getByFeature(uint256 const& feature) const
+ {
+ const auto& feature_index = features.get<1>();
+ auto feature_it = feature_index.find(feature);
+ return feature_it == feature_index.end() ? nullptr : &*feature_it;
+ }
+ Feature const*
+ getByName(std::string const& name) const
+ {
+ const auto& name_index = features.get<2>();
+ auto name_it = name_index.find(name);
+ return name_it == name_index.end() ? nullptr : &*name_it;
+ }
+
 public:
  FeatureCollections();
 
@@ -127,18 +189,16 @@ class FeatureCollections
 detail::FeatureCollections::FeatureCollections()
 {
  features.reserve(ripple::detail::numFeatures);
- featureToIndex.reserve(ripple::detail::numFeatures);
- nameToFeature.reserve(ripple::detail::numFeatures);
 }
 
 std::optional<uint256>
 detail::FeatureCollections::getRegisteredFeature(std::string const& name) const
 {
  readOnly = true;
- auto const i = nameToFeature.find(name);
- if (i == nameToFeature.end())
- return std::nullopt;
- return i->second;
+ Feature const* feature = getByName(name);
+ if (feature)
+ return feature->feature;
+ return std::nullopt;
 }
 
 uint256
@@ -149,10 +209,10 @@ detail::FeatureCollections::registerFeature(
 {
  assert(!readOnly);
  assert(support != Supported::no || vote != DefaultVote::yes);
- auto const i = nameToFeature.find(name);
+ Feature const* i = getByName(name);
  // Each feature should only be registered once
- assert(i == nameToFeature.end());
- if (i == nameToFeature.end())
+ assert(!i);
+ if (!i)
  {
  // If this assertion fails, and you just added a feature, increase the
  // numFeatures value in Feature.h
@@ -169,15 +229,7 @@ detail::FeatureCollections::registerFeature(
  }
 #endif
 
- auto const& feature = features.emplace_back(name, f, features.size());
- featureToIndex[f] = feature.index;
- nameToFeature[name] = f;
-
- assert(features.size() == featureToIndex.size());
- assert(features.size() == nameToFeature.size());
-
- assert(features[featureToIndex[f]].name == name);
- assert(features[featureToIndex[f]].feature == f);
+ auto const& feature = features.emplace_back(name, f);
 
  if (support == Supported::yes)
  {
@@ -193,34 +245,35 @@ detail::FeatureCollections::registerFeature(
  return f;
  }
  else
- return i->second;
+ return i->feature;
 }
 
 size_t
 detail::FeatureCollections::featureToBitsetIndex(uint256 const& f) const
 {
  readOnly = true;
- auto const i = featureToIndex.find(f);
- if (i == featureToIndex.end())
+
+ Feature const* feature = getByFeature(f);
+ if (!feature)
  LogicError("Invalid Feature ID");
- return i->second;
+
+ return getIndex(*feature);
 }
 
 uint256 const&
 detail::FeatureCollections::bitsetIndexToFeature(size_t i) const
 {
  readOnly = true;
- if (i >= features.size())
- LogicError("Invalid FeatureBitset index");
- return features[i].feature;
+ Feature const& feature = getByIndex(i);
+ return feature.feature;
 }
 
 std::string
 detail::FeatureCollections::featureToName(uint256 const& f) const
 {
  readOnly = true;
- auto const i = featureToIndex.find(f);
- return i == featureToIndex.end() ? to_string(f) : features[i->second].name;
+ Feature const* feature = getByFeature(f);
+ return feature ? feature->name : to_string(f);
 }
 
 static detail::FeatureCollections featureCollections;