Skip to content

Commit 1bf4e6a

Browse files
authored
Revert "improve lookup time for matches_any_publishers(). (#3068)" (#3077)
This reverts commit b6730f9. Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
1 parent fc1afcb commit 1bf4e6a

File tree

3 files changed

+10
-80
lines changed

3 files changed

+10
-80
lines changed

rclcpp/include/rclcpp/experimental/intra_process_manager.hpp

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -386,40 +386,6 @@ class IntraProcessManager
386386
std::vector<uint64_t> take_ownership_subscriptions;
387387
};
388388

389-
/// Hash function for rmw_gid_t to enable use in unordered_map
390-
struct rmw_gid_hash
391-
{
392-
std::size_t operator()(const rmw_gid_t & gid) const noexcept
393-
{
394-
// Using the FNV-1a hash algorithm on the gid data
395-
constexpr std::size_t FNV_prime = 1099511628211u;
396-
std::size_t result = 14695981039346656037u;
397-
398-
for (std::size_t i = 0; i < RMW_GID_STORAGE_SIZE; ++i) {
399-
result ^= gid.data[i];
400-
result *= FNV_prime;
401-
}
402-
return result;
403-
}
404-
};
405-
406-
/// Equality comparison for rmw_gid_t to enable use in unordered_map
407-
struct rmw_gid_equal
408-
{
409-
bool operator()(const rmw_gid_t & lhs, const rmw_gid_t & rhs) const noexcept
410-
{
411-
// Compare implementation identifier first for fast rejection
412-
if (lhs.implementation_identifier != rhs.implementation_identifier) {
413-
return false;
414-
}
415-
// Compare the data bytes
416-
return std::equal(
417-
std::begin(lhs.data),
418-
std::end(lhs.data),
419-
std::begin(rhs.data));
420-
}
421-
};
422-
423389
using SubscriptionMap =
424390
std::unordered_map<uint64_t, rclcpp::experimental::SubscriptionIntraProcessBase::WeakPtr>;
425391

@@ -432,16 +398,6 @@ class IntraProcessManager
432398
using PublisherToSubscriptionIdsMap =
433399
std::unordered_map<uint64_t, SplittedSubscriptions>;
434400

435-
/// Structure to store publisher information in GID lookup map
436-
struct PublisherInfo
437-
{
438-
uint64_t pub_id;
439-
rclcpp::PublisherBase::WeakPtr publisher;
440-
};
441-
442-
using GidToPublisherInfoMap =
443-
std::unordered_map<rmw_gid_t, PublisherInfo, rmw_gid_hash, rmw_gid_equal>;
444-
445401
RCLCPP_PUBLIC
446402
static
447403
uint64_t
@@ -684,7 +640,6 @@ class IntraProcessManager
684640
SubscriptionMap subscriptions_;
685641
PublisherMap publishers_;
686642
PublisherBufferMap publisher_buffers_;
687-
GidToPublisherInfoMap gid_to_publisher_info_;
688643

689644
mutable std::shared_timed_mutex mutex_;
690645
};

rclcpp/src/rclcpp/intra_process_manager.cpp

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,6 @@ IntraProcessManager::add_publisher(
5151
}
5252
}
5353

54-
// Add GID to publisher info mapping for fast lookups (stores both ID and weak_ptr)
55-
gid_to_publisher_info_[publisher->get_gid()] = {pub_id, publisher};
56-
5754
// Initialize the subscriptions storage for this publisher.
5855
pub_to_subs_[pub_id] = SplittedSubscriptions();
5956

@@ -101,15 +98,6 @@ IntraProcessManager::remove_publisher(uint64_t intra_process_publisher_id)
10198
{
10299
std::unique_lock<std::shared_timed_mutex> lock(mutex_);
103100

104-
// Remove GID to publisher info mapping
105-
auto pub_it = publishers_.find(intra_process_publisher_id);
106-
if (pub_it != publishers_.end()) {
107-
auto publisher = pub_it->second.lock();
108-
if (publisher) {
109-
gid_to_publisher_info_.erase(publisher->get_gid());
110-
}
111-
}
112-
113101
publishers_.erase(intra_process_publisher_id);
114102
publisher_buffers_.erase(intra_process_publisher_id);
115103
pub_to_subs_.erase(intra_process_publisher_id);
@@ -120,15 +108,16 @@ IntraProcessManager::matches_any_publishers(const rmw_gid_t * id) const
120108
{
121109
std::shared_lock<std::shared_timed_mutex> lock(mutex_);
122110

123-
// Single O(1) hash map lookup - struct contains both ID and weak_ptr
124-
auto it = gid_to_publisher_info_.find(*id);
125-
if (it == gid_to_publisher_info_.end()) {
126-
return false;
111+
for (auto & publisher_pair : publishers_) {
112+
auto publisher = publisher_pair.second.lock();
113+
if (!publisher) {
114+
continue;
115+
}
116+
if (*publisher.get() == id) {
117+
return true;
118+
}
127119
}
128-
129-
// Verify the publisher still exists by checking the weak_ptr
130-
auto publisher = it->second.publisher.lock();
131-
return publisher != nullptr;
120+
return false;
132121
}
133122

134123
size_t

rclcpp/test/rclcpp/test_intra_process_manager.cpp

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -162,14 +162,7 @@ class PublisherBase
162162
explicit PublisherBase(const std::string & topic, const rclcpp::QoS & qos)
163163
: topic_name(topic),
164164
qos_profile(qos)
165-
{
166-
// Initialize a mock GID with unique data based on this pointer
167-
gid_.implementation_identifier = "mock_rmw";
168-
auto ptr_value = reinterpret_cast<std::uintptr_t>(this);
169-
for (size_t i = 0; i < RMW_GID_STORAGE_SIZE; ++i) {
170-
gid_.data[i] = static_cast<uint8_t>((ptr_value >> (i * 8)) & 0xFF);
171-
}
172-
}
165+
{}
173166

174167
virtual ~PublisherBase()
175168
{}
@@ -199,12 +192,6 @@ class PublisherBase
199192
return qos_profile.durability() == rclcpp::DurabilityPolicy::TransientLocal;
200193
}
201194

202-
const rmw_gid_t &
203-
get_gid() const
204-
{
205-
return gid_;
206-
}
207-
208195
bool
209196
operator==([[maybe_unused]] const rmw_gid_t & gid) const
210197
{
@@ -223,7 +210,6 @@ class PublisherBase
223210
private:
224211
std::string topic_name;
225212
rclcpp::QoS qos_profile;
226-
rmw_gid_t gid_;
227213
};
228214

229215
template<typename T, typename Alloc = std::allocator<void>>

0 commit comments

Comments
 (0)