Correct performance regression in StrategySupportProfile (#742)

The reimplementation of StrategySupportProfile which was focused on improving contingency iteration had substantial negative performance impact on code which relied on strategy iteration directly on the support.

This restores the implementation of the support profile as a sub-index of the full support and reverses the negative performance impact.  The internal changes which support improved contingency iteration have been retained.

Author: tturocy

src/games/gametable.cc

@@ -115,17 +115,17 @@ template <class T> class TableMixedStrategyProfileRep : public MixedStrategyProf
                       T &value) const;
   //@}
 
+  const CartesianProductSpace &m_pureStrategies;
+
   long StrategyOffset(const GameStrategy &s) const
   {
-    const auto &space = this->GetSupport().GetGame()->m_pureStrategies;
-    const auto &player = s->GetPlayer();
-    const size_t i = player->GetNumber() - 1;
-    return (s->GetNumber() - 1) * space.m_strides[i];
+    return (s->GetNumber() - 1) * m_pureStrategies.m_strides[s->m_player->GetNumber() - 1];
   }
 
 public:
   explicit TableMixedStrategyProfileRep(const StrategySupportProfile &p_support)
-    : MixedStrategyProfileRep<T>(p_support)
+    : MixedStrategyProfileRep<T>(p_support),
+      m_pureStrategies(p_support.GetGame()->m_pureStrategies)
   {
   }
   ~TableMixedStrategyProfileRep() override = default;

src/games/stratspt.cc

