-
Notifications
You must be signed in to change notification settings - Fork 435
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
Use spin() in component_manager_isolated.hpp #1881
Conversation
…lated.hpp spin_until_future_complete() is more inefficient as it needs to check the state of the future and the timeout after every work iteration Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
sounds great, would you mind sharing the platform you benchmarked this? |
rclcpp_components/include/rclcpp_components/component_manager_isolated.hpp
Show resolved
Hide resolved
@fujitatomoya the tests have been carried out by @gezp, he can definitely provide you more details but I think the 10% less cpu was on a raspberry pi 4, while on x86 the improvement was ~5% |
Rasp4 is one of the common benchmark platform we have too, this is good information. thanks! |
rclcpp_components/include/rclcpp_components/component_manager_isolated.hpp
Show resolved
Hide resolved
LGTM with green CI |
Hi, can someone give a look at the windows CI above? It says that there are more than 70 test failures. They all seem unrelated tests to what I'm doing here, but they are really too many to ignore. Looking at the logs it may also seem an infrastructure issue, with some tests unable to start. |
Same failures are happening on nightlies, e.g.: https://ci.ros2.org/view/nightly/job/nightly_win_rel/2207/testReport/junit/(root)/projectroot/test_action_client/. |
Ignoring the windows pytest issues that also happen on nightlies, going in! |
Replace
spin_until_future_complete
withspin
incomponent_manager_isolated.hpp
.spin_until_future_complete()
is more inefficient as it needs to check the state of the future and the timeout after every work iteration.This PR addresses the race-condition issues mentioned in #1781 by using the recently added
is_spinning()
APITests on the navigation stack show 10% less CPU usage when using
spin()
rather thanspin_until_future_complete()
FYI @SteveMacenski @gezp @ivanpauno