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.

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

Rename guid in liveliness_utils to keyexpr_hash. #296

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 18 additions & 12 deletions rmw_zenoh_cpp/src/detail/graph_cache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ void GraphCache::take_entities_with_events(const EntityEventMap & entities_with_
for (const auto & [local_entity, event_set] : entities_with_events) {
// Trigger callback set for this entity for the event type.
GraphEventCallbackMap::const_iterator event_callbacks_it =
event_callbacks_.find(local_entity->guid());
event_callbacks_.find(local_entity->keyexpr_hash());
if (event_callbacks_it != event_callbacks_.end()) {
for (const rmw_zenoh_event_type_t & event_type : event_set) {
GraphEventCallbacks::const_iterator callback_it =
Expand Down Expand Up @@ -1240,7 +1240,7 @@ rmw_ret_t GraphCache::service_server_is_available(

///=============================================================================
void GraphCache::set_qos_event_callback(
std::size_t entity_guid,
std::size_t entity_keyexpr_hash,
const rmw_zenoh_event_type_t & event_type,
GraphCacheEventCallback callback)
{
Expand All @@ -1253,20 +1253,20 @@ void GraphCache::set_qos_event_callback(
return;
}

const GraphEventCallbackMap::iterator event_cb_it = event_callbacks_.find(entity_guid);
const GraphEventCallbackMap::iterator event_cb_it = event_callbacks_.find(entity_keyexpr_hash);
if (event_cb_it == event_callbacks_.end()) {
// First time a callback is being set for this entity.
event_callbacks_[entity_guid] = {std::make_pair(event_type, std::move(callback))};
event_callbacks_[entity_keyexpr_hash] = {std::make_pair(event_type, std::move(callback))};
return;
}
event_cb_it->second[event_type] = std::move(callback);
}

///=============================================================================
void GraphCache::remove_qos_event_callbacks(std::size_t entity_guid)
void GraphCache::remove_qos_event_callbacks(std::size_t entity_keyexpr_hash)
{
std::lock_guard<std::mutex> lock(graph_mutex_);
event_callbacks_.erase(entity_guid);
event_callbacks_.erase(entity_keyexpr_hash);
}

///=============================================================================
Expand Down Expand Up @@ -1342,28 +1342,34 @@ std::unique_ptr<rmw_zenoh_event_status_t> GraphCache::take_event_status(
///=============================================================================
void GraphCache::set_querying_subscriber_callback(
const std::string & sub_keyexpr,
const std::size_t sub_guid,
const std::size_t sub_keyxpr_hash,
QueryingSubscriberCallback cb)
{
auto cb_it = querying_subs_cbs_.find(sub_keyexpr);
std::unordered_map<
std::string,
std::unordered_map<std::size_t, QueryingSubscriberCallback>
>::iterator cb_it = querying_subs_cbs_.find(sub_keyexpr);
if (cb_it == querying_subs_cbs_.end()) {
querying_subs_cbs_[sub_keyexpr] = std::move(
std::unordered_map<std::size_t, QueryingSubscriberCallback>{});
cb_it = querying_subs_cbs_.find(sub_keyexpr);
}
cb_it->second.insert(std::make_pair(sub_guid, std::move(cb)));
cb_it->second.insert(std::make_pair(sub_keyxpr_hash, std::move(cb)));
}

///=============================================================================
void GraphCache::remove_querying_subscriber_callback(
const std::string & sub_keyexpr,
const std::size_t sub_guid)
const std::size_t sub_keyexpr_hash)
{
auto cb_map_it = querying_subs_cbs_.find(sub_keyexpr);
std::unordered_map<
std::string,
std::unordered_map<std::size_t, QueryingSubscriberCallback>
>::iterator cb_map_it = querying_subs_cbs_.find(sub_keyexpr);
if (cb_map_it == querying_subs_cbs_.end()) {
return;
}
cb_map_it->second.erase(sub_guid);
cb_map_it->second.erase(sub_keyexpr_hash);
}

} // namespace rmw_zenoh_cpp
12 changes: 6 additions & 6 deletions rmw_zenoh_cpp/src/detail/graph_cache.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,24 +179,24 @@ class GraphCache final
/// Set a qos event callback for an entity from the current session.
/// @note The callback will be removed when the entity is removed from the graph.
void set_qos_event_callback(
std::size_t entity_guid,
std::size_t entity_keyexpr_hash,
const rmw_zenoh_event_type_t & event_type,
GraphCacheEventCallback callback);

/// Remove all qos event callbacks for an entity.
void remove_qos_event_callbacks(std::size_t entity_guid);
void remove_qos_event_callbacks(std::size_t entity_keyexpr_hash);

/// Returns true if the entity is a publisher or client. False otherwise.
static bool is_entity_pub(const liveliness::Entity & entity);

void set_querying_subscriber_callback(
const std::string & sub_keyexpr,
const std::size_t sub_guid,
const std::size_t sub_keyexpr_hash,
QueryingSubscriberCallback cb);

void remove_querying_subscriber_callback(
const std::string & sub_keyexpr,
const std::size_t sub_guid);
const std::size_t sub_keyexpr_hash);

private:
// Helper function to convert an Entity into a GraphNode.
Expand Down Expand Up @@ -287,7 +287,7 @@ class GraphCache final
GraphNode::TopicMap graph_services_ = {};

using GraphEventCallbacks = std::unordered_map<rmw_zenoh_event_type_t, GraphCacheEventCallback>;
// Map an entity's guid to a map of event callbacks.
// Map an entity's keyexpr_hash to a map of event callbacks.
// Note: Since we use unordered_map, we will only store a single callback for an
// entity string. So we do not support the case where a node create a duplicate
// pub/sub with the exact same topic, type & QoS but registers a different callback
Expand All @@ -296,7 +296,7 @@ class GraphCache final
using GraphEventCallbackMap = std::unordered_map<std::size_t, GraphEventCallbacks>;
// EventCallbackMap for each type of event we support in rmw_zenoh_cpp.
GraphEventCallbackMap event_callbacks_;
// Map key expressions to a map of sub guid and QueryingSubscriberCallback.
// Map key expressions to a map of sub keyexpr_hash and QueryingSubscriberCallback.
std::unordered_map<std::string, std::unordered_map<std::size_t,
QueryingSubscriberCallback>> querying_subs_cbs_;
// Counters to track changes to event statues for each topic.
Expand Down
10 changes: 4 additions & 6 deletions rmw_zenoh_cpp/src/detail/liveliness_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ Entity::Entity(
// Append the delimiter unless it is the last component.
this->liveliness_keyexpr_ += KEYEXPR_DELIMITER;
}
this->guid_ = std::hash<std::string>{}(this->liveliness_keyexpr_);
this->keyexpr_hash_ = std::hash<std::string>{}(this->liveliness_keyexpr_);
}

///=============================================================================
Expand Down Expand Up @@ -585,9 +585,9 @@ std::string Entity::id() const
}

///=============================================================================
std::size_t Entity::guid() const
std::size_t Entity::keyexpr_hash() const
{
return this->guid_;
return this->keyexpr_hash_;
}

///=============================================================================
Expand Down Expand Up @@ -632,9 +632,7 @@ std::string Entity::liveliness_keyexpr() const
///=============================================================================
bool Entity::operator==(const Entity & other) const
{
// TODO(Yadunund): If we decide to directly store the guid as a
// rmw_gid_t type, we should rely on rmw_compare_gids_equal() instead.
return other.guid() == guid_;
return other.keyexpr_hash() == keyexpr_hash_;
}

///=============================================================================
Expand Down
16 changes: 7 additions & 9 deletions rmw_zenoh_cpp/src/detail/liveliness_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,11 @@ class Entity
std::string nid() const;

// Get the id of the entity local to a zenoh session.
// Use guid() to retrieve a globally unique id.
// Use keyexpr_hash() to retrieve a globally unique id.
std::string id() const;

// Interim method to get a globally unique id for this entity which is the hash of the keyexpr.
// TODO(Yadunund): Should this return a rmw_gid_t?
// This is named guid and not gid to remain distinct as it is not of type rmw_gid_t.
std::size_t guid() const;
std::size_t keyexpr_hash() const;

/// Get the entity type.
EntityType type() const;
Expand All @@ -171,7 +169,7 @@ class Entity
/// Get the liveliness keyexpr for this entity.
std::string liveliness_keyexpr() const;

// Two entities are equal if their guids are equal.
// Two entities are equal if their keyexpr_hash are equal.
bool operator==(const Entity & other) const;

private:
Expand All @@ -186,7 +184,7 @@ class Entity
std::string zid_;
std::string nid_;
std::string id_;
std::size_t guid_;
std::size_t keyexpr_hash_;
EntityType type_;
NodeInfo node_info_;
std::optional<TopicInfo> topic_info_;
Expand Down Expand Up @@ -254,7 +252,7 @@ struct hash<rmw_zenoh_cpp::liveliness::Entity>
{
auto operator()(const rmw_zenoh_cpp::liveliness::Entity & entity) const -> size_t
{
return entity.guid();
return entity.keyexpr_hash();
}
};

Expand All @@ -263,7 +261,7 @@ struct hash<rmw_zenoh_cpp::liveliness::ConstEntityPtr>
{
auto operator()(const rmw_zenoh_cpp::liveliness::ConstEntityPtr & entity) const -> size_t
{
return entity->guid();
return entity->keyexpr_hash();
}
};

Expand All @@ -274,7 +272,7 @@ struct equal_to<rmw_zenoh_cpp::liveliness::ConstEntityPtr>
const rmw_zenoh_cpp::liveliness::ConstEntityPtr & lhs,
const rmw_zenoh_cpp::liveliness::ConstEntityPtr & rhs) const -> bool
{
return lhs->guid() == rhs->guid();
return lhs->keyexpr_hash() == rhs->keyexpr_hash();
}
};
} // namespace std
Expand Down
4 changes: 2 additions & 2 deletions rmw_zenoh_cpp/src/detail/rmw_publisher_data.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,10 @@ rmw_ret_t PublisherData::publish_serialized_message(
}

