From bb3c5a825743f51dcf8a586404d6bd7853cd9a31 Mon Sep 17 00:00:00 2001 From: Rainer Kartmann <rainer.kartmann@kit.edu> Date: Tue, 20 Jul 2021 13:28:36 +0200 Subject: [PATCH] Update locking scheme --- .../armem_objects/server/class/Segment.cpp | 41 ++++++++----------- .../armem_objects/server/instance/Segment.cpp | 29 +++++++------ 2 files changed, 31 insertions(+), 39 deletions(-) diff --git a/source/RobotAPI/libraries/armem_objects/server/class/Segment.cpp b/source/RobotAPI/libraries/armem_objects/server/class/Segment.cpp index 49c9b6dd8..a1f678d52 100644 --- a/source/RobotAPI/libraries/armem_objects/server/class/Segment.cpp +++ b/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)); } } diff --git a/source/RobotAPI/libraries/armem_objects/server/instance/Segment.cpp b/source/RobotAPI/libraries/armem_objects/server/instance/Segment.cpp index 95611ed7a..1a18f84a1 100644 --- a/source/RobotAPI/libraries/armem_objects/server/instance/Segment.cpp +++ b/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(); } } -- GitLab