Update locking scheme

source/RobotAPI/libraries/armem_objects/server/class/Segment.cpp

@@ -90,11 +90,6 @@ namespace armarx::armem::server::obj::clazz
         std::vector<ObjectInfo> infos = objectFinder.findAllObjects(checkPaths);
 
         const MemoryID providerID = coreSegment->id().withProviderSegmentName(objectFinder.getPackageName());
-        if (not coreSegment->hasProviderSegment(providerID.providerSegmentName))
-        {
-            coreSegment->addProviderSegment(providerID.providerSegmentName);
-        }
-
         ARMARX_INFO << "Loading up to " << infos.size() << " object classes from '"
                     << objectFinder.getPackageName() << "' ...";
         Commit commit;
@@ -102,7 +97,7 @@ namespace armarx::armem::server::obj::clazz
         {
             info.setLogError(false);
 
-            EntityUpdate& update = commit.updates.emplace_back();
+            EntityUpdate& update = commit.add();
             update.entityID = providerID.withEntityName(info.id().str());
             update.timeArrived = update.timeCreated = update.timeSent = now;
 
@@ -114,7 +109,7 @@ namespace armarx::armem::server::obj::clazz
         }
         ARMARX_INFO << "Loaded " << commit.updates.size() << " object classes from '"
                     << objectFinder.getPackageName() << "'.";
-        iceMemory.commit(commit);
+        iceMemory.commitLocking(commit);
     }
 
 
