Skip to content

os : implement os.sched_yield, os.sched_get_priority_min, os.sched_get_priority_max #2997

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

Merged
merged 2 commits into from
Sep 2, 2021

Conversation

zetwhite
Copy link
Contributor

@zetwhite zetwhite commented Sep 1, 2021

This PR contains implementation of os.sched_yield, os.sched_get_priority_min and os.sched_get_priority_max.
You can check the interface of each function here : https://docs.python.org/3/library/os.html#interface-to-the-scheduler

Inside each function, related system call is called through the libc Crate.
with this code, rust python works as below.

>>>>> import os 
>>>>> os.sched_get_priority_min(os.SCHED_RR)
1
>>>>> os.sched_get_priority_max(os.SCHED_RR)
99
>>>>> os.sched_get_priority_min(os.SCHED_OTHER)
0
>>>>> os.sched_get_priority_max(os.SCHED_OTHER)
0
>>>>> os.sched_yield()

@zetwhite zetwhite force-pushed the os_sched branch 2 times, most recently from c90cd0f to a0624dd Compare September 1, 2021 11:39
@youknowone
Copy link
Member

youknowone commented Sep 1, 2021

Is this unix api or linux api? if it is unix apis, we don't need linux check.
CPython supports them in macos

>>> import os
>>> os.sched_yield()
>>> os.sched_get_priority_min(10)
15

Comment on lines 2400 to 2404
let res = nix::sched::sched_yield();
match res {
Ok(_) => Ok(()),
Err(err) => Err(err.into_pyexception(vm)),
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let res = nix::sched::sched_yield();
match res {
Ok(_) => Ok(()),
Err(err) => Err(err.into_pyexception(vm)),
}
let _ = nix::sched::sched_yield().map_err(|e| e.into_pyexception(vm))?;
Ok(())

@zetwhite zetwhite force-pushed the os_sched branch 2 times, most recently from fb86bd9 to 3a70a6f Compare September 1, 2021 14:44
@youknowone
Copy link
Member

youknowone commented Sep 1, 2021

huh, I have no idea why macos build fails on CI. Let's go back to linux only for now. I am sorry. 😅

I think SCHED_OTHER needs to be marked as some platform-only to fit exotic targets.
Not sure why other attributes fails.

I found I didn't remove target cfg from pyattr. The functions work. But pyattrs will not be simple.

@zetwhite
Copy link
Contributor Author

zetwhite commented Sep 1, 2021

Okay. I understood.
I will check how it works on other platforms through the libc documentation(https://rust-lang.github.io/libc/#platform-specific-documentation) and fix it! Thank you

@zetwhite
Copy link
Contributor Author

zetwhite commented Sep 2, 2021

About sched_get_priority_min , sched_get_priority_max, as you can see, i just added not(target_os = redox).

About scheduling policies such as SCHED_RR, SCHED_FIFO, that can be much different depending on the OS.
In CPython, ifdef is used as below, but I have no idea how to do that in Rust.
So, I just added each OS in #[cfg(any(~)] to make it works based on whitelist.

#ifdef SCHED_OTHER
    if (PyModule_AddIntMacro(m, SCHED_OTHER)) return -1;
#endif
#ifdef SCHED_FIFO
    if (PyModule_AddIntMacro(m, SCHED_FIFO)) return -1;
#endif
#ifdef SCHED_RR
    if (PyModule_AddIntMacro(m, SCHED_RR)) return -1;
#endif

https://github.com/python/cpython/blob/main/Modules/posixmodule.c#L15267-L15300

@youknowone
Copy link
Member

SCHED_RR in macos is defined in <pthread.h> but not in libc. I created a patch about it (rust-lang/libc#2384) but it will take time.

@youknowone youknowone merged commit 61e79ff into RustPython:main Sep 2, 2021
@youknowone youknowone added the z-ca-2021 Tag to track contrubution-academy 2021 label Oct 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
z-ca-2021 Tag to track contrubution-academy 2021
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants