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.

Sign up for GitHub

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

Jump to bottom

Add support for nogil Python #2885

Closed
wants to merge 4 commits into from
Closed

Add support for nogil Python #2885

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e9b9f22
Add support for nogil Python
colesbury Apr 21, 2022
b390838
Use sys.implementation.name instead of SOABI.
colesbury Jan 18, 2023
800c20d
Use assert!
colesbury Jan 18, 2023
17b4c15
Avoid unsafe borrowed references when using Py_NOGIL
colesbury Jan 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions pyo3-build-config/src/impl_.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ impl InterpreterConfig {
println!("cargo:rustc-cfg=Py_LIMITED_API");
}

if self.implementation == PythonImplementation::NoGIL {
println!("cargo:rustc-cfg=Py_NOGIL");
}

for flag in &self.build_flags.0 {
println!("cargo:rustc-cfg=py_sys_config=\"{}\"", flag);
}
Expand Down Expand Up @@ -220,7 +224,11 @@ FRAMEWORK = bool(get_config_var("PYTHONFRAMEWORK"))
# unix-style shared library enabled
SHARED = bool(get_config_var("Py_ENABLE_SHARED"))

print("implementation", platform.python_implementation())
implementation = platform.python_implementation()
if sys.implementation.name == "nogil":
implementation = "NoGIL"

