Improve the overall speed and memory use of libCellML

.github/workflows/deploy-on-release.yml

@@ -322,11 +322,11 @@ jobs:
             if [[ "${{ matrix.py }}" == "3.8" ]]; then
               echo "Setting macos_archs as: macos_archs='x86_64'"
               echo "macos_archs=x86_64" >> $GITHUB_OUTPUT
-              echo "macos_deployment_target=10.15" >> $GITHUB_OUTPUT
+              echo "macos_deployment_target=14" >> $GITHUB_OUTPUT
             else
               echo "Setting macos_archs as: macos_archs='x86_64 arm64'"
               echo 'macos_archs=x86_64 arm64' >> $GITHUB_OUTPUT
-              echo "macos_deployment_target=11.0" >> $GITHUB_OUTPUT
+              echo "macos_deployment_target=14" >> $GITHUB_OUTPUT
             fi
           else
             echo "Setting macos_archs as: macos_archs='x86_64'"

CMakeLists.txt

@@ -14,14 +14,19 @@
 
 cmake_minimum_required(VERSION 3.18.0)
 
-set(CMAKE_OSX_DEPLOYMENT_TARGET 10.15 CACHE STRING "Minimum OS X deployment version.")
+set(CMAKE_OSX_DEPLOYMENT_TARGET 14 CACHE STRING "Minimum OS X deployment version.")
 
 set(PROJECT_NAME libCellML)
 set(PROJECT_URL https://libcellml.org)
 set(_PROJECT_VERSION 0.6.3)
 set(PROJECT_DEVELOPER_VERSION)
 project(${PROJECT_NAME} VERSION ${_PROJECT_VERSION} LANGUAGES CXX)
 
+# Set the C++ standard to be used for all targets.
+set(CMAKE_CXX_STANDARD 20)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+set(CMAKE_CXX_EXTENSIONS OFF)
+
 # Set policies that affect the build.
 set(NEW_POLICIES CMP0056 CMP0063 CMP0074 CMP0078 CMP0086 CMP0092)
 foreach(NEW_POLICY ${NEW_POLICIES})

src/CMakeLists.txt

@@ -303,6 +303,10 @@ if(LIBCELLML_LLVM_COVERAGE)
   append_target_property(cellml LINK_FLAGS "-fprofile-instr-generate")
 endif()
 
+if(LIBCELLML_COVERAGE OR LIBCELLML_LLVM_COVERAGE)
+  target_compile_definitions(cellml PRIVATE CODE_COVERAGE_ENABLED)
+endif()
+
 install(TARGETS cellml EXPORT libcellml-targets
   COMPONENT runtime
   RUNTIME DESTINATION bin

src/analyser_p.h

@@ -17,6 +17,8 @@ limitations under the License.
 #include "libcellml/generatorprofile.h"
 #include "libcellml/issue.h"
 
+#include <unordered_set>
+
 #include "analysermodel_p.h"
 #include "internaltypes.h"
 #include "logger_p.h"
@@ -96,9 +98,12 @@ struct AnalyserInternalEquation
     ComponentPtr mComponent;
 
     AnalyserInternalVariablePtrs mVariables;
+    std::unordered_set<AnalyserInternalVariable *> mVariablesSet;
     AnalyserInternalVariablePtrs mStateVariables;
+    std::unordered_set<AnalyserInternalVariable *> mStateVariablesSet;
     AnalyserInternalVariablePtrs mAllVariables;
     AnalyserInternalVariablePtrs mUnknownVariables;
+    std::unordered_set<AnalyserInternalVariable *> mUnknownVariablesSet;
 
     size_t mNlaSystemIndex = MAX_SIZE_T;
     AnalyserInternalEquationWeakPtrs mNlaSiblings;
@@ -160,12 +165,13 @@ class Analyser::AnalyserImpl: public Logger::LoggerImpl
     AnalyserExternalVariablePtrs mExternalVariables;
 
     AnalyserInternalVariablePtrs mInternalVariables;
+    std::unordered_map<Variable *, AnalyserInternalVariablePtr> mInternalVariableCache;
     AnalyserInternalEquationPtrs mInternalEquations;
 
     GeneratorProfilePtr mGeneratorProfile = GeneratorProfile::create();
 
-    std::map<std::string, UnitsPtr> mStandardUnits;
-    std::map<AnalyserEquationAstPtr, UnitsPtr> mCiCnUnits;
+    std::unordered_map<std::string, UnitsPtr> mStandardUnits;
+    std::unordered_map<AnalyserEquationAstPtr, UnitsPtr> mCiCnUnits;
 
     AnalyserImpl();
 
@@ -181,10 +187,6 @@ class Analyser::AnalyserImpl: public Logger::LoggerImpl
     void analyseComponent(const ComponentPtr &component);
     void analyseComponentVariables(const ComponentPtr &component);
 
-    void equivalentVariables(const VariablePtr &variable,
-                             VariablePtrs &equivalentVariables) const;
-    VariablePtrs equivalentVariables(const VariablePtr &variable) const;
-
     void analyseEquationAst(const AnalyserEquationAstPtr &ast);
 
     void updateUnitsMapWithStandardUnit(const std::string &unitsName,
@@ -251,7 +253,7 @@ class Analyser::AnalyserImpl: public Logger::LoggerImpl
     static bool isExternalVariable(const AnalyserInternalVariablePtr &variable);
 
     bool isStateRateBased(const AnalyserEquationPtr &analyserEquation,
-                          AnalyserEquationPtrs &checkedEquations);
+                          std::unordered_set<AnalyserEquation *> &checkedEquations);
 
     void addInvalidVariableIssue(const AnalyserInternalVariablePtr &variable,
                                  Issue::ReferenceRule referenceRule);

src/analyserequation.cpp

@@ -93,6 +93,8 @@ std::vector<AnalyserEquationPtr> AnalyserEquation::dependencies() const
 {
     std::vector<AnalyserEquationPtr> res;
 
+    res.reserve(mPimpl->mDependencies.size());
+
     for (const auto &dependency : mPimpl->mDependencies) {
         res.push_back(dependency.lock());
     }
@@ -123,6 +125,8 @@ std::vector<AnalyserEquationPtr> AnalyserEquation::nlaSiblings() const
 {
     std::vector<AnalyserEquationPtr> res;
 
+    res.reserve(mPimpl->mNlaSiblings.size());
+
     for (const auto &nlaSibling : mPimpl->mNlaSiblings) {
         res.push_back(nlaSibling.lock());
     }
@@ -149,7 +153,7 @@ size_t AnalyserEquation::stateCount() const
     return mPimpl->mStates.size();
 }
 
-std::vector<AnalyserVariablePtr> AnalyserEquation::states() const
+const std::vector<AnalyserVariablePtr> &AnalyserEquation::states() const
 {
     return mPimpl->mStates;
 }
@@ -168,7 +172,7 @@ size_t AnalyserEquation::computedConstantCount() const
     return mPimpl->mComputedConstants.size();
 }
 
-std::vector<AnalyserVariablePtr> AnalyserEquation::computedConstants() const
+const std::vector<AnalyserVariablePtr> &AnalyserEquation::computedConstants() const
 {
     return mPimpl->mComputedConstants;
 }
@@ -187,7 +191,7 @@ size_t AnalyserEquation::algebraicVariableCount() const
     return mPimpl->mAlgebraicVariables.size();
 }
 
-std::vector<AnalyserVariablePtr> AnalyserEquation::algebraicVariables() const
+const std::vector<AnalyserVariablePtr> &AnalyserEquation::algebraicVariables() const
 {
     return mPimpl->mAlgebraicVariables;
 }
@@ -206,7 +210,7 @@ size_t AnalyserEquation::externalVariableCount() const
     return mPimpl->mExternalVariables.size();
 }
 
