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

Make get_rcl_allocator a function family (not a template) #1324

Open
wants to merge 9 commits into
base: rolling
Choose a base branch
from
70 changes: 11 additions & 59 deletions rclcpp/include/rclcpp/allocator/allocator_common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,71 +29,23 @@ namespace allocator
template<typename T, typename Alloc>
using AllocRebind = typename std::allocator_traits<Alloc>::template rebind_traits<T>;

template<typename Alloc>
void * retyped_allocate(size_t size, void * untyped_allocator)
{
auto typed_allocator = static_cast<Alloc *>(untyped_allocator);
if (!typed_allocator) {
throw std::runtime_error("Received incorrect allocator type");
}
return std::allocator_traits<Alloc>::allocate(*typed_allocator, size);
}

template<typename T, typename Alloc>
void retyped_deallocate(void * untyped_pointer, void * untyped_allocator)
{
auto typed_allocator = static_cast<Alloc *>(untyped_allocator);
if (!typed_allocator) {
throw std::runtime_error("Received incorrect allocator type");
}
auto typed_ptr = static_cast<T *>(untyped_pointer);
std::allocator_traits<Alloc>::deallocate(*typed_allocator, typed_ptr, 1);
}

template<typename T, typename Alloc>
void * retyped_reallocate(void * untyped_pointer, size_t size, void * untyped_allocator)
{
auto typed_allocator = static_cast<Alloc *>(untyped_allocator);
if (!typed_allocator) {
throw std::runtime_error("Received incorrect allocator type");
}
auto typed_ptr = static_cast<T *>(untyped_pointer);
std::allocator_traits<Alloc>::deallocate(*typed_allocator, typed_ptr, 1);
return std::allocator_traits<Alloc>::allocate(*typed_allocator, size);
}

} // namespace allocator

// Convert a std::allocator_traits-formatted Allocator into an rcl allocator
template<
typename T,
typename Alloc,
typename std::enable_if<!std::is_same<Alloc, std::allocator<void>>::value>::type * = nullptr>
rcl_allocator_t get_rcl_allocator(Alloc & allocator)
/// Return the equivalent rcl_allocator_t for the C++ standard allocator.
/**
* This assumes that the user intent behind both allocators is the
* same: Using system malloc for allocation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK this is not exactly true: std::allocator is implemented in terms of new/delete. Although new/delete are often implemented in terms of malloc/free, it is not demanded by the C++ Standard (AFAIK, might be wrong), meaning that implementations are free to implement new/delete independently from malloc/free.

*
* If you're using a custom allocator in ROS, you'll need to provide
* your own overload for this function.
*/
template<typename T>
rcl_allocator_t get_rcl_allocator(std::allocator<T> allocator)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be in the rclcpp::allocator namespace, so that it matches the folder structure include/rclcpp/allocator/....

{
rcl_allocator_t rcl_allocator = rcl_get_default_allocator();
#ifndef _WIN32
rcl_allocator.allocate = &retyped_allocate<Alloc>;
rcl_allocator.deallocate = &retyped_deallocate<T, Alloc>;
rcl_allocator.reallocate = &retyped_reallocate<T, Alloc>;
rcl_allocator.state = &allocator;
#else
(void)allocator; // Remove warning
#endif
return rcl_allocator;
}

// TODO(jacquelinekay) Workaround for an incomplete implementation of std::allocator<void>
template<
typename T,
typename Alloc,
typename std::enable_if<std::is_same<Alloc, std::allocator<void>>::value>::type * = nullptr>
rcl_allocator_t get_rcl_allocator(Alloc & allocator)
{
(void)allocator;
return rcl_get_default_allocator();
}

} // namespace allocator
} // namespace rclcpp

#endif // RCLCPP__ALLOCATOR__ALLOCATOR_COMMON_HPP_
4 changes: 2 additions & 2 deletions rclcpp/include/rclcpp/message_memory_strategy.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ class MessageMemoryStrategy
message_allocator_ = std::make_shared<MessageAlloc>();
serialized_message_allocator_ = std::make_shared<SerializedMessageAlloc>();
buffer_allocator_ = std::make_shared<BufferAlloc>();
rcutils_allocator_ = allocator::get_rcl_allocator<char, BufferAlloc>(*buffer_allocator_.get());
rcutils_allocator_ = get_rcl_allocator(*buffer_allocator_.get());
}

explicit MessageMemoryStrategy(std::shared_ptr<Alloc> allocator)
{
message_allocator_ = std::make_shared<MessageAlloc>(*allocator.get());
serialized_message_allocator_ = std::make_shared<SerializedMessageAlloc>(*allocator.get());
buffer_allocator_ = std::make_shared<BufferAlloc>(*allocator.get());
rcutils_allocator_ = allocator::get_rcl_allocator<char, BufferAlloc>(*buffer_allocator_.get());
rcutils_allocator_ = get_rcl_allocator(*buffer_allocator_.get());
}