print("implementation", implementation)
print("version_major", sys.version_info[0])
print("version_minor", sys.version_info[1])
print("shared", PYPY or ANACONDA or WINDOWS or FRAMEWORK or SHARED)
Expand Down Expand Up @@ -639,6 +647,7 @@ impl FromStr for PythonVersion {
pub enum PythonImplementation {
CPython,
PyPy,
NoGIL,
}

impl PythonImplementation {
Expand All @@ -653,6 +662,8 @@ impl PythonImplementation {
Ok(PythonImplementation::PyPy)
} else if soabi.starts_with("cpython") {
Ok(PythonImplementation::CPython)
} else if soabi.starts_with("nogil") {
Ok(PythonImplementation::NoGIL)
} else {
bail!("unsupported Python interpreter");
}
Expand All @@ -664,6 +675,7 @@ impl Display for PythonImplementation {
match self {
PythonImplementation::CPython => write!(f, "CPython"),
PythonImplementation::PyPy => write!(f, "PyPy"),
PythonImplementation::NoGIL => write!(f, "NoGIL"),
}
}
}
Expand All @@ -674,6 +686,7 @@ impl FromStr for PythonImplementation {
match s {
"CPython" => Ok(PythonImplementation::CPython),
"PyPy" => Ok(PythonImplementation::PyPy),
"NoGIL" => Ok(PythonImplementation::NoGIL),
_ => bail!("unknown interpreter: {}", s),
}
}
Expand Down Expand Up @@ -1523,7 +1536,7 @@ fn default_lib_name_unix(
ld_version: Option<&str>,
) -> String {
match implementation {
PythonImplementation::CPython => match ld_version {
PythonImplementation::CPython | PythonImplementation::NoGIL => match ld_version {
Some(ld_version) => format!("python{}", ld_version),
None => {
if version > PythonVersion::PY37 {
Expand Down
56 changes: 44 additions & 12 deletions pyo3-ffi/src/object.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ pub const PyObject_HEAD_INIT: PyObject = PyObject {
_ob_next: std::ptr::null_mut(),
#[cfg(py_sys_config = "Py_TRACE_REFS")]
_ob_prev: std::ptr::null_mut(),
#[cfg(Py_NOGIL)]
ob_tid: 0,
#[cfg(Py_NOGIL)]
ob_reflocal: 1,
#[cfg(Py_NOGIL)]
ob_ref_shared: 0,
#[cfg(not(Py_NOGIL))]
ob_refcnt: 1,
#[cfg(PyPy)]
ob_pypy_link: 0,
Expand All @@ -35,6 +42,13 @@ pub struct PyObject {
pub _ob_next: *mut PyObject,
#[cfg(py_sys_config = "Py_TRACE_REFS")]
pub _ob_prev: *mut PyObject,
#[cfg(Py_NOGIL)]
pub ob_tid: usize,
#[cfg(Py_NOGIL)]
pub ob_reflocal: u32,
#[cfg(Py_NOGIL)]
pub ob_ref_shared: u32,
#[cfg(not(Py_NOGIL))]
pub ob_refcnt: Py_ssize_t,
#[cfg(PyPy)]
pub ob_pypy_link: Py_ssize_t,
Expand Down Expand Up @@ -62,11 +76,21 @@ pub unsafe fn Py_Is(x: *mut PyObject, y: *mut PyObject) -> c_int {
// skipped _Py_REFCNT: defined in Py_REFCNT

#[inline]
#[cfg(not(Py_NOGIL))]
pub unsafe fn Py_REFCNT(ob: *mut PyObject) -> Py_ssize_t {
assert!(!ob.is_null());
(*ob).ob_refcnt
}

#[inline]
#[cfg(Py_NOGIL)]
pub unsafe fn Py_REFCNT(ob: *mut PyObject) -> Py_ssize_t {
if ob.is_null() {
panic!();
colesbury marked this conversation as resolved.
Show resolved Hide resolved
}
Py_RefCnt(ob)
}

#[inline]
pub unsafe fn Py_TYPE(ob: *mut PyObject) -> *mut PyTypeObject {
(*ob).ob_type
Expand Down Expand Up @@ -409,26 +433,32 @@ extern "C" {

// Reference counting macros.
#[inline]
#[cfg(not(any(py_sys_config = "Py_REF_DEBUG", Py_NOGIL)))]
pub unsafe fn Py_INCREF(op: *mut PyObject) {
if cfg!(py_sys_config = "Py_REF_DEBUG") {
Py_IncRef(op)
} else {
(*op).ob_refcnt += 1
}
(*op).ob_refcnt += 1
}

#[inline]
#[cfg(any(py_sys_config = "Py_REF_DEBUG", Py_NOGIL))]
pub unsafe fn Py_INCREF(op: *mut PyObject) {
Py_IncRef(op)
Copy link
Member

Choose a reason for hiding this comment

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

@colesbury - I understood that for the CPython stable API there was a discussion about making refcounting details internal to CPython but it was deemed infeasible due to performance regression.

Here this patch seems to do exactly that. Are you able to comment on the estimated performance impact for extensions by changing refcounting to be through an FFI call?

Copy link
Author

Choose a reason for hiding this comment

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

I think I have heard something similar, but I can't find the original source and don't know for which extensions it was deemed too great a cost.

There didn't seem to be an noticeable performance impact for the cryptography extension, but I don't know enough to make any general estimates.

}

#[inline]
#[cfg(not(any(py_sys_config = "Py_REF_DEBUG", Py_NOGIL)))]
pub unsafe fn Py_DECREF(op: *mut PyObject) {
if cfg!(py_sys_config = "Py_REF_DEBUG") {
Py_DecRef(op)
} else {
(*op).ob_refcnt -= 1;
if (*op).ob_refcnt == 0 {
_Py_Dealloc(op)
}
(*op).ob_refcnt -= 1;
if (*op).ob_refcnt == 0 {
_Py_Dealloc(op)
}
}

#[inline]
#[cfg(any(py_sys_config = "Py_REF_DEBUG", Py_NOGIL))]
pub unsafe fn Py_DECREF(op: *mut PyObject) {
Py_DecRef(op)
}

#[inline]
pub unsafe fn Py_CLEAR(op: *mut *mut PyObject) {
let tmp = *op;
Expand Down Expand Up @@ -457,6 +487,8 @@ extern "C" {
pub fn Py_IncRef(o: *mut PyObject);
#[cfg_attr(PyPy, link_name = "PyPy_DecRef")]
pub fn Py_DecRef(o: *mut PyObject);
#[cfg(Py_NOGIL)]
pub fn Py_RefCnt(o: *mut PyObject) -> Py_ssize_t;

#[cfg(Py_3_10)]
pub fn Py_NewRef(obj: *mut PyObject) -> *mut PyObject;
Expand Down