-std::vector<AnalyserVariablePtr> AnalyserEquation::externalVariables() const
+const std::vector<AnalyserVariablePtr> &AnalyserEquation::externalVariables() const
 {
     return mPimpl->mExternalVariables;
 }

src/analyserequationast.cpp

@@ -170,7 +170,7 @@ void AnalyserEquationAst::setType(Type type)
     mPimpl->mType = type;
 }
 
-std::string AnalyserEquationAst::value() const
+const std::string &AnalyserEquationAst::value() const
 {
     return mPimpl->mValue;
 }

src/analyserexternalvariable.cpp

@@ -31,7 +31,7 @@ std::vector<VariablePtr>::iterator AnalyserExternalVariable::AnalyserExternalVar
                                                                                                           const std::string &componentName,
                                                                                                           const std::string &variableName)
 {
-    return std::find_if(mDependencies.begin(), mDependencies.end(), [=](const auto &v) {
+    return std::find_if(mDependencies.begin(), mDependencies.end(), [&](const auto &v) {
         return (owningModel(v) == model)
                && (owningComponent(v)->name() == componentName)
                && (v->name() == variableName);
@@ -40,7 +40,7 @@ std::vector<VariablePtr>::iterator AnalyserExternalVariable::AnalyserExternalVar
 
 std::vector<VariablePtr>::iterator AnalyserExternalVariable::AnalyserExternalVariableImpl::findDependency(const VariablePtr &variable)
 {
-    return std::find_if(mDependencies.begin(), mDependencies.end(), [=](const auto &v) {
+    return std::find_if(mDependencies.begin(), mDependencies.end(), [&](const auto &v) {
         return v == variable;
     });
 }
@@ -155,7 +155,7 @@ VariablePtr AnalyserExternalVariable::dependency(const ModelPtr &model,
                nullptr;
 }
 
-std::vector<VariablePtr> AnalyserExternalVariable::dependencies() const
+const std::vector<VariablePtr> &AnalyserExternalVariable::dependencies() const
 {
     return mPimpl->mDependencies;
 }

src/analysermodel.cpp

@@ -23,6 +23,9 @@ limitations under the License.
 
 namespace libcellml {
 
+static const std::vector<AnalyserEquationPtr> NO_ANALYSER_EQUATION;
+static const std::vector<AnalyserVariablePtr> NO_ANALYSER_VARIABLE;
+
 AnalyserModelPtr AnalyserModel::AnalyserModelImpl::create(const ModelPtr &model)
 {
     return std::shared_ptr<AnalyserModel> {new AnalyserModel(model)};
@@ -104,10 +107,10 @@ size_t AnalyserModel::stateCount() const
     return mPimpl->mStates.size();
 }
 
-std::vector<AnalyserVariablePtr> AnalyserModel::states() const
+const std::vector<AnalyserVariablePtr> &AnalyserModel::states() const
 {
     if (!isValid()) {
-        return {};
+        return NO_ANALYSER_VARIABLE;
     }
 
     return mPimpl->mStates;
@@ -132,10 +135,10 @@ size_t AnalyserModel::constantCount() const
     return mPimpl->mConstants.size();
 }
 
-std::vector<AnalyserVariablePtr> AnalyserModel::constants() const
+const std::vector<AnalyserVariablePtr> &AnalyserModel::constants() const
 {
     if (!isValid()) {
-        return {};
+        return NO_ANALYSER_VARIABLE;
     }
 
     return mPimpl->mConstants;
@@ -159,10 +162,10 @@ size_t AnalyserModel::computedConstantCount() const
     return mPimpl->mComputedConstants.size();
 }
 
-std::vector<AnalyserVariablePtr> AnalyserModel::computedConstants() const
+const std::vector<AnalyserVariablePtr> &AnalyserModel::computedConstants() const
 {
     if (!isValid()) {
-        return {};
+        return NO_ANALYSER_VARIABLE;
     }
 
     return mPimpl->mComputedConstants;
@@ -186,10 +189,10 @@ size_t AnalyserModel::algebraicVariableCount() const
     return mPimpl->mAlgebraicVariables.size();
 }
 
-std::vector<AnalyserVariablePtr> AnalyserModel::algebraicVariables() const
+const std::vector<AnalyserVariablePtr> &AnalyserModel::algebraicVariables() const
 {
     if (!isValid()) {
-        return {};
+        return NO_ANALYSER_VARIABLE;
     }
 
     return mPimpl->mAlgebraicVariables;
@@ -213,10 +216,10 @@ size_t AnalyserModel::externalVariableCount() const
     return mPimpl->mExternalVariables.size();
 }
 
-std::vector<AnalyserVariablePtr> AnalyserModel::externalVariables() const
+const std::vector<AnalyserVariablePtr> &AnalyserModel::externalVariables() const
 {
     if (!isValid()) {
-        return {};
+        return NO_ANALYSER_VARIABLE;
     }
 
     return mPimpl->mExternalVariables;
@@ -237,8 +240,20 @@ AnalyserVariablePtr AnalyserModel::analyserVariable(const VariablePtr &variable)
         return {};
     }
 
+    // Check the cache first.
+
+    auto analyserVariableIt = mPimpl->mAnalyserVariables.find(variable.get());
+
+    if (analyserVariableIt != mPimpl->mAnalyserVariables.end()) {
+        return analyserVariableIt->second;
+    }
+
+    // Not in the cache, so do the equivalence-based search.
+
     for (const auto &analyserVariable : analyserVariables(shared_from_this())) {
         if (areEquivalentVariables(variable, analyserVariable->variable())) {
+            mPimpl->mAnalyserVariables.emplace(variable.get(), analyserVariable);
+
             return analyserVariable;
         }
     }
@@ -255,10 +270,10 @@ size_t AnalyserModel::analyserEquationCount() const
     return mPimpl->mAnalyserEquations.size();
 }
 
-std::vector<AnalyserEquationPtr> AnalyserModel::analyserEquations() const
+const std::vector<AnalyserEquationPtr> &AnalyserModel::analyserEquations() const
 {
     if (!isValid()) {
-        return {};
+        return NO_ANALYSER_EQUATION;
     }
 
     return mPimpl->mAnalyserEquations;
@@ -496,25 +511,20 @@ bool AnalyserModel::areEquivalentVariables(const VariablePtr &variable1,
     // an AnalyserModel object refers to a static version of a model, which
     // means that we can safely cache the result of a call to that utility. In
     // turn, this means that we can speed up any feature (e.g., code generation)
-    // that also relies on that utility. When it comes to the key for the cache,
-    // we use the Cantor pairing function with the address of the two variables
-    // as parameters, thus ensuring the uniqueness of the key (see
-    // https://en.wikipedia.org/wiki/Pairing_function#Cantor_pairing_function).
+    // that also relies on that utility.
 
     auto v1 = reinterpret_cast<uintptr_t>(variable1.get());
     auto v2 = reinterpret_cast<uintptr_t>(variable2.get());
 
-    if (v2 < v1) {
-        v1 += v2;
-        v2 = v1 - v2;
-        v1 = v1 - v2;
+    if (v1 > v2) {
+        std::swap(v1, v2);
     }
 
-    auto key = ((v1 + v2) * (v1 + v2 + 1) >> 1U) + v2;
-    auto cacheKey = mPimpl->mCachedEquivalentVariables.find(key);
+    auto key = AnalyserModel::AnalyserModelImpl::VariableKeyPair {v1, v2};
+    auto it = mPimpl->mCachedEquivalentVariables.find(key);
 
-    if (cacheKey != mPimpl->mCachedEquivalentVariables.end()) {
-        return cacheKey->second;
+    if (it != mPimpl->mCachedEquivalentVariables.end()) {
+        return it->second;
     }
 
     auto res = libcellml::areEquivalentVariables(variable1, variable2);

src/analysermodel_p.h

@@ -16,7 +16,8 @@ limitations under the License.
 
 #pragma once
 
-#include <map>
+#include <cstdint>
+#include <unordered_map>
 
 #include "libcellml/analysermodel.h"
 
@@ -45,6 +46,44 @@ struct AnalyserModel::AnalyserModelImpl
 
     std::vector<AnalyserEquationPtr> mAnalyserEquations;
 
+    struct VariableKeyPair
+    {
+        uintptr_t first;
+        uintptr_t second;
+
+        bool operator==(const VariableKeyPair &other) const
+        {
+#ifdef CODE_COVERAGE_ENABLED
+            // Use a branchless form so coverage does not depend on short-circuit behaviour.
+
+            const auto firstEqual = static_cast<uintptr_t>(first == other.first);
+            const auto secondEqual = static_cast<uintptr_t>(second == other.second);
+
+            return firstEqual * secondEqual != 0;
+#else
+            return first == other.first && second == other.second;
+#endif
+        }
+    };
+
+    mutable std::unordered_map<Variable *, AnalyserVariablePtr> mAnalyserVariables;
+
+    struct VariableKeyPairHash
+    {
+        size_t operator()(const VariableKeyPair &pair) const
+        {
+            // A simple and portable hash function for a pair of pointers.
+
+            size_t hash = pair.first;
+
+            hash ^= pair.second + 0x9e3779b9 + (hash << 6) + (hash >> 2);
+
+            return hash;
+        }
+    };
+
+    std::unordered_map<VariableKeyPair, bool, VariableKeyPairHash> mCachedEquivalentVariables;
+
     bool mNeedEqFunction = false;
     bool mNeedNeqFunction = false;
     bool mNeedLtFunction = false;
@@ -72,8 +111,6 @@ struct AnalyserModel::AnalyserModelImpl
     bool mNeedAcschFunction = false;
     bool mNeedAcothFunction = false;
 
-    std::map<uintptr_t, bool> mCachedEquivalentVariables;
-
     static AnalyserModelPtr create(const ModelPtr &model = nullptr);
 
     AnalyserModelImpl(const ModelPtr &model);