-
Notifications
You must be signed in to change notification settings - Fork 340
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
Replacing old-style C++ allocator with a polymorphic memory resource (PMR) #653
Conversation
This is a change for issue #650 . |
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.
some uncrustify errors, https://build.ros2.org/job/Rpr__demos__ubuntu_jammy_amd64/235/testReport/junit/demo_nodes_cpp/uncrustify/src_topics_allocator_tutorial_cpp/
and, could you also address DCO error?
#include <string> | ||
#include <utility> | ||
#include <memory_resource> |
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.
Alphabetical order.
#include <string> | |
#include <utility> | |
#include <memory_resource> | |
#include <memory_resource> | |
#include <string> | |
#include <utility> |
Signed-off-by: Ali Ashkani Nia <[email protected]>
Signed-off-by: Ali Ashkani Nia <[email protected]>
My apologies, I should have read the ROS2 developer guides before making changes. Now, it is compliant with ROS2 style, and I have also reordered the |
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.
We could use a more advanced resource, like unsynchronized_pool_resource, for a realistic scenario.
I am not sure about this, we could. but we would not want to introduce the polymorphic memory details here, instead we would like it to be simple for demonstration code.
Yeah, I agree. What we have here is easy to understand, and is the main point of this tutorial, so I think we should keep it simple. |
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.
Thanks, this is much easier to understand.
Glad I could assist! |
For the std::array<std::uint8_t, 10*1024> buffer{};
std::pmr::monotonic_buffer_resource upstream_resource(buffer.data(), buffer.size());
std::pmr::unsynchronized_pool_resource mem_resource(&upstream_resource);
auto alloc = std::make_shared<Alloc>(&mem_resource); Again, this is just a suggestion, and overall, I agree that it might be too complicated for the tutorial. I just mentioned it for the sake of completeness. |
This is a basic example mimicking the original demo. We could use a more advanced resource, like
unsynchronized_pool_resource
, for a realistic scenario.