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

set_current_thread_priority(get_current_thread_priority) seems to fail #25

Closed
matteomonti opened this issue Oct 25, 2022 · 5 comments
Closed

Comments

@matteomonti
Copy link

matteomonti commented Oct 25, 2022

I've decided to try out thread_priority for my project and started by trying out the most simple example:

let current = thread_priority::get_current_thread_priority().unwrap();
println!("Current thread priority is: {current:?}");
thread_priority::set_current_thread_priority(current).unwrap();

Sadly, the example panics with an Error::PriorityNotInRange(1..=99) (current is logged to be ThreadPriority::CrossPlatform(0)). I am wondering if ThreadPriority::CrossPlatform(0) stands for something like "cannot retrieve priority"? I would naively imagine the default priority of a thread to hover somewhere around 50, rather than at (actually, below) the lowest possible value.

@iddm
Copy link
Owner

iddm commented Oct 25, 2022

Hi @matteomonti! Thank you for opening an issue here. To better help you with that issue, can you please specify what platform did you run your example on?

@matteomonti
Copy link
Author

Hello there @vityafx :)
Thank you so much for the prompt answer!

I am running on a 64-bit server, x86 architecture, Intel Xeon processor, running Ubuntu 20.04.5 LTS (Focal Fossa).

If you have any other question, I am at your disposal!

@iddm
Copy link
Owner

iddm commented Oct 26, 2022

@matteomonti I have found the reason for this problem.

Let me explain it here so that I don't forget this myself, and, perhaps, it might be of interest for you as well. This has appeared quite recently after the changes I made for the realtime threads support and niceness. So this actually comes from a problem of unifying different APIs from windows and various unixes/linuxes. The get_current_thread_priority returns just the priority value (0) which is valid in this case - we have just created a thread and by default it has some default priority, which, in this case, is zero (niceness) for a non-realtime thread. But when we try to set this value again - it fails because the code assumes nobody will ever call the "set_current_thread_priority" function with a value of "0" is it doesn't make sense; with this assumption, it would attempt to also set the thread's policy to a real-time one, where the "zero" value is not evaluated properly. All of this logic is a workaround. Perhaps, I could do a better job at APIs unification.
See, in case of unix, threads actually have two priorities, not just one: dynamic and a static one. One is used when there is one of a few thread priority policies used, and another - with other thread priority policies. For example, for SCHED_NORMAL and SCHED_BATCH we should use niceness, while for SCHED_FIFO and SCHED_RR - pthread's priority. There is even a SCHED_DEADLINE policy, for which we should use completely different set of values. So it is quite hard to map a single library interface (function) for a priority to all of these correctly and I had to come up with some compromises.

I have an idea how to solve this for such a simple case: just get the current thread policy and map the Crossplatform value accordingly. For some reason I didn't do this, or I just forgot. Thank you for rising this issue again, I'll try to solve it in the near future, can't promise today or tomorrow, but I am almost certain - until the end of the next week.

P.S. All the text in italics is an incorrect code made on a wrong assumption, perhaps, before I made use of niceness.

@iddm
Copy link
Owner

iddm commented Oct 26, 2022

I have made some changes there, now it should be accounting for the current thread's scheduling policy. This way you should be able to reset the same thread priority.

@iddm
Copy link
Owner

iddm commented Oct 28, 2022

@matteomonti Can you please share when you will be able to verify the fix works for you?

@iddm iddm closed this as completed Mar 4, 2023
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