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

Fix unnecessary allocations in executor.cpp #2135

Merged

Conversation

cwecht
Copy link
Contributor

@cwecht cwecht commented Mar 21, 2023

std::function will allocate on every call of take_and_do_error_handling (at least with libstdc++ as in this implementation std::function's small buffer can only fit two pointers but most of the time, the lambdas will contain 3 pointers). This impose multiple heap allocations (and deallocations) on every callback.

This comes with the cost of increased binary size. But I suppose it's worth it as this change also enables better/easier inling and avoids indirect function calls through the std::function machinery.

@cwecht cwecht force-pushed the fix_unnecessary_allocations_in_executor branch from 9ed13a9 to dc1dfdd Compare March 21, 2023 08:07
Signed-off-by: Christopher Wecht <cwecht@mailbox.org>
@cwecht cwecht force-pushed the fix_unnecessary_allocations_in_executor branch from dc1dfdd to d13f2aa Compare March 21, 2023 08:09
@mjcarroll mjcarroll self-assigned this Mar 21, 2023
@mjcarroll
Copy link
Member

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mjcarroll
Copy link
Member

The windows test failures are known flakes.

@mjcarroll mjcarroll merged commit 73d555b into ros2:rolling Apr 3, 2023
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request Apr 28, 2023
Signed-off-by: Christopher Wecht <cwecht@mailbox.org>
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request Apr 29, 2023
Signed-off-by: Christopher Wecht <cwecht@mailbox.org>
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request Apr 29, 2023
Signed-off-by: Christopher Wecht <cwecht@mailbox.org>
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request Apr 29, 2023
Signed-off-by: Christopher Wecht <cwecht@mailbox.org>
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request May 3, 2023
Signed-off-by: Christopher Wecht <cwecht@mailbox.org>
Barry-Xu-2018 pushed a commit to Barry-Xu-2018/rclcpp that referenced this pull request Jan 12, 2024
Signed-off-by: Christopher Wecht <cwecht@mailbox.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants