-
Notifications
You must be signed in to change notification settings - Fork 108
feat: ability to override the default allocator #508
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
base: rolling
Are you sure you want to change the base?
feat: ability to override the default allocator #508
Conversation
614de94
to
2772941
Compare
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.
Pull Request Overview
This PR adds functionality to override the default allocator in rcutils, enabling custom memory management integration (specifically for Unreal Engine's allocator API). The changes introduce a new public API function and modify the existing default allocator retrieval behavior.
- Adds
rcutils_set_default_allocator()
function to allow setting a custom default allocator - Modifies
rcutils_get_default_allocator()
to return the override allocator when set and valid - Introduces static storage for the override allocator with validation checks
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
File | Description |
---|---|
src/allocator.c | Implements override allocator storage and setter function, updates default allocator logic |
include/rcutils/allocator.h | Adds public API declaration for the new setter function |
2772941
to
b6e86f1
Compare
Signed-off-by: Russ Webber <[email protected]>
b6e86f1
to
499575e
Compare
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.
@russkel thanks for creating PR.
could you tell a bit more how this API is used by the application?
when this API is expected to be called? there are many packages depend on rcutils to get the default allocator such as rclcpp, rclpy, rmw and idl libraries. if this is up to the user application to set the default allocator, some uses default allocator (because they already use the default allocator before this API is called) and some uses use specified default allocator. i am not sure if that is what we want to do here, can you explain the user application behavior and expectation with this API?
i think that would be more useful to allow the user application to pass the allocator argument to be used by each API instead of changing the default allocator for everyone? and if not specified, it falls back to the default allocator as currently some APIs are implemented so?
Hi @fujitatomoya,
In this use case, I set the allocator after
I wrote this some years ago - but it is intended to set the allocator for all the libraries before they are initialised. If I remember correctly, I was unable to individually set them.
Happy to follow your lead with what you think would be a better solution. Maintaining patches on top of a custom |
I dug through the code again, and I realise this is probably why I did it this way: https://github.com/ros2/rmw/blob/e6addf2411b8ee8a2ac43d691533b8c05ae8f1b6/rmw/src/allocators.c#L28 I was never able to get |
Description
Added a function to set the default allocator, and modified the
rcutils_get_default_allocator
function to return it if it is set and valid.I needed this for use with Unreal Engine in order to expose UE's allocator API to ROS 2 so that the UE garbage cleaner could free ROS 2 objects successfully.
I have also seen references to a similar function in another PR (https://github.com/ros2/rcutils/pull/458/files#diff-8352803836a54313eb08fa96e6b6590eae0cfad976ae3187ae45d322d6f50551R101), so I assume I am not the only one to have added this functionality.
Is this user-facing behavior change?
Yes, an additional function in the rcutils allocator API.
Did you use Generative AI?
No.