virtual ~MessageMemoryStrategy() = default;
Expand Down
5 changes: 1 addition & 4 deletions rclcpp/include/rclcpp/publisher_options.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,7 @@ struct PublisherOptionsWithAllocator : public PublisherOptionsBase
to_rcl_publisher_options(const rclcpp::QoS & qos) const
{
rcl_publisher_options_t result = rcl_publisher_get_default_options();
using AllocatorTraits = std::allocator_traits<Allocator>;
using MessageAllocatorT = typename AllocatorTraits::template rebind_alloc<MessageT>;
auto message_alloc = std::make_shared<MessageAllocatorT>(*this->get_allocator().get());
result.allocator = rclcpp::allocator::get_rcl_allocator<MessageT>(*message_alloc);
result.allocator = get_rcl_allocator(*this->get_allocator());
result.qos = qos.get_rmw_qos_profile();

// Apply payload to rcl_publisher_options if necessary.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ class AllocatorMemoryStrategy : public memory_strategy::MemoryStrategy

rcl_allocator_t get_allocator() override
{
return rclcpp::allocator::get_rcl_allocator<void *, VoidAlloc>(*allocator_.get());
return get_rcl_allocator(*allocator_.get());
}

size_t number_of_ready_subscriptions() const override
Expand Down
5 changes: 1 addition & 4 deletions rclcpp/include/rclcpp/subscription_options.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,7 @@ struct SubscriptionOptionsWithAllocator : public SubscriptionOptionsBase
to_rcl_subscription_options(const rclcpp::QoS & qos) const
{
rcl_subscription_options_t result = rcl_subscription_get_default_options();
using AllocatorTraits = std::allocator_traits<Allocator>;
using MessageAllocatorT = typename AllocatorTraits::template rebind_alloc<MessageT>;
auto message_alloc = std::make_shared<MessageAllocatorT>(*allocator.get());
result.allocator = allocator::get_rcl_allocator<MessageT>(*message_alloc);
result.allocator = get_rcl_allocator(*this->get_allocator());
result.qos = qos.get_rmw_qos_profile();
result.rmw_subscription_options.ignore_local_publications = this->ignore_local_publications;

Expand Down
32 changes: 2 additions & 30 deletions rclcpp/test/rclcpp/allocator/test_allocator_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,36 +18,9 @@

#include "rclcpp/allocator/allocator_common.hpp"

TEST(TestAllocatorCommon, retyped_allocate) {
std::allocator<int> allocator;
void * untyped_allocator = &allocator;
void * allocated_mem =
rclcpp::allocator::retyped_allocate<std::allocator<char>>(1u, untyped_allocator);
ASSERT_NE(nullptr, allocated_mem);

auto code = [&untyped_allocator, allocated_mem]() {
rclcpp::allocator::retyped_deallocate<int, std::allocator<int>>(
allocated_mem, untyped_allocator);
};
EXPECT_NO_THROW(code());

allocated_mem = allocator.allocate(1);
ASSERT_NE(nullptr, allocated_mem);
void * reallocated_mem =
rclcpp::allocator::retyped_reallocate<int, std::allocator<int>>(
allocated_mem, 2u, untyped_allocator);
ASSERT_NE(nullptr, reallocated_mem);

auto code2 = [&untyped_allocator, reallocated_mem]() {
rclcpp::allocator::retyped_deallocate<int, std::allocator<int>>(
reallocated_mem, untyped_allocator);
};
EXPECT_NO_THROW(code2());
}

TEST(TestAllocatorCommon, get_rcl_allocator) {
std::allocator<int> allocator;
auto rcl_allocator = rclcpp::allocator::get_rcl_allocator<int>(allocator);
auto rcl_allocator = rclcpp::get_rcl_allocator(allocator);
EXPECT_NE(nullptr, rcl_allocator.allocate);
EXPECT_NE(nullptr, rcl_allocator.deallocate);
EXPECT_NE(nullptr, rcl_allocator.reallocate);
Expand All @@ -57,8 +30,7 @@ TEST(TestAllocatorCommon, get_rcl_allocator) {

TEST(TestAllocatorCommon, get_void_rcl_allocator) {
std::allocator<void> allocator;
auto rcl_allocator =
rclcpp::allocator::get_rcl_allocator<void, std::allocator<void>>(allocator);
auto rcl_allocator = rclcpp::get_rcl_allocator(allocator);
EXPECT_NE(nullptr, rcl_allocator.allocate);
EXPECT_NE(nullptr, rcl_allocator.deallocate);
EXPECT_NE(nullptr, rcl_allocator.reallocate);
Expand Down