search

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

Fix 100% CPU usage if there are no handles #751

Closed meyerj closed 3 years ago

meyerj commented 3 years ago

Originally posted by @renehoelbling in https://github.com/Intermodalics/rclnodejs/pull/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 https://github.com/RobotWebTools/rclnodejs/pull/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?

minggangw commented 3 years ago

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.

minggangw commented 3 years ago

@meyerj would you please create a new issue for this? Thanks!

meyerj commented 3 years ago

Done: https://github.com/RobotWebTools/rclnodejs/issues/752