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

[WIP] Adding template for thread priorization in components at run-time #2373

Draft
wants to merge 1 commit into
base: rolling
Choose a base branch
from

Conversation

SteveMacenski
Copy link
Collaborator

@SteveMacenski SteveMacenski commented Nov 21, 2023

Howdy,

Per a conversation with @JanStaschulat at ROSCon / in Nav2 ros-navigation/navigation2#3974: I think it would be dandy to be able to set increase thread prioritizations for soft-realtime in the Composable Node containers. Both to be able to set at launch-time instead of setting on a per-node basis in a stack. In Nav2, there are a couple of nodes we have set able to do this recently, but it would be great if there was a generic way that users could select prioritization more broadly and easily at run-time.

This PR implements a prototype of this behavior that I would like you feedback on to resolve necessary problems seen. I added a setSoftRealTimePriority to the ComponentManager that is used to set the prioritization when necessary. By default, I have the priority set to 49 after discussions with Bosch, but we could also make this configurable. Though, there's something to be said about simplicity and the suggestions in the RT workshop had the options of HIGH and LOW, so I think this might be OK to start off with.

The way that I see this workflow occurring is the following for Isolated Component Manager:

  • Users set in the parameters field of LoadComposableNodes or from the parameters in CompositionNode use_soft_realtime_priority to its value in their launch files for the nodes of interest
  • That is communicated in the parameters field of LoadNode service to the component container and sent to override_parameters in NodeOptions
  • When we create the thread for our executor, we check the parameter overrides for this value. If it exists & is true then we set the prioritization
  • This way, we can control which nodes are added to the higher priority threads and which are not individually

Q: I believe all of the launch stuff (linked above) is setup already to be able to handle this workflow. Please I would appreciate making sure that's true since the launch_ros package is a little difficult to parse at times.

The way that I see this workflow occurring is the following for Normal Component Manager:

  • We declare use_soft_realtime_priority in the Component Manager constructor (false, for off default)
  • If true, we also set the priority
  • This way, we can control the single threaded & multithreaded containers to have thread prioritization as well for its nodes
  • Technically speaking, you could also set this to true in the Isolated case to increase the prioritization of the Isolated's load/unload/list services. While that's an odd choice, that's a perfectly valid option

I have no questions on this one, I think this is good.

@Zard-C
Copy link
Contributor

Zard-C commented Nov 22, 2023

hi @SteveMacenski, glad to see this draft, I think that sched_setscheduler api is only for linux system, but for MacOS or Windows, they may have their own apis to set thread priority. Thus another 2 platforms should be taking considered.

I think maybe we could visit the std::thread via the wrapper, and use std::thread::native_handle() to get the native thread handle(which depends on platforms), and use specific api to set thread's real-time attributes.

I am currently working on real time tuning on linux and qnx, If you have better ideas, feel free to let me know. 😸

@JanStaschulat
Copy link

hi @SteveMacenski, glad to see this draft, I think that sched_setscheduler api is only for linux system, but for MacOS or Windows, they may have their own apis to set thread priority. Thus another 2 platforms should be taking considered.

This utility function supports thread configuration for Linux, Windows Mac. A similar version could be used here as well.

@Zard-C
Copy link
Contributor

Zard-C commented Nov 22, 2023

hi @SteveMacenski, glad to see this draft, I think that sched_setscheduler api is only for linux system, but for MacOS or Windows, they may have their own apis to set thread priority. Thus another 2 platforms should be taking considered.

This utility function supports thread configuration for Linux, Windows Mac. A similar version could be used here as well.

I took a look at this function, it supports QNX as well, very impressive 👍

@SteveMacenski
Copy link
Collaborator Author

This utility function supports thread configuration for Linux, Windows Mac. A similar version could be used here as well.

Agreed for the Isolated case. How do we handle this for the normal case where this is simply called while inside of a thread, not in creating one (e.g. we call setSoftRealTimePriority in the ComponentManager constructor)?

If that function can handle that and/or be made to handle that, then happy to try to find a home for it as part of this effort.

@JanStaschulat
Copy link

This utility function supports thread configuration for Linux, Windows Mac. A similar version could be used here as well.

Agreed for the Isolated case. How do we handle this for the normal case where this is simply called while inside of a thread, not in creating one (e.g. we call setSoftRealTimePriority in the ComponentManager constructor)?

If that function can handle that and/or be made to handle that, then happy to try to find a home for it as part of this effort.

int sched_setscheduler(pid_t pid, int policy, const struct sched_param *param);
If pid equals zero, the scheduling policy and parameters of the calling thread will be set.

This is, however, not documented for pthread_setschedparam.

@SteveMacenski
Copy link
Collaborator Author

Sure, but your method takes in a T native_handle that needs to implement a number of things to a number of OS's. Can you just send that as 0, I wouldn't have thought so.

@Zard-C
Copy link
Contributor

Zard-C commented Nov 22, 2023

Sure, but your method takes in a T native_handle that needs to implement a number of things to a number of OS's. Can you just send that as 0, I wouldn't have thought so.

For windows, change
windows's `native_handler to GetCurrentThread()

For Mac, QNX, and linux,
change native_handle to pthread_self()
pthread_self(macos)
pthread_self(qnx)
pthread_self(linux)

Thus we don't need take T handler as parameter

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

Successfully merging this pull request may close these issues.

3 participants