From 7bd94748e50b16e1b4debdf3f46514e02dec4696 Mon Sep 17 00:00:00 2001
From: Rainer Kartmann <rainer.kartmann@kit.edu>
Date: Fri, 16 Jul 2021 12:18:48 +0200
Subject: [PATCH] Change wm::Memory::commit() to run updates and lock per core
 segment

---
 .../libraries/armem/core/base/MemoryBase.h    |  2 +-
 .../armem/core/workingmemory/Memory.cpp       | 52 +++++++++++++++++++
 .../armem/core/workingmemory/Memory.h         | 10 ++++
 3 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/source/RobotAPI/libraries/armem/core/base/MemoryBase.h b/source/RobotAPI/libraries/armem/core/base/MemoryBase.h
index 5fe6f875a..fc29ce6f5 100644
--- a/source/RobotAPI/libraries/armem/core/base/MemoryBase.h
+++ b/source/RobotAPI/libraries/armem/core/base/MemoryBase.h
@@ -223,7 +223,7 @@ namespace armarx::armem::base
          * @param commit The commit.
          * @return The resulting memory IDs.
          */
-        std::vector<UpdateResult> update(const Commit& commit)
+        virtual std::vector<UpdateResult> update(const Commit& commit)
         {
             std::vector<UpdateResult> results;
             for (const auto& update : commit.updates)
diff --git a/source/RobotAPI/libraries/armem/core/workingmemory/Memory.cpp b/source/RobotAPI/libraries/armem/core/workingmemory/Memory.cpp
index 623858a44..d0895a53a 100644
--- a/source/RobotAPI/libraries/armem/core/workingmemory/Memory.cpp
+++ b/source/RobotAPI/libraries/armem/core/workingmemory/Memory.cpp
@@ -1,9 +1,58 @@
 #include "Memory.h"
 
+#include <map>
+#include <vector>
+
 
 namespace armarx::armem::wm
 {
 
+    std::vector<Memory::Base::UpdateResult>
+    Memory::update(const Commit& commit)
+    {
+        // Group updates by core segment, then update each core segment in a batch to only lock it once.
+        std::map<std::string, std::vector<const EntityUpdate*>> updatesPerCoreSegment;
+        for (const EntityUpdate& update : commit.updates)
+        {
+            updatesPerCoreSegment[update.entityID.coreSegmentName].push_back(&update);
+        }
+
+        std::vector<Memory::Base::UpdateResult> result;
+        // To throw an exception after the commit if a core segment is missing.
+        std::vector<std::string> missingCoreSegmentNames;
+        for (const auto& [coreSegmentName, updates] : updatesPerCoreSegment)
+        {
+            auto it = this->_container.find(coreSegmentName);
+            if (it != this->_container.end())
+            {
+                CoreSegment& coreSegment = it->second;
+                // Lock the core segment for the whole batch.
+                std::scoped_lock lock(coreSegment.mutex());
+
+                for (const EntityUpdate* update : updates)
+                {
+                    auto r = coreSegment.update(*update);
+                    Base::UpdateResult ret { r };
+                    ret.memoryUpdateType = UpdateType::UpdatedExisting;
+                    result.push_back(ret);
+                }
+            }
+            else
+            {
+                // Perform the other updates first, then throw afterwards.
+                missingCoreSegmentNames.push_back(coreSegmentName);
+            }
+        }
+        // Throw an exception if something went wrong.
+        if (not missingCoreSegmentNames.empty())
+        {
+            // Just throw an exception for the first entry. We can extend this exception in the future.
+            throw armem::error::MissingEntry::create<CoreSegment>(missingCoreSegmentNames.front(), *this);
+        }
+        return result;
+    }
+
+
     Memory::Base::UpdateResult Memory::update(const EntityUpdate& update)
     {
         this->_checkContainerName(update.entityID.memoryName, this->name());
@@ -25,6 +74,7 @@ namespace armarx::armem::wm
         }
     }
 
+
     Commit Memory::toCommit() const
     {
         Commit c;
@@ -35,6 +85,7 @@ namespace armarx::armem::wm
         return c;
     }
 
+
     void Memory::_copySelfWithoutData(Memory& other) const
     {
         other.id() = _id;
@@ -43,4 +94,5 @@ namespace armarx::armem::wm
             other.addCoreSegment(s.copyWithoutData());
         }
     }
+
 }
diff --git a/source/RobotAPI/libraries/armem/core/workingmemory/Memory.h b/source/RobotAPI/libraries/armem/core/workingmemory/Memory.h
index b2db9f7bc..368b36bec 100644
--- a/source/RobotAPI/libraries/armem/core/workingmemory/Memory.h
+++ b/source/RobotAPI/libraries/armem/core/workingmemory/Memory.h
@@ -28,6 +28,16 @@ namespace armarx::armem::wm
         Memory& operator=(Memory&& other) = default;
 
 
+        /**
+         * @brief Perform the commit, locking the core segments.
+         *
+         * Groups the commits by core segment, and updates each core segment
+         * in a batch, locking the core segment.
+         */
+        virtual std::vector<Base::UpdateResult> update(const Commit& commit) override;
+        /**
+         * @brief Update the memory, locking the updated core segment.
+         */
         virtual Base::UpdateResult update(const EntityUpdate& update) override;
 
 
-- 
GitLab