-
Notifications
You must be signed in to change notification settings - Fork 417
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
stevewolter
wants to merge
9
commits into
ros2:rolling
Choose a base branch
from
stevewolter:master
base: rolling
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
32e7fe8
Make get_rcl_allocator a function family
stevewolter b98e90f
Addressed code style test failures.
stevewolter e63917f
Update comments.
stevewolter 928bf4f
Also update recently added tests.
stevewolter 3f97bc3
Added reference to bug number.
stevewolter fca62b2
Extend allocator lifetime in options.
stevewolter 7f0e5b8
Drop the generic version of get_rcl_allocator.
stevewolter af20431
Update rclcpp/include/rclcpp/allocator/allocator_common.hpp
stevewolter c51a72e
Revert accidental deletion of allocator creation.
stevewolter File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
* | ||
* 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to be in the |
||
{ | ||
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_ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.