Proposal of changes for RMW_Preallocate. #383
Conversation
rcl/src/rcl/publisher.c
Outdated
| rcl_publisher_options_t options; | ||
| rcl_context_t * context; | ||
| rmw_publisher_t * rmw_handle; | ||
| rmw_publisher_allocation_t * allocation; |
There was a problem hiding this comment.
You cannot just store a publish preallocation in the publisher because it is not thread safe. The preallocation needs to be created by the user or by rclcpp in order to ensure it is used in a thread safe manner.
There was a problem hiding this comment.
I agree. In that case the RCL should be changed or augmented to allow the user to pass the allocation to RCL.
ac7a66f to
c03e4fb
Compare
|
Currently the test_memory_prealloc tests are built with the LD_BIND_NOW=1 option. This is because without that option the tests fail. Apparently the dynamic linker is allocating memory on run-time and that makes the memory tests fail. |
At what step? publish? The library should already be loaded at that point. If it is TLS, we have functions to address specific cases of that, e.g.: https://github.com/ros2/rcutils/blob/fe82622f3c9d76400ad6065229777aa013fb8628/include/rcutils/error_handling.h#L101-L141 |
|
I think this is connected to the wrong card. The description should be updated to connect to ros2/rmw#159, right? |
You are right. It is now connected to the right card. Thanks |
Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
…nt ests Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
Signed-off-by: Gonzalo de Pedro <gonzalo@depedro.com.ar>
5042aa5 to
ae8e0ca
Compare
* Baseline test and force threads to yield. * Add timer tracking for executor. * Add locking and test happy-path exit conditions. * Move logic to multi_threaded_executor * Address reviewer feedback by reducing scope of set. * Expand tolerance on testing. * comment fixup Otherwise it seemed to me like it would yield twice.
The idea is to have a bool value in init_options to enable the
rmw_preallocatefeature.Connects to ros2/rmw#159