Skip to content

Commit

Permalink
Rename guid in liveliness_utils to keyexpr_hash.
Browse files Browse the repository at this point in the history
This more clearly reflect what it is, which is a
hash of the keyexpression that is used to index into
various maps.

Signed-off-by: Chris Lalancette <[email protected]>
  • Loading branch information
clalancette committed Oct 10, 2024
1 parent 439d6dc commit a43b861
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 48 deletions.
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

0 comments on commit a43b861

Please sign in to comment.