iox-eclipse-iceoryx#1030 Speed up ServiceDiscovery

iceoryx_binding_c/test/moduletests/test_service_discovery.cpp

@@ -17,6 +17,7 @@
 #include "iceoryx_hoofs/error_handling/error_handling.hpp"
 #include "iceoryx_hoofs/testing/fatal_failure.hpp"
 #include "iceoryx_posh/runtime/service_discovery.hpp"
+#include "iceoryx_posh/testing/roudi_environment/minimal_roudi_config.hpp"
 #include "iceoryx_posh/testing/roudi_gtest.hpp"
 
 using namespace iox;
@@ -39,6 +40,10 @@ using description_vector = vector<iox_service_description_t, MAX_FINDSERVICE_RES
 class iox_service_discovery_test : public RouDi_GTest
 {
   public:
+    iox_service_discovery_test()
+        : RouDi_GTest(MinimalRouDiConfigBuilder().introspectionChunkCount(4).create())
+    {
+    }
     void SetUp() override
     {
         iox_runtime_init("runtime");
@@ -76,6 +81,10 @@ TEST_F(iox_service_discovery_test,
        FindServiceWithCallableAndContextDataWithNullptrsForServiceInstanceEventReturnsAllServices)
 {
     ::testing::Test::RecordProperty("TEST_ID", "09a2cd6c-fba9-4b9d-af96-c5a6cc168d98");
+
+    // let the roudi discovery loop run at least once
+    InterOpWait();
+
     iox_service_discovery_find_service_apply_callable_with_context_data(
         sut, nullptr, nullptr, nullptr, findHandler, &searchResult, MessagingPattern_PUB_SUB);
     for (const auto& service : searchResult)
@@ -94,6 +103,8 @@ TEST_F(iox_service_discovery_test, FindServiceWithCallableAndContextDataReturnsO
     ASSERT_NE(publisher, nullptr);
     const iox_service_description_t SERVICE_DESCRIPTION = iox_pub_get_service_description(publisher);
 
+    InterOpWait();
+
     iox_service_discovery_find_service_apply_callable_with_context_data(sut,
                                                                         SERVICE_DESCRIPTION.serviceString,
                                                                         SERVICE_DESCRIPTION.instanceString,
@@ -112,6 +123,9 @@ TEST_F(iox_service_discovery_test, FindServiceWithCallableAndContextDataReturnsO
 TEST_F(iox_service_discovery_test, FindServiceWithCallableWithNullptrsForServiceInstanceEventFindsCorrectServices)
 {
     ::testing::Test::RecordProperty("TEST_ID", "ec565ca3-7494-42d7-9440-2000f1513759");
+
+    InterOpWait();
+
     auto findHandler = [](const iox_service_description_t s) { EXPECT_THAT(s.instanceString, StrEq("RouDi_ID")); };
     iox_service_discovery_find_service_apply_callable(
         sut, nullptr, nullptr, nullptr, findHandler, MessagingPattern_PUB_SUB);
@@ -126,6 +140,8 @@ TEST_F(iox_service_discovery_test, FindServiceWithCallableReturnsFindsCorrectSer
     auto* publisher = iox_pub_init(&storage, "service", "instance", "event", &options);
     ASSERT_NE(publisher, nullptr);
 
+    InterOpWait();
+
     auto findHandler = [](const iox_service_description_t s) {
         EXPECT_THAT(s.serviceString, StrEq("service"));
         EXPECT_THAT(s.instanceString, StrEq("instance"));
@@ -140,6 +156,9 @@ TEST_F(iox_service_discovery_test, FindServiceWithCallableReturnsFindsCorrectSer
 TEST_F(iox_service_discovery_test, FindServiceWithNullptrsForServiceInstanceEventReturnsAllServices)
 {
     ::testing::Test::RecordProperty("TEST_ID", "75b411d7-b8c7-42d5-8acd-3916fd172081");
+
+    InterOpWait();
+
     const uint64_t SERVICE_CONTAINER_CAPACITY = 10U;
     iox_service_description_t serviceContainer[SERVICE_CONTAINER_CAPACITY];
     uint64_t missedServices = 0U;
@@ -170,6 +189,8 @@ TEST_F(iox_service_discovery_test, FindServiceReturnsOfferedService)
     ASSERT_NE(publisher, nullptr);
     const iox_service_description_t SERVICE_DESCRIPTION = iox_pub_get_service_description(publisher);
 
+    InterOpWait();
+
     const uint64_t SERVICE_CONTAINER_CAPACITY = 10U;
     iox_service_description_t serviceContainer[SERVICE_CONTAINER_CAPACITY];
     uint64_t missedServices = 0U;
@@ -194,6 +215,9 @@ TEST_F(iox_service_discovery_test, FindServiceReturnsOfferedService)
 TEST_F(iox_service_discovery_test, FindServiceReturnsCorrectNumberOfServicesWhenServiceContainerTooSmall)
 {
     ::testing::Test::RecordProperty("TEST_ID", "01047b88-f257-447c-8c5e-9bef7c358433");
+
+    InterOpWait();
+
     const uint64_t SERVICE_CONTAINER_CAPACITY = 3U;
     iox_service_description_t serviceContainer[SERVICE_CONTAINER_CAPACITY];
     uint64_t missedServices = 0U;

iceoryx_posh/include/iceoryx_posh/internal/roudi/port_manager.hpp

@@ -198,7 +198,7 @@ class PortManager
 
     bool isInternal(const capro::ServiceDescription& service) const noexcept;
 
-    void publishServiceRegistry() const noexcept;
+    void publishServiceRegistry() noexcept;
 
     const ServiceRegistry& serviceRegistry() const noexcept;
 

iceoryx_posh/include/iceoryx_posh/internal/roudi/service_registry.hpp

@@ -91,11 +91,15 @@ class ServiceRegistry
               const optional<capro::IdString_t>& event,
               function_ref<void(const ServiceDescriptionEntry&)> callable) const noexcept;
 
-    /// @brief Applys a callable to all entries
+    /// @brief Applies a callable to all entries
     /// @param[in] callable, callable to apply to each entry
     /// @note Can be used to obtain all entries or count them
     void forEach(function_ref<void(const ServiceDescriptionEntry&)> callable) const noexcept;
 
+    /// @brief Checks whether the registry data changed since the last time this method was called
+    /// @return true when the registry changed since the last call, false otherwise
+    bool hasDataChangedSinceLastCall() noexcept;
+
   private:
     using Entry_t = optional<ServiceDescriptionEntry>;
     using ServiceDescriptionContainer_t = vector<Entry_t, CAPACITY>;
@@ -109,6 +113,8 @@ class ServiceRegistry
     // for the filling pattern of a vector (prefer entries close to the front)
     uint32_t m_freeIndex{NO_INDEX};
 
+    bool m_dataChanged{true}; // initially true in order to also get notified of the empty registry
+
   private:
     uint32_t findIndex(const capro::ServiceDescription& serviceDescription) const noexcept;
 

iceoryx_posh/source/roudi/port_manager.cpp

@@ -115,6 +115,8 @@ void PortManager::doDiscovery() noexcept
     handleNodes();
 
     handleConditionVariables();
+
+    publishServiceRegistry();
 }
 
 void PortManager::handlePublisherPorts() noexcept
@@ -1036,8 +1038,13 @@ popo::InterfacePortData* PortManager::acquireInterfacePortData(capro::Interfaces
     }
 }
 
-void PortManager::publishServiceRegistry() const noexcept
+void PortManager::publishServiceRegistry() noexcept
 {
+    if (!m_serviceRegistry.hasDataChangedSinceLastCall())
+    {
+        return;
+    }
+
     if (!m_serviceRegistryPublisherPortData.has_value())
     {
         // should not happen (except during RouDi shutdown)
@@ -1071,13 +1078,11 @@ void PortManager::addPublisherToServiceRegistry(const capro::ServiceDescription&
         IOX_LOG(WARN) << "Could not add publisher with service description '" << service << "' to service registry!";
         errorHandler(PoshError::POSH__PORT_MANAGER_COULD_NOT_ADD_SERVICE_TO_REGISTRY, ErrorLevel::MODERATE);
     });
-    publishServiceRegistry();
 }
 
 void PortManager::removePublisherFromServiceRegistry(const capro::ServiceDescription& service) noexcept
 {
     m_serviceRegistry.removePublisher(service);
-    publishServiceRegistry();
 }
 
 void PortManager::addServerToServiceRegistry(const capro::ServiceDescription& service) noexcept
@@ -1086,13 +1091,11 @@ void PortManager::addServerToServiceRegistry(const capro::ServiceDescription& se
         IOX_LOG(WARN) << "Could not add server with service description '" << service << "' to service registry!";
         errorHandler(PoshError::POSH__PORT_MANAGER_COULD_NOT_ADD_SERVICE_TO_REGISTRY, ErrorLevel::MODERATE);
     });
-    publishServiceRegistry();
 }
 
 void PortManager::removeServerFromServiceRegistry(const capro::ServiceDescription& service) noexcept
 {
     m_serviceRegistry.removeServer(service);
-    publishServiceRegistry();
 }
 
 expected<runtime::NodeData*, PortPoolError> PortManager::acquireNodeData(const RuntimeName_t& runtimeName,

iceoryx_posh/source/roudi/service_registry.cpp

@@ -37,6 +37,7 @@ expected<void, ServiceRegistry::Error> ServiceRegistry::add(const capro::Service
         // entry exists, increment counter
         auto& entry = m_serviceDescriptions[index];
         ((*entry).*count)++;
+        m_dataChanged = true;
         return ok();
     }
 
@@ -49,6 +50,7 @@ expected<void, ServiceRegistry::Error> ServiceRegistry::add(const capro::Service
         auto& entry = m_serviceDescriptions[m_freeIndex];
         entry.emplace(serviceDescription);
         (*entry).*count = 1U;
+        m_dataChanged = true;
         m_freeIndex = NO_INDEX;
         return ok();
     }
@@ -60,6 +62,7 @@ expected<void, ServiceRegistry::Error> ServiceRegistry::add(const capro::Service
         {
             entry.emplace(serviceDescription);
             (*entry).*count = 1U;
+            m_dataChanged = true;
             return ok();
         }
     }
@@ -70,6 +73,7 @@ expected<void, ServiceRegistry::Error> ServiceRegistry::add(const capro::Service
         auto& entry = m_serviceDescriptions.back();
         entry.emplace(serviceDescription);
         (*entry).*count = 1U;
+        m_dataChanged = true;
         return ok();
     }
 
@@ -102,6 +106,7 @@ void ServiceRegistry::removePublisher(const capro::ServiceDescription& serviceDe
                 entry.reset();
                 // reuse the slot in the next insertion
                 m_freeIndex = index;
+                m_dataChanged = true;
             }
         }
     }
@@ -121,6 +126,7 @@ void ServiceRegistry::removeServer(const capro::ServiceDescription& serviceDescr
                 entry.reset();
                 // reuse the slot in the next insertion
                 m_freeIndex = index;
+                m_dataChanged = true;
             }
         }
     }
@@ -135,6 +141,7 @@ void ServiceRegistry::purge(const capro::ServiceDescription& serviceDescription)
         entry.reset();
         // reuse the slot in the next insertion
         m_freeIndex = index;
+        m_dataChanged = true;
     }
 }
 
@@ -183,5 +190,12 @@ void ServiceRegistry::forEach(function_ref<void(const ServiceDescriptionEntry&)>
     }
 }
 
+bool ServiceRegistry::hasDataChangedSinceLastCall() noexcept
+{
+    auto dataChanged = m_dataChanged;
+    m_dataChanged = false;
+    return dataChanged;
+}
+
 } // namespace roudi
 } // namespace iox