Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Iox #1150 remove findservice with container from discovery #1151

Merged
merged 5 commits into from
Feb 24, 2022
Merged

Iox #1150 remove findservice with container from discovery #1151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e5e84cf
iox-#1150 Remove container based findService from discovery
MatthiasKillat Feb 22, 2022
3a81748
iox-#1150 Use dynamic memory to store ServiceRegistry
MatthiasKillat Feb 22, 2022
1592b46
iox-#1150 Remove container based findService from registry
MatthiasKillat Feb 22, 2022
1879568
iox-#1150 Protect findService with a mutex
MatthiasKillat Feb 23, 2022
c299e37
iox-#1150 Add noexcept ServiceDiscovery constructor
MatthiasKillat Feb 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 5 additions & 15 deletions iceoryx_posh/include/iceoryx_posh/internal/roudi/service_registry.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ class ServiceRegistry
ReferenceCounter_t serverCount{0U};
};

using ServiceDescriptionVector_t = cxx::vector<ServiceDescriptionEntry, CAPACITY>;

/// @brief Adds a given publisher service description to registry
/// @param[in] serviceDescription, service to be added
/// @return ServiceRegistryError, error wrapped in cxx::expected
Expand All @@ -84,26 +82,19 @@ class ServiceRegistry
void purge(const capro::ServiceDescription& serviceDescription) noexcept;

/// @brief Searches for given service description in registry
/// @param[in] searchResult, reference to the vector which will be filled with the results
/// @param[in] service, string or wildcard (= iox::cxx::nullopt) to search for
/// @param[in] instance, string or wildcard (= iox::cxx::nullopt) to search for
/// @param[in] event, string or wildcard (= iox::cxx::nullopt) to search for
void find(ServiceDescriptionVector_t& searchResult,
const cxx::optional<capro::IdString_t>& service,
const cxx::optional<capro::IdString_t>& instance,
const cxx::optional<capro::IdString_t>& event) const noexcept;

/// @copydoc ServiceDiscovery::findService
/// @param[in] callable, callable to apply to each matching entry
void find(const cxx::optional<capro::IdString_t>& service,
const cxx::optional<capro::IdString_t>& instance,
const cxx::optional<capro::IdString_t>& event,
cxx::function_ref<void(const ServiceDescriptionEntry&)> callable) const noexcept;
MatthiasKillat marked this conversation as resolved.
Show resolved Hide resolved

/// @todo #415 this may not be needed later or we can move applyToAll to the public interface,
/// (we want to avoid large containers on the stack)
/// @brief Returns all service descriptions as copy
/// @return ServiceDescriptionVector_t, copy of complete service registry
const ServiceDescriptionVector_t getServices() const noexcept;
/// @brief Applys 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 applyToAll(cxx::function_ref<void(const ServiceDescriptionEntry&)> callable) const noexcept;
MatthiasKillat marked this conversation as resolved.
Show resolved Hide resolved

private:
using Entry_t = cxx::optional<ServiceDescriptionEntry>;
Expand All @@ -121,7 +112,6 @@ class ServiceRegistry
private:
uint32_t findIndex(const capro::ServiceDescription& serviceDescription) const noexcept;

void applyToAll(cxx::function_ref<void(const ServiceDescriptionEntry&)> callable) const noexcept;

cxx::expected<Error> add(const capro::ServiceDescription& serviceDescription,
ReferenceCounter_t ServiceDescriptionEntry::*count);
Expand Down
28 changes: 13 additions & 15 deletions iceoryx_posh/include/iceoryx_posh/runtime/service_discovery.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
#include "iceoryx_posh/popo/subscriber.hpp"
#include "iceoryx_posh/runtime/posh_runtime.hpp"

#include <memory>
#include <mutex>

namespace iox
{
namespace popo
Expand All @@ -41,24 +44,14 @@ enum class ServiceDiscoveryEvent : popo::EventEnumIdentifier
class ServiceDiscovery
{
public:
ServiceDiscovery() noexcept = default;
ServiceDiscovery() noexcept;

ServiceDiscovery(const ServiceDiscovery&) = delete;
ServiceDiscovery& operator=(const ServiceDiscovery&) = delete;
ServiceDiscovery(ServiceDiscovery&&) = delete;
ServiceDiscovery& operator=(ServiceDiscovery&&) = delete;
~ServiceDiscovery() noexcept = default;

/// @brief Searches all services that match the provided service description
/// @param[in] service service string to search for, a nullopt corresponds to a wildcard
/// @param[in] instance instance string to search for, a nullopt corresponds to a wildcard
/// @param[in] event event string to search for, a nullopt corresponds to a wildcard
/// @return ServiceContainer
/// ServiceContainer: container that is filled with all matching instances
ServiceContainer findService(const cxx::optional<capro::IdString_t>& service,
const cxx::optional<capro::IdString_t>& instance,
const cxx::optional<capro::IdString_t>& event,
const popo::MessagingPattern pattern) noexcept;

/// @brief Searches all services that match the provided service description and applies a function to each of them
/// @param[in] service service string to search for, a nullopt corresponds to a wildcard
/// @param[in] instance instance string to search for, a nullopt corresponds to a wildcard
Expand All @@ -78,15 +71,20 @@ class ServiceDiscovery
void invalidateTrigger(const uint64_t uniqueTriggerId);
iox::popo::WaitSetIsConditionSatisfiedCallback
getCallbackForIsStateConditionSatisfied(const popo::SubscriberState state);
roudi::ServiceRegistry m_serviceRegistry;

// use dynamic memory to reduce stack usage
/// @todo #1155 improve solution to avoid stack usage without using dynamic memory
std::unique_ptr<roudi::ServiceRegistry> m_serviceRegistry{new iox::roudi::ServiceRegistry};
MatthiasKillat marked this conversation as resolved.
Show resolved Hide resolved
std::mutex m_serviceRegistryMutex;

popo::Subscriber<roudi::ServiceRegistry> m_serviceRegistrySubscriber{
{SERVICE_DISCOVERY_SERVICE_NAME, SERVICE_DISCOVERY_INSTANCE_NAME, SERVICE_DISCOVERY_EVENT_NAME},
{1U, 1U, iox::NodeName_t("Service Registry"), true}};
};

void update();
};

} // namespace runtime

} // namespace iox

