search

jsk-ros-pkg / jsk_pr2eus

PR2 euslisp packages
https://github.com/jsk-ros-pkg/jsk_pr2eus
4 stars 41 forks source link

Kinematics Simulator is CPU-heavy even when idle #493

Open Affonso-Gui opened 1 year ago

Affonso-Gui commented 1 year ago

The kinematics simulator takes around 50% of my CPU even when the program is idle (waiting at the top-level prompt).

This is because the *timer-job* is still publishing joint states and updating the viewport at around 100Hz. https://github.com/jsk-ros-pkg/jsk_pr2eus/blob/master/pr2eus/robot-interface.l#L257-L270 https://github.com/jsk-ros-pkg/jsk_pr2eus/blob/master/pr2eus/robot-interface.l#L351-L353

But do we need this kind of frequency when the simulator is not processing any commands? Can we throttle this or just stop updating when there are no awaiting commands?

ref. 619251426e568e184585274c9977ec5b2f162137

pazeshun commented 1 year ago

I think publishing joint_states enables us to test some ROS nodes. For example, when you launch kinematic simulator and play rosbag of fixed camera, you may be able to test recognition-based motion code. If you throttle or stop publishing, you will break this feature, won't you?

Affonso-Gui commented 1 year ago

@pazeshun

  1. When integrating external ROS nodes into the kinematics simulator (input), some kind of ros::spin is needed. Since the *timer-job* does not run when spinning (or any other form of evaluation), the idle callback can be safely throttled / removed.
  2. When integrating external ROS nodes from an (idle) kinematics simulator (output), we need the idle callback active and publishing. However, we could still optimize the callback by skipping the xwindow update and stashing messages (after some quick testing it went from 50% CPU 50Hz output to 30% CPU 70Hz output).
pazeshun commented 1 year ago
  1. When integrating external ROS nodes into the kinematics simulator (input), some kind of ros::spin is needed. Since the *timer-job* does not run when spinning (or any other form of evaluation), the idle callback can be safely throttled / removed.

I don't think so (at least, I'm against removing). As you pointed out, when we type ros::spin or execute loop including ros::spin-once, updating joint_states is stopped. Due to this, updating TF is also stopped and recognition nodes may come to publish no new message. However, old messages were already published, and they can be stored in roseus subscriber queue. So we can test the callback of these messages by typing ros::spin*. Also, if that callback stores the message in a variable, we can test further functions using that variable.

However, we could still optimize the callback by skipping the xwindow update and stashing messages (after some quick testing it went from 50% CPU 50Hz output to 30% CPU 70Hz output).

I think we should become careful about this. Currently, grippers of many robots are not moved by :angle-vector, so we change the robot model in the kinematics simulator directly when we want to move the gripper. PR2 explicitly updates xwindow after moving its gripper but Fetch doesn't. If we skip xwindow update in the idle callback, Fetch (and maybe many other robots) apparently cannot move its gripper in kinematics simulator. And this type of bug (?) may exist in other than grippers. Does what we can get by skipping the xwindow update really balance with our effort to find and fix these bugs?

Affonso-Gui commented 1 year ago

Ultimately there likely is some kind of edge case in which this function is strictly needed (e.g. adding ros::spin-once to *timer-job*), so I agree that this may not be worth changing.

However, it is still sad that we are using so much computing resources to do so little.