@@ -133,31 +128,32 @@ namespace armarx::armem::server::obj::clazz
         {
             try
             {
-                const armem::wm::Entity& entity = coreSegment->getEntity(entityID);
-                const armem::wm::EntityInstance& instance = entity.getLatestSnapshot().getInstance(0);
-
-                arondto::ObjectClass aron;
-                aron.fromAron(instance.data());
+                std::optional<arondto::ObjectClass> aron =
+                    coreSegment->getLatestEntityInstanceDataLockingAs<arondto::ObjectClass>(entityID, 0);
+                if (not aron.has_value())
+                {
+                    return;
+                }
 
-                if (!aron.simoxXmlPath.package.empty())
+                if (not aron->simoxXmlPath.package.empty())
                 {
                     layerObject.add(viz::Object(entityID.str())
-                                    .file(aron.simoxXmlPath.package, aron.simoxXmlPath.path)
+                                    .file(aron->simoxXmlPath.package, aron->simoxXmlPath.path)
                                     .pose(pose));
                 }
 
                 if (showAABB)
                 {
                     layerAABB.add(viz::Box("AABB")
-                                  .pose(pose * simox::math::pose(aron.aabb.center))
-                                  .size(aron.aabb.extents)
+                                  .pose(pose * simox::math::pose(aron->aabb.center))
+                                  .size(aron->aabb.extents)
                                   .color(simox::Color::cyan(255, 64)));
                 }
                 if (showOOBB)
                 {
                     layerOOBB.add(viz::Box("OOBB")
-                                  .pose(pose * simox::math::pose(aron.oobb.center, aron.oobb.orientation))
-                                  .size(aron.oobb.extents)
+                                  .pose(pose * simox::math::pose(aron->oobb.center, aron->oobb.orientation))
+                                  .size(aron->oobb.extents)
                                   .color(simox::Color::lime(255, 64)));
                 }
             }
@@ -182,12 +178,10 @@ namespace armarx::armem::server::obj::clazz
         namespace fs = std::filesystem;
 
         arondto::ObjectClass data;
-
         toAron(data.id, info.id());
 
-        auto setPathIfExists = [](
-                                   armarx::arondto::PackagePath & aron,
-                                   const PackageFileLocation & location)
+        auto setPathIfExists = [](armarx::arondto::PackagePath & aron,
+                                  const PackageFileLocation & location)
         {
             if (fs::is_regular_file(location.absolutePath))
             {
@@ -273,7 +267,7 @@ namespace armarx::armem::server::obj::clazz
     {
         if (reloadButton.wasClicked())
         {
-            std::scoped_lock lock(segment.mutex());
+            // Core segment mutex will be locked on commit.
             segment.loadByObjectFinder();
             rebuild = true;
         }
@@ -331,7 +325,6 @@ namespace armarx::armem::server::obj::clazz
             const size_t index = static_cast<size_t>(showComboBox.getIndex());
             if (/*index >= 0 &&*/ index < showOptionsIndex.size())
             {
-                std::scoped_lock lock(segment.mutex());
                 segment.visualizeClass(showOptionsIndex.at(index));
             }
         }

source/RobotAPI/libraries/armem_objects/server/instance/Segment.cpp

@@ -106,7 +106,6 @@ namespace armarx::armem::server::obj::instance
         coreSegment = &iceMemory.workingMemory->addCoreSegment(p.coreSegmentName, arondto::ObjectInstance::toAronType());
         coreSegment->setMaxHistorySize(p.maxHistorySize);
 
-
         if (not p.sceneSnapshotToLoad.empty())
         {
             bool lockMemory = false;
@@ -116,6 +115,7 @@ namespace armarx::armem::server::obj::instance
 
     void Segment::connect(viz::Client arviz)
     {
+        (void) arviz;
         // ARMARX_INFO << "ArticulatedObjectVisu";
         // this->visu = std::make_unique<ArticulatedObjectVisu>(arviz, *this);
         // visu->init();
@@ -141,7 +141,7 @@ namespace armarx::armem::server::obj::instance
         stats.numUpdated = 0;
         for (const objpose::data::ProvidedObjectPose& provided : providedPoses)
         {
-            const IceUtil::Time timestamp = IceUtil::Time::microSeconds(provided.timestampMicroSeconds);
+            const Time timestamp = Time::microSeconds(provided.timestampMicroSeconds);
 
             // Check whether we have an old snapshot for this object.
             std::optional<objpose::ObjectPose> previousPose;
@@ -214,9 +214,9 @@ namespace armarx::armem::server::obj::instance
 
     void Segment::commitObjectPoses(const ObjectPoseSeq& objectPoses, const std::string& providerName)
     {
-        ARMARX_CHECK_NOT_NULL(coreSegment);
         Time now = TimeUtil::GetTime();
 
+        ARMARX_CHECK_NOT_NULL(coreSegment);
         const MemoryID coreSegmentID = coreSegment->id();
 
         Commit commit;
@@ -247,6 +247,7 @@ namespace armarx::armem::server::obj::instance
             update.instancesData.push_back(dto.toAron());
 
         }
+        // Commit non-locking.
         iceMemory.commit(commit);
     }
 
@@ -674,7 +675,6 @@ namespace armarx::armem::server::obj::instance
                     // Store non-attached pose in new snapshot.
                     storeDetachedSnapshot(*entity, data, now, input.commitAttachedPose);
                 }
-
                 if (providerName.empty())
                 {
                     providerName = data.pose.providerName;
@@ -979,7 +979,6 @@ namespace armarx::armem::server::obj::instance
 
                 if (lockMemory)
                 {
-                    std::scoped_lock lock(mutex());
                     commitSceneSnapshot(snapshot.value(), filename.string());
                 }
                 else
@@ -1032,39 +1031,39 @@ namespace armarx::armem::server::obj::instance
     }
 
 
-    void Segment::RemoteGui::update(Segment& data)
+    void Segment::RemoteGui::update(Segment& segment)
     {
         if (loadButton.wasClicked())
         {
             bool lockMemory = true;
-            data.commitSceneSnapshotFromFilename(storeLoadLine.getValue(), lockMemory);
+            segment.commitSceneSnapshotFromFilename(storeLoadLine.getValue(), lockMemory);
         }
 
         if (storeButton.wasClicked())
         {
             armem::obj::SceneSnapshot scene;
             {
-                std::scoped_lock lock(data.mutex());
-                scene = data.getSceneSnapshot();
+                std::scoped_lock lock(segment.mutex());
+                scene = segment.getSceneSnapshot();
             }
-            data.storeScene(storeLoadLine.getValue(), scene);
+            segment.storeScene(storeLoadLine.getValue(), scene);
         }
 
         if (infiniteHistory.hasValueChanged() || maxHistorySize.hasValueChanged()
             || discardSnapshotsWhileAttached.hasValueChanged())
         {
-            std::scoped_lock lock(data.mutex());
+            std::scoped_lock lock(segment.mutex());
 
             if (infiniteHistory.hasValueChanged() || maxHistorySize.hasValueChanged())
             {
-                data.p.maxHistorySize = infiniteHistory.getValue() ? -1 : maxHistorySize.getValue();
-                if (data.coreSegment)
+                segment.p.maxHistorySize = infiniteHistory.getValue() ? -1 : maxHistorySize.getValue();
+                if (segment.coreSegment)
                 {
-                    data.coreSegment->setMaxHistorySize(long(data.p.maxHistorySize));
+                    segment.coreSegment->setMaxHistorySize(long(segment.p.maxHistorySize));
                 }
             }
 
-            data.p.discardSnapshotsWhileAttached = discardSnapshotsWhileAttached.getValue();
+            segment.p.discardSnapshotsWhileAttached = discardSnapshotsWhileAttached.getValue();
         }
     }