#endif // IOX_POSH_RUNTIME_SERVICE_DISCOVERY_HPP
17 changes: 0 additions & 17 deletions iceoryx_posh/source/roudi/service_registry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,6 @@ void ServiceRegistry::purge(const capro::ServiceDescription& serviceDescription)
}
}

void ServiceRegistry::find(ServiceDescriptionVector_t& searchResult,
const cxx::optional<capro::IdString_t>& service,
const cxx::optional<capro::IdString_t>& instance,
const cxx::optional<capro::IdString_t>& event) const noexcept
{
auto function = [&](const ServiceDescriptionEntry& entry) { searchResult.emplace_back(entry); };
find(service, instance, event, function);
}

void ServiceRegistry::find(const cxx::optional<capro::IdString_t>& service,
const cxx::optional<capro::IdString_t>& instance,
const cxx::optional<capro::IdString_t>& event,
Expand All @@ -173,14 +164,6 @@ void ServiceRegistry::find(const cxx::optional<capro::IdString_t>& service,
}
}

const ServiceRegistry::ServiceDescriptionVector_t ServiceRegistry::getServices() const noexcept
{
ServiceDescriptionVector_t allEntries;
auto function = [&](const ServiceDescriptionEntry& entry) { allEntries.emplace_back(entry); };
applyToAll(function);
return allEntries;
}

uint32_t ServiceRegistry::findIndex(const capro::ServiceDescription& serviceDescription) const noexcept
{
for (uint32_t i = 0; i < m_serviceDescriptions.size(); ++i)
Expand Down
26 changes: 12 additions & 14 deletions iceoryx_posh/source/runtime/service_discovery.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,17 @@ namespace iox
{
namespace runtime
{
ServiceContainer ServiceDiscovery::findService(const cxx::optional<capro::IdString_t>& service,
const cxx::optional<capro::IdString_t>& instance,
const cxx::optional<capro::IdString_t>& event,
const popo::MessagingPattern pattern) noexcept
ServiceDiscovery::ServiceDiscovery() noexcept
{
ServiceContainer searchResult;

auto lambda = [&](const capro::ServiceDescription& entry) { searchResult.emplace_back(entry); };
findService(service, instance, event, lambda, pattern);
}

return searchResult;
void ServiceDiscovery::update()
{
// allows us to use update and hence findService concurrently
std::lock_guard<std::mutex> lock(m_serviceRegistryMutex);
mossmaurice marked this conversation as resolved.
Show resolved Hide resolved
m_serviceRegistrySubscriber.take().and_then([&](popo::Sample<const roudi::ServiceRegistry>& serviceRegistrySample) {
*m_serviceRegistry = *serviceRegistrySample;
});
}

void ServiceDiscovery::findService(const cxx::optional<capro::IdString_t>& service,
Expand All @@ -45,15 +45,13 @@ void ServiceDiscovery::findService(const cxx::optional<capro::IdString_t>& servi
return;
}

m_serviceRegistrySubscriber.take().and_then([&](popo::Sample<const roudi::ServiceRegistry>& serviceRegistrySample) {
m_serviceRegistry = *serviceRegistrySample;
});
update();

switch (pattern)
{
case popo::MessagingPattern::PUB_SUB:
{
m_serviceRegistry.find(
m_serviceRegistry->find(
service, instance, event, [&](const roudi::ServiceRegistry::ServiceDescriptionEntry& serviceEntry) {
if (serviceEntry.publisherCount > 0)
{
Expand All @@ -64,7 +62,7 @@ void ServiceDiscovery::findService(const cxx::optional<capro::IdString_t>& servi
}
case popo::MessagingPattern::REQ_RES:
{
m_serviceRegistry.find(
m_serviceRegistry->find(
service, instance, event, [&](const roudi::ServiceRegistry::ServiceDescriptionEntry& serviceEntry) {
if (serviceEntry.serverCount > 0)
{
Expand Down
Loading