Skip to content
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

Add non-default constructors to avoid initializing messages with dummy data #749

Open
alsora opened this issue Jun 2, 2023 · 7 comments
Assignees
Labels
more-information-needed Further information is required

Comments

@alsora
Copy link

alsora commented Jun 2, 2023

Feature request

ROS 2 C++ messages are implemented as data-structures with a default constructor.
Usually the workflow is

auto my_header = std_msgs::msg::Header();
my_header.frame_id = "hello";

This results in first zero-initializing the messages and then populating them again with the true data.
This has a non-negligible overhead in case of messages with large bounded members.

We propose to modify the IDL code to include a constructor that takes argument values, for example in the form of a brace-enclosed list

auto my_header = std_msgs::msg::Header({.frame_id = "hello"});
@clalancette
Copy link
Contributor

@alsora We think that the syntax there isn't supported until C++17 or C++20. Can you check which version supports it?

@clalancette clalancette added the more-information-needed Further information is required label Jun 15, 2023
@clalancette
Copy link
Contributor

Looking at it further, this looks to be a C++20 feature: https://en.cppreference.com/w/cpp/language/aggregate_initialization .

Also, it is possible that there don't need to be changes to the core to do this, and it would Just Work(tm) if we upgraded to C++20.

@MiguelCompany
Copy link
Contributor

I think that passing this constant to the constructor and then assign all the fields in the message will serve the same purposes.

@alsora
Copy link
Author

alsora commented Jul 3, 2023

@mauropasse did you test the SKIP mode?

@mauropasse
Copy link

Yes, passing rosidl_runtime_cpp::MessageInitialization::SKIP as argument to the message constructor has the effect of not initialize anything. So for example, the cost of instantiating an 8MB array message is the same as instantiating a message containing just a bool.

@MiguelCompany
Copy link
Contributor

It might be nice to have an overload of the LoanedMessage constructor that allows forwarding arguments (i.e. rosidl_runtime_cpp::MessageInitialization) to the message constructor when it is called here, and perhaps also passing a similar argument to this rmw function to indicate whether the borrowed memory shall be initialized or not.

@ros-discourse
Copy link

This issue has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/rosidl-message-builder-utilize-default-field-values/36745/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more-information-needed Further information is required
Projects
None yet
Development

No branches or pull requests

5 participants