nobleo / static_executor

Library that adds alternative(s) to the default executor in ROS2
Apache License 2.0
2 stars 0 forks source link

IPC causes spurious awakes of the executor #2

Open alsora opened 4 years ago

alsora commented 4 years ago

I'm investigating spurious awakes in both the standard and this executor. See https://github.com/ros2/rclcpp/issues/1021 for the standard executor

The problem mentioned in the ticket above does not appear in the static executor when I run that dummy executable with only 1 timer. However, if I run a system made of 2 nodes (1 pub and 1 sub) in the same executor with IPC on, I see the exact same pattern.

I think that the root cause is how the static executor handles the waitable objects: in each "iteration" of the static executor:

@MartinCornelis2 what do you think? I think that this problem with IPC is not present with the standard executor because only 1 item is processed in every iteration and the waitables are managed by the memory strategy.

MartinCornelis2 commented 4 years ago

Is this fixed / captured by the PR that was just merged, or is this a separate issue?

Additionally, if this is a separate issue, what impact does it have on the executor? I agree that the executor waking up for no reason is not desirable, but it does not induce unexpected behaviour and I don't expect the performance impact to be major. Can you give me an indication of the urgency of the fix?

Also, what fix would you suggest. Is there another check besides is_ready() that can be performed to prevent this behaviour. And if so, what is the performance impact of the check vs. the performance impact of the unnecessary awakes?

alsora commented 4 years ago

This is a separate issue from the PR created by @mauropasse . However, that PR already contributes to mitigate the overhead, because it makes sure that no work is performed in these "spurious" awakes.

I agree with you that the impact should no be that much, anyhow, I'm creating some tests for exactly measuring it.

Unfortunately I don't have a solution at the moment, I opened this ticket mainly to make you aware of the issue. I will investigate more at how the SingleThreadExecutor uses the memory strategy to get ready executables, because that approach apparently fixes the problem. I will see if it's possible to integrate it in the StaticExecutor to gain some performance.