@@ -35,20 +35,26 @@ namespace Gambit {
 StrategySupportProfile::StrategySupportProfile(const Game &p_game) : m_game(p_game)
 {
   m_game->BuildComputedValues();
-  m_strategies.m_space = &p_game->m_pureStrategies;
-  m_strategies.m_allowedDigits.resize(p_game->NumPlayers());
+  for (const auto &player : m_game->GetPlayers()) {
+    for (const auto &strategy : player->GetStrategies()) {
+      m_support[player].push_back(strategy);
+    }
+  }
+
+  m_strategyDigits.m_space = &p_game->m_pureStrategies;
+  m_strategyDigits.m_allowedDigits.resize(p_game->NumPlayers());
 
   for (size_t i = 0; i < p_game->NumPlayers(); ++i) {
     const int radix = p_game->m_pureStrategies.m_radices[i];
-    auto &digits = m_strategies.m_allowedDigits[i];
+    auto &digits = m_strategyDigits.m_allowedDigits[i];
     digits.resize(radix);
     std::iota(digits.begin(), digits.end(), 0);
   }
 }
 
 int StrategySupportProfile::MixedProfileLength() const
 {
-  return sum_function(m_strategies.m_allowedDigits, [](const std::vector<int> &digits) {
+  return sum_function(m_strategyDigits.m_allowedDigits, [](const std::vector<int> &digits) {
     return static_cast<int>(digits.size());
   });
 }
@@ -68,8 +74,8 @@ bool StrategySupportProfile::IsSubsetOf(const StrategySupportProfile &p_other) c
   if (m_game != p_other.m_game) {
     return false;
   }
-  const auto &A = m_strategies.m_allowedDigits;
-  const auto &B = p_other.m_strategies.m_allowedDigits;
+  const auto &A = m_strategyDigits.m_allowedDigits;
+  const auto &B = p_other.m_strategyDigits.m_allowedDigits;
   const size_t n = A.size();
 
   for (size_t i = 0; i < n; ++i) {
@@ -112,12 +118,22 @@ void StrategySupportProfile::AddStrategy(const GameStrategy &p_strategy)
   if (p_strategy->GetGame() != m_game) {
     throw MismatchException();
   }
+
+  auto &support = m_support[p_strategy->GetPlayer()];
+  auto pos = std::find_if(support.begin(), support.end(), [p_strategy](const GameStrategy &s) {
+    return s->GetNumber() >= p_strategy->GetNumber();
+  });
+  if (pos == support.end() || *pos != p_strategy) {
+    // Strategy is not in the support for the player; add at this location to keep sorted by number
+    support.insert(pos, p_strategy);
+  }
+
   const size_t index = p_strategy->GetPlayer()->GetNumber() - 1;
   const int digit = p_strategy->GetNumber() - 1;
-  auto &digits = m_strategies.m_allowedDigits[index];
-  auto pos = std::lower_bound(digits.begin(), digits.end(), digit);
-  if (pos == digits.end() || *pos != digit) {
-    digits.insert(pos, digit);
+  auto &digits = m_strategyDigits.m_allowedDigits[index];
+  auto ipos = std::lower_bound(digits.begin(), digits.end(), digit);
+  if (ipos == digits.end() || *ipos != digit) {
+    digits.insert(ipos, digit);
   }
 }
 
@@ -126,15 +142,21 @@ bool StrategySupportProfile::RemoveStrategy(const GameStrategy &p_strategy)
   if (p_strategy->GetGame() != m_game) {
     throw MismatchException();
   }
-  const size_t index = p_strategy->GetPlayer()->GetNumber() - 1;
-  const int digit = p_strategy->GetNumber() - 1;
-  auto &digits = m_strategies.m_allowedDigits[index];
-  if (digits.size() == 1) {
+  auto &support = m_support[p_strategy->GetPlayer()];
+  if (support.size() == 1) {
     return false;
   }
-  auto pos = std::lower_bound(digits.begin(), digits.end(), digit);
-  if (pos != digits.end() && *pos == digit) {
-    digits.erase(pos);
+  auto pos = std::find(support.begin(), support.end(), p_strategy);
+  if (pos != support.end()) {
+    support.erase(pos);
+  }
+
+  const size_t index = p_strategy->GetPlayer()->GetNumber() - 1;
+  const int digit = p_strategy->GetNumber() - 1;
+  auto &digits = m_strategyDigits.m_allowedDigits[index];
+  auto ipos = std::lower_bound(digits.begin(), digits.end(), digit);
+  if (ipos != digits.end() && *ipos == digit) {
+    digits.erase(ipos);
     return true;
   }
   return false;

src/games/stratspt.h

@@ -40,88 +40,32 @@ namespace Gambit {
 /// in which they appear in the underlying game.
 class StrategySupportProfile {
   Game m_game;
-  CartesianSubset m_strategies;
+  std::map<GamePlayer, std::vector<GameStrategy>> m_support;
+  CartesianSubset m_strategyDigits;
 
 public:
   class Support {
     const StrategySupportProfile *m_profile;
-    size_t m_playerIndex;
+    GamePlayer m_player;
 
   public:
-    class const_iterator {
-      const StrategySupportProfile *m_profile{nullptr};
-      size_t m_playerIndex{0};
-      std::vector<int>::const_iterator m_it;
-
-    public:
-      using value_type = GameStrategy;
-      using reference = GameStrategy;
-      using pointer = void;
-      using difference_type = std::ptrdiff_t;
-      using iterator_category = std::forward_iterator_tag;
-
-      const_iterator() = default;
-      const_iterator(const StrategySupportProfile *profile, const size_t playerIndex,
-                     const std::vector<int>::const_iterator it)
-        : m_profile(profile), m_playerIndex(playerIndex), m_it(it)
-      {
-      }
-
-      GameStrategy operator*() const
-      {
-        const auto &player = m_profile->m_game->GetPlayer(m_playerIndex + 1);
-        return player->GetStrategy(*m_it + 1);
-      }
-
-      const_iterator &operator++()
-      {
-        ++m_it;
-        return *this;
-      }
-
-      bool operator==(const const_iterator &other) const { return m_it == other.m_it; }
-
-      bool operator!=(const const_iterator &other) const { return !(*this == other); }
-    };
-
-    Support() : m_profile(nullptr), m_playerIndex(0) {}
-
-    Support(const StrategySupportProfile *profile, GamePlayer player)
-      : m_profile(profile), m_playerIndex(player->GetNumber() - 1)
-    {
-    }
-
-    size_t size() const { return m_profile->m_strategies.m_allowedDigits[m_playerIndex].size(); }
-
-    GameStrategy operator[](const size_t index) const
-    {
-      const int digit = m_profile->m_strategies.m_allowedDigits[m_playerIndex][index];
-      return m_profile->m_game->GetPlayer(m_playerIndex + 1)->GetStrategy(digit + 1);
-    }
+    using const_iterator = std::vector<GameStrategy>::const_iterator;
 
-    GameStrategy front() const
+    Support() : m_profile(nullptr), m_player(nullptr) {}
+    Support(const StrategySupportProfile *profile, const GamePlayer &player)
+      : m_profile(profile), m_player(player)
     {
-      const int digit = m_profile->m_strategies.m_allowedDigits[m_playerIndex].front();
-      return m_profile->m_game->GetPlayer(m_playerIndex + 1)->GetStrategy(digit + 1);
     }
 
-    GameStrategy back() const
-    {
-      const int digit = m_profile->m_strategies.m_allowedDigits[m_playerIndex].back();
-      return m_profile->m_game->GetPlayer(m_playerIndex + 1)->GetStrategy(digit + 1);
-    }
-
-    const_iterator begin() const
-    {
-      const auto &digits = m_profile->m_strategies.m_allowedDigits[m_playerIndex];
-      return {m_profile, m_playerIndex, digits.begin()};
-    }
-
-    const_iterator end() const
+    size_t size() const { return m_profile->m_support.at(m_player).size(); }
+    GameStrategy operator[](const size_t index) const
     {
-      const auto &digits = m_profile->m_strategies.m_allowedDigits[m_playerIndex];
-      return {m_profile, m_playerIndex, digits.end()};
+      return m_profile->m_support.at(m_player)[index];
     }
+    GameStrategy front() const { return m_profile->m_support.at(m_player).front(); }
+    GameStrategy back() const { return m_profile->m_support.at(m_player).back(); }
+    const_iterator begin() const { return m_profile->m_support.at(m_player).begin(); }
+    const_iterator end() const { return m_profile->m_support.at(m_player).end(); }
   };
 
   /// @name Lifecycle
@@ -136,13 +80,13 @@ class StrategySupportProfile {
   bool operator==(const StrategySupportProfile &p_support) const
   {
     return m_game == p_support.m_game &&
-           m_strategies.m_allowedDigits == p_support.m_strategies.m_allowedDigits;
+           m_strategyDigits.m_allowedDigits == p_support.m_strategyDigits.m_allowedDigits;
   }
   /// Test for the inequality of two supports
   bool operator!=(const StrategySupportProfile &p_support) const
   {
     return m_game != p_support.m_game ||
-           m_strategies.m_allowedDigits != p_support.m_strategies.m_allowedDigits;
+           m_strategyDigits.m_allowedDigits != p_support.m_strategyDigits.m_allowedDigits;
   }
   //@}
 
@@ -166,7 +110,7 @@ class StrategySupportProfile {
   /// Returns true exactly when the strategy is in the support.
   bool Contains(const GameStrategy &s) const
   {
-    const auto &digits = m_strategies.m_allowedDigits[s->GetPlayer()->GetNumber() - 1];
+    const auto &digits = m_strategyDigits.m_allowedDigits[s->GetPlayer()->GetNumber() - 1];
     const int digit = s->GetNumber() - 1;
     return std::binary_search(digits.begin(), digits.end(), digit);
   }
@@ -203,7 +147,7 @@ class StrategySupportProfile {
     const size_t player_index = player->GetNumber() - 1;
     const int digit = p_strategy->GetNumber() - 1;
     StrategySupportProfile restricted(*this);
-    restricted.m_strategies.m_allowedDigits[player_index].assign(1, digit);
+    restricted.m_strategyDigits.m_allowedDigits[player_index].assign(1, digit);
     return restricted;
   }
   //@}