Skip to content

deprecate rclcpp::spin_some and rclcpp::spin_all #2848

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

Open
wants to merge 3 commits into
base: rolling
Choose a base branch
from

Conversation

alsora
Copy link
Collaborator

@alsora alsora commented May 17, 2025

Description

Deprecate rclcpp::spin_some and rclcpp::spin_all.
These functions can lead to very poor performance for developers who are not aware that calling them will:

  • create an executor
  • add the node to the executor
  • do the spin
  • destroy the executor

If called in a loop, you are wasting a lot of CPU cycles repeating those operations every time.
This instead is a lot more efficient because only the spin operation is repeated as expected.

SingleThreadedExecutor executor;
executor.add_node(node);
while (should_spin)
{
    executor.spin_some();
}

Adding and removing nodes from executors are expensive operations that shouldn't be done at "steady state" of the system.

IMPORTANT
In this PR I'm also re-ordering the events-executor constructor arguments so that this becomes possible:

MyExecutorType executor(options);

Now all executors take the executor options as first argument.

Is this user-facing behavior change?

Yes, deprecating some functions and modifying a constructor (although it's in the experimental namespace)

Did you use Generative AI?

No

closes #2675

@alsora alsora force-pushed the asoragna/deprecate-global-spin-some-all branch 2 times, most recently from e8e8faf to f1e7310 Compare May 17, 2025 08:21
@fujitatomoya
Copy link
Collaborator

@Mergifyio rebase

alsora added 3 commits July 24, 2025 11:53
…match the other executors

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
…omments

Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
Copy link
Contributor

mergify bot commented Jul 24, 2025

rebase

✅ Branch has been successfully rebased

@fujitatomoya fujitatomoya force-pushed the asoragna/deprecate-global-spin-some-all branch from f9d55bd to 5cbdae6 Compare July 24, 2025 11:53
@fujitatomoya
Copy link
Collaborator

Pulls: #2848
Gist: https://gist.githubusercontent.com/fujitatomoya/fa4647ec68c1811cf6576850ed1d022a/raw/3ac4860bbd88f8c2ba1df0e0962127021685089d/ros2.repos
BUILD args: --packages-above-and-dependencies rclcpp rclcpp_action rclcpp_lifecycle
TEST args: --packages-above rclcpp rclcpp_action rclcpp_lifecycle
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16595

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

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.

Deprecate all spin_xxx functions in rclcpp namespace instead of spin.
4 participants