API for pre-allocating messages in the middleware

[mjcarroll]

Feature request

Feature description

In order to reduce dynamic memory allocations during publish and receipt of messages during the operation of a Node, this API would ideally allow for users of the RMW API to request the middleware to preallocate memory for predefined message types and sizes.

Implementation considerations

Implement a struct that hides the implementation of the buffer allocation that future calls to publish will take as an argument. If a publisher is called with the argument, then it will use the previously-allocated buffer rather than allocating a new one as necessary.

typedef struct RMW_PUBLIC_TYPE rmw_publisher_allocation_t
{
  const char * implementation_identifier;
  void * data;
} rmw_publisher_allocation_t;

typedef struct RMW_PUBLIC_TYPE rmw_subscription_allocation_t
{
  const char * implementation_identifier;
  void * data;
} rmw_subscription_allocation_t;

These can then be used by the rmw_publish and rmw_subscription methods to provide a reserved space for serializing/deserializing incoming/outgoing messages.

Before any messages were publish or take, these would need to be initialized via an API as follows:

rmw_init_publisher_allocation_t(rmw_publisher_allocation_t*, const rosidl_message_typesupport_t*, const rosidl_message_bounds_t*);
rmw_init_subscription_allocation_t(rmw_publisher_allocation_t*, const rosidl_message_typesupport_t*, const rosidl_message_bounds_t*)
rmw_fini_publisher_allocation_t(rmw_publisher_allocation_t*);
rmw_fini_subscription_allocation_t(rmw_subscription_allocation_t*)

This will also required additional information from the typesupport to get the serialized message sizes, also taking into account the potential size of unbounded fields. To account for unbounded fields, I propose adding a second structure rosidl_message_bounds_t that will give the user a way of specifying known fixed bounds for unbounded messages.

For fixed and bounded sized messages, the API should take a const rosidl_message_typesupport_t* which will then be allocated by the RMW implementation.

The rosidl_message_bounds_t structure would be on a per message basis and describe any of the unbounded fields. Besides the nominal case, there are three main cases to handle.

# Message with unbounded primitive
uint8[] data

struct Msg__bounds {
  size_t data__length = 0;
};

# Message with string array
string[] data

# special structure for delimiting string bounds
struct string__bounds {
  size_t bounds = 0;
};

struct Msg__bounds {
  size_t data__length = 0;
  string__bounds data__bounds;
};

# Message with unbounded array of complex structures
std_msgs/Header header[]

# special structure for delimiting string bounds
struct string__bounds {
  size_t bounds = 0;
};

struct std_msgs__msg__Header__bounds {
  string__bounds frame_id__bounds;
};

struct Msg__bounds {
  size_t header__length = 0;
  std_msgs__msg__Header__bounds header__bounds;
};

Serialized length can be retrieved via:

rmw_get_serialized_length(const rosidl_message_typesupport_t*, const rosidl_message_bounds_t*);

These structures can then be used in conjunction with rmw_publisher:

rmw_publisher_allocation_t alloc;
rmw_init_publisher_allocation(&alloc, ts, bounds);
# TODO: What happens when ros_message violates bounds given?
rmw_publish(publisher, ros_message, alloc);
# Also rmw_publish_serialized_message()

Or with rmw_subscriber:

rmw_subscription_allocation_t alloc;
rmw_init_subscription_allocation(&alloc, ts, bounds);
rmw_create_subscription(node, ts, topic, qos, ignore_local, alloc)

# TODO: What happens when incoming message violates bounds?
rmw_take()

Open Questions:

• What do we do when a message violates bounds?
• Drop message
• Error and allow a retry
• Realloc and retry

[wjwwood]

/cc @dejanpan

[dejanpan]

@serge-nikulin @deeplearningrobotics FYI. Can you please review @mjcarroll proposal and leave a feedback?

[serge-nikulin]

The design looks good to me. Of course, during the actual use, I might want some additions or changes but that would be a separate issue.

Regarding bounds violation, I vote for an error return. I am not sure what "allow retry" means.

[deeplearningrobotics]

@mjcarroll, @wjwwood, @dejanpan, @serge-nikulin:

Is there a higher level architecture overview regarding this feature? I think we need to first specify how this API should, for example, look like on a rclcpp level and how the memory allocation functions of common DDS implementations work. Then we can start to think about what needs to be implemented in the middle layer to provide the desired functionality to the end user.

As a side note: If you plan to cast a void * to the uint8[] data you will trigger UB according to http://eel.is/c++draft/basic.lval#11.8. While uint8 is a char in gcc it might not be for other compilers or future gcc version.

[mjcarroll]

@serge-nikulin Allow retry would be a best-effort at preallocating, so I believe it would be unsuitable for any sort of no allocations allowed. It would essentially allow the publication or subscription to allocate more memory if the message violated any of the bounds. I envisioned it as more of a convenience for users who had a good guess at message sizes a priori.

[mjcarroll]

@deeplearningrobotics There isn't one at the moment, but I can put one together to help clarify what it would look like from the rclcpp level.

[gonzodepedro]

Just for further reference. Memory allocation and frees were found on rmw_publish and rmw_take.

[wjwwood]

@gonzodepedro Any update on this issue? I know you've been working on pr's but can you summarize what's left to be done in a comment here?

[gonzodepedro]

So far we have preallocation for connext publish and subscribe, with bounded and unbounded sequences.

To be done:

• In subscription, when there is an unbounded sequence that has been "bounded" artificially, and the size of the sequence is different than the maximum, then a calloc and a free is done by the connext code. That case should be investigated further.
• In subscription, when there is a take, the convert_dds_to_ros function of the typesupport free's and then reallocates a sequence in the ros2 message.
  Ie:

508:             DDS_ReturnCode_t
26: #9    Source "/home/gonzalo/PROJECTS/ROS2/ROS2WS6/build/test_msgs/rosidl_typesupport_connext_cpp/test_msgs/msg/dds_connext/UnboundedInt32Array_Plugin.cxx", line 397, in test_msgs::msg::dds_::UnboundedInt32Array_Plugin_deserialize_sample(void*, test_msgs::msg::dds_::UnboundedInt32Array_*, RTICdrStream*, int, int, void*)
26:         394:                             if (!RTICdrStream_lookUnsignedLong(stream,&sequence_length)) {
26:         395:                                 goto fin; 
26:         396:                             }
26:       > 397:                             if (!DDS_LongSeq_set_maximum(&sample->int32_values_,sequence_length)) {
26:         398:                                 return RTI_FALSE;
26:         399:                             }
26:         400:                             if (DDS_LongSeq_get_contiguous_bufferI(&sample->int32_values_) != NULL) {

• Currently the api for preallocation uses the rosidl_message_bounds_t structure, but as we discussed this may not be necessary because it is not tied to a particular dds implementation, probably this should be removed and start passing the *__bounds struct as a void *

[mikaelarguedas]

This broke master build, mikaelarguedas/rclpy@f55005f

[clalancette]

Thanks for the heads up, ros2/rclpy#337 should fix it.