///=============================================================================
std::size_t PublisherData::guid() const
std::size_t PublisherData::keyexpr_hash() const
{
std::lock_guard<std::mutex> lock(mutex_);
return entity_->guid();
return entity_->keyexpr_hash();
}

///=============================================================================
Expand Down
4 changes: 2 additions & 2 deletions rmw_zenoh_cpp/src/detail/rmw_publisher_data.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ class PublisherData final
const rmw_serialized_message_t * serialized_message,
std::optional<zc_owned_shm_manager_t> & shm_manager);

// Get a copy of the GUID of this PublisherData's liveliness::Entity.
std::size_t guid() const;
// Get a copy of the keyexpr_hash of this PublisherData's liveliness::Entity.
std::size_t keyexpr_hash() const;

// Get a copy of the TopicInfo of this PublisherData.
liveliness::TopicInfo topic_info() const;
Expand Down
6 changes: 3 additions & 3 deletions rmw_zenoh_cpp/src/detail/rmw_subscription_data.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ std::shared_ptr<SubscriptionData> SubscriptionData::make(
std::weak_ptr<SubscriptionData> data_wp = sub_data;
graph_cache->set_querying_subscriber_callback(
sub_data->entity_->topic_info().value().topic_keyexpr_,
sub_data->entity_->guid(),
sub_data->entity_->keyexpr_hash(),
[data_wp](const std::string & queryable_prefix) -> void
{
auto sub_data = data_wp.lock();
Expand Down Expand Up @@ -359,10 +359,10 @@ SubscriptionData::SubscriptionData(
}

///=============================================================================
std::size_t SubscriptionData::guid() const
std::size_t SubscriptionData::keyexpr_hash() const
{
std::lock_guard<std::mutex> lock(mutex_);
return entity_->guid();
return entity_->keyexpr_hash();
}

///=============================================================================
Expand Down
4 changes: 2 additions & 2 deletions rmw_zenoh_cpp/src/detail/rmw_subscription_data.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ class SubscriptionData final
const void * ros_message,
std::optional<zc_owned_shm_manager_t> & shm_manager);

// Get a copy of the GUID of this SubscriptionData's liveliness::Entity.
std::size_t guid() const;
// Get a copy of the keyexpr_hash of this SubscriptionData's liveliness::Entity.
std::size_t keyexpr_hash() const;

// Get a copy of the TopicInfo of this SubscriptionData.
liveliness::TopicInfo topic_info() const;
Expand Down
4 changes: 2 additions & 2 deletions rmw_zenoh_cpp/src/rmw_event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ rmw_publisher_event_init(
// Register the event with graph cache.
std::weak_ptr<rmw_zenoh_cpp::PublisherData> data_wp = pub_data;
context_impl->graph_cache()->set_qos_event_callback(
pub_data->guid(),
pub_data->keyexpr_hash(),
zenoh_event_type,
[data_wp,
zenoh_event_type](std::unique_ptr<rmw_zenoh_cpp::rmw_zenoh_event_status_t> zenoh_event)
Expand Down Expand Up @@ -130,7 +130,7 @@ rmw_subscription_event_init(

// std::weak_ptr<rmw_zenoh_cpp::SubscriptionData> data_wp = sub_data;
sub_data->graph_cache()->set_qos_event_callback(
sub_data->guid(),
sub_data->keyexpr_hash(),
zenoh_event_type,
[sub_data,
zenoh_event_type](std::unique_ptr<rmw_zenoh_cpp::rmw_zenoh_event_status_t> zenoh_event)
Expand Down
8 changes: 4 additions & 4 deletions rmw_zenoh_cpp/src/rmw_zenoh.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ rmw_destroy_publisher(rmw_node_t * node, rmw_publisher_t * publisher)
return RMW_RET_INVALID_ARGUMENT;
}
// Remove any event callbacks registered to this publisher.
context_impl->graph_cache()->remove_qos_event_callbacks(pub_data->guid());
context_impl->graph_cache()->remove_qos_event_callbacks(pub_data->keyexpr_hash());
// Remove the PublisherData from NodeData.
node_data->delete_pub_data(publisher);

Expand Down Expand Up @@ -1028,13 +1028,13 @@ rmw_destroy_subscription(rmw_node_t * node, rmw_subscription_t * subscription)
rcutils_allocator_t * allocator = &node->context->options.allocator;

// Remove the registered callback from the GraphCache if any.
const std::size_t guid = sub_data->guid();
const std::size_t keyexpr_hash = sub_data->keyexpr_hash();
context_impl->graph_cache()->remove_querying_subscriber_callback(
sub_data->topic_info().topic_keyexpr_,
guid
keyexpr_hash
);
// Remove any event callbacks registered to this subscription.
context_impl->graph_cache()->remove_qos_event_callbacks(guid);
context_impl->graph_cache()->remove_qos_event_callbacks(keyexpr_hash);
// Finally remove the SubscriptionData from NodeData.
node_data->delete_sub_data(subscription);

Expand Down