RobotWebTools / rclnodejs

Node.js version of ROS 2.0 client
https://docs.ros.org/en/humble/Concepts/Basic/About-Client-Libraries.html?highlight=rclnodejs#community-maintained
Apache License 2.0
316 stars 70 forks source link

High CPU load if there are no registered handles #752

Closed meyerj closed 3 years ago

meyerj commented 3 years ago

From https://github.com/RobotWebTools/rclnodejs/pull/751:

Originally posted by @renehoelbling in Intermodalics#1:

Add sleep to avoid busy waiting

Without the patch we observed situations where the ros2-web-bridge process consumed 100% of CPU while looping in Executor::Run(), if there is actually nothing to do (no clients connected). But I am not familiar with rclnodejs internals to check whether it is still an issue with the latest version, and in which cases handle_manager->is_empty() can return true.

This patch was based on version 0.10.3 (for ROS dashing). In the meantime the is_empty() check has been moved to https://github.com/RobotWebTools/rclnodejs/blob/45d7bd0a5cbc7b9d5bd6b397b33abc770845771b/src/executor.cpp#L166-L167

in #563, but nothing fundamentally changed and the loop in Executor::Run() could still busy-wait. What do you think? Is this a real issue, and if yes, would you solve it in a different way?

Thanks for giving the feedback and submitting the PR. Indeed, if there are no subscriptions/clients created, the CPU load will be high (although we usually created a node with clients added).

Some thoughts: Waiting for a certain period, .e.g 1ms, could solve the high CPU load issue, but using a semaphore is a perfect way to do this because there will be no extra looping, and totally event-driven.

Originally posted by @minggangw in https://github.com/RobotWebTools/rclnodejs/issues/751#issuecomment-743775623

minggangw commented 3 years ago

Because currently the node will create parameter services by default, it's small chance that no handle is added at the beginning. I'd like to put this issue to be low-priority. Meanwhile, #763 has been submitted to solve it. https://github.com/RobotWebTools/rclnodejs/blob/6f8a1a156780b131823e48650a35d9ff4760e462/lib/node_options.js#L22-L31

meyerj commented 3 years ago

Cool, thanks!