Skip to content
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

IPC causes spurious awakes of the executor #2

Open
alsora opened this issue Mar 10, 2020 · 2 comments
Open

IPC causes spurious awakes of the executor #2

alsora opened this issue Mar 10, 2020 · 2 comments

Comments

@alsora
Copy link

alsora commented Mar 10, 2020

I'm investigating spurious awakes in both the standard and this executor.
See ros2/rclcpp#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:

  • timer makes the publisher to publish a message. It's intra-process only, so the message is pushed into the subscription queue and the condition variable is triggered.
  • the executor checks waitable objects only through the is_ready() condition, that in this case consists in checking if the buffer is not empty. Since a message has just been pushed, the IPC subscription is invoked.
  • the executor goes back to wait
  • the executor wakes up immediately due to the condition variable triggered by the publisher above
  • there is no work to do
  • the executor goes back to wait

@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
Copy link
Member

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
Copy link
Author

alsora commented Mar 11, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants