From 0f705c8a82d552114776a354cc0fa9529d5e1444 Mon Sep 17 00:00:00 2001
From: chriseth <chris@ethereum.org>
Date: Thu, 3 Feb 2022 17:58:55 +0100
Subject: [PATCH] Cache solution for the case where we are not interested in
 models.

---
 libsolutil/LP.cpp | 35 +++++++++++++++++------------------
 libsolutil/LP.h   | 22 ++++++++++++++++------
 2 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/libsolutil/LP.cpp b/libsolutil/LP.cpp
index c6d3915fe..02da1fb96 100644
--- a/libsolutil/LP.cpp
+++ b/libsolutil/LP.cpp
@@ -475,25 +475,17 @@ bool Constraint::operator==(Constraint const& _other) const
 	return true;
 }
 
-bool SolvingState::operator<(SolvingState const& _other) const
+bool SolvingState::Compare::operator()(SolvingState const& _a, SolvingState const& _b) const
 {
-	if (variableNames == _other.variableNames)
+	if (!considerVariableNames || _a.variableNames == _b.variableNames)
 	{
-		if (bounds == _other.bounds)
-			return constraints < _other.constraints;
+		if (_a.bounds == _b.bounds)
+			return _a.constraints < _b.constraints;
 		else
-			return bounds < _other.bounds;
+			return _a.bounds < _b.bounds;
 	}
 	else
-		return variableNames < _other.variableNames;
-}
-
-bool SolvingState::operator==(SolvingState const& _other) const
-{
-	return
-		variableNames == _other.variableNames &&
-		bounds == _other.bounds &&
-		constraints == _other.constraints;
+		return _a.variableNames < _b.variableNames;
 }
 
 namespace
@@ -734,6 +726,12 @@ SolvingState ProblemSplitter::next()
 	return splitOff;
 }
 
+LPSolver::LPSolver(bool _supportModels):
+	m_supportModels(_supportModels),
+	m_cache(SolvingState::Compare{_supportModels})
+{
+}
+
 pair<LPResult, map<string, rational>> LPSolver::check(SolvingState _state)
 {
 	normalizeRowLengths(_state);
@@ -760,9 +758,7 @@ pair<LPResult, map<string, rational>> LPSolver::check(SolvingState _state)
 
 		LPResult lpResult;
 		vector<rational> solution;
-		// TODO this actually compares including the variable names.
-		// If we only compare based on coefficients, we will get way more cache hits.
-		// The downside is that we need to adjust the model.
+
 		auto it = m_cache.find(split);
 		if (it != m_cache.end())
 			tie(lpResult, solution) = it->second;
@@ -778,7 +774,10 @@ pair<LPResult, map<string, rational>> LPSolver::check(SolvingState _state)
 				objectives.resize(split.constraints.front().data.size(), rational(bigint(1)));
 				tie(lpResult, solution) = simplex(split.constraints, move(objectives));
 			}
-			m_cache.emplace(move(orig), make_pair(lpResult, solution));
+			// If we do not support models, do not store it in the cache because
+			// the variable associations will be wrong.
+			// Otherwise, it is fine to use the model.
+			m_cache.emplace(move(orig), make_pair(lpResult, m_supportModels ? solution : vector<rational>{}));
 		}
 
 		switch (lpResult)
diff --git a/libsolutil/LP.h b/libsolutil/LP.h
index 9fb0ef943..9fd50aca1 100644
--- a/libsolutil/LP.h
+++ b/libsolutil/LP.h
@@ -62,8 +62,13 @@ struct SolvingState
 	std::vector<Bounds> bounds;
 	std::vector<Constraint> constraints;
 
-	bool operator<(SolvingState const& _other) const;
-	bool operator==(SolvingState const& _other) const;
+	struct Compare
+	{
+		explicit Compare(bool _considerVariableNames = true): considerVariableNames(_considerVariableNames) {}
+		bool operator()(SolvingState const& _a, SolvingState const& _b) const;
+		bool considerVariableNames;
+	};
+
 	std::string toString() const;
 };
 
@@ -153,17 +158,22 @@ private:
  *
  * Tries to split a given problem into sub-problems and utilizes a cache to quickly solve
  * similar problems.
+ *
+ * Can be used in a mode where it does not support returning models. In that case, the
+ * cache is more efficient.
  */
 class LPSolver
 {
 public:
+	explicit LPSolver(bool _supportModels = true);
+
 	std::pair<LPResult, std::map<std::string, boost::rational<bigint>>> check(SolvingState _state);
 
 private:
-	// TODO check if the model is requested in production. If not, we do not need to cache it.
-	// TODO This cache is inefficient because it compares including the variable names.
-	// See comment in LPSolver::check for details.
-	std::map<SolvingState, std::pair<LPResult, std::vector<boost::rational<bigint>>>> m_cache;
+	using CacheValue = std::pair<LPResult, std::vector<boost::rational<bigint>>>;
+
+	bool m_supportModels = true;
+	std::map<SolvingState, CacheValue, SolvingState::Compare> m_cache;
 };
 
 }