From e9b9f226a6c5687020804bf765c5d6118dc374af Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 20 Apr 2022 17:33:21 -0700 Subject: [PATCH 1/4] Add support for nogil Python --- pyo3-build-config/src/impl_.rs | 17 +++++++++-- pyo3-ffi/src/object.rs | 56 ++++++++++++++++++++++++++-------- 2 files changed, 59 insertions(+), 14 deletions(-) diff --git a/pyo3-build-config/src/impl_.rs b/pyo3-build-config/src/impl_.rs index 2f0f5afda13..db03c7e8daf 100644 --- a/pyo3-build-config/src/impl_.rs +++ b/pyo3-build-config/src/impl_.rs @@ -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); } @@ -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 get_config_var("SOABI").startswith("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) @@ -639,6 +647,7 @@ impl FromStr for PythonVersion { pub enum PythonImplementation { CPython, PyPy, + NoGIL, } impl PythonImplementation { @@ -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"); } @@ -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"), } } } @@ -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), } } @@ -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 { diff --git a/pyo3-ffi/src/object.rs b/pyo3-ffi/src/object.rs index 7fabcdf6f60..1e757df2fa6 100644 --- a/pyo3-ffi/src/object.rs +++ b/pyo3-ffi/src/object.rs @@ -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, @@ -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, @@ -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!(); + } + Py_RefCnt(ob) +} + #[inline] pub unsafe fn Py_TYPE(ob: *mut PyObject) -> *mut PyTypeObject { (*ob).ob_type @@ -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) } #[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; @@ -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; From b39083831f2c97b593a532121cc1ffa797806ed1 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Tue, 17 Jan 2023 18:35:46 -0800 Subject: [PATCH 2/4] Use sys.implementation.name instead of SOABI. Windows does not have the SOABI sysconfig variable set. --- pyo3-build-config/src/impl_.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyo3-build-config/src/impl_.rs b/pyo3-build-config/src/impl_.rs index db03c7e8daf..6cd1546bd59 100644 --- a/pyo3-build-config/src/impl_.rs +++ b/pyo3-build-config/src/impl_.rs @@ -225,7 +225,7 @@ FRAMEWORK = bool(get_config_var("PYTHONFRAMEWORK")) SHARED = bool(get_config_var("Py_ENABLE_SHARED")) implementation = platform.python_implementation() -if get_config_var("SOABI").startswith("nogil"): +if sys.implementation.name == "nogil": implementation = "NoGIL" print("implementation", implementation) From 800c20d13251253d1a30b8e931fa5ebcb45d1ab0 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 18 Jan 2023 10:45:41 -0800 Subject: [PATCH 3/4] Use assert! --- pyo3-ffi/src/object.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pyo3-ffi/src/object.rs b/pyo3-ffi/src/object.rs index 1e757df2fa6..93e2693cdaa 100644 --- a/pyo3-ffi/src/object.rs +++ b/pyo3-ffi/src/object.rs @@ -85,9 +85,7 @@ pub unsafe fn Py_REFCNT(ob: *mut PyObject) -> Py_ssize_t { #[inline] #[cfg(Py_NOGIL)] pub unsafe fn Py_REFCNT(ob: *mut PyObject) -> Py_ssize_t { - if ob.is_null() { - panic!(); - } + assert!(!ob.is_null()); Py_RefCnt(ob) } From 17b4c158f0ac6e44ba617a7602794b8f50d2952d Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 18 Jan 2023 12:08:05 -0800 Subject: [PATCH 4/4] Avoid unsafe borrowed references when using Py_NOGIL --- pyo3-ffi/src/dictobject.rs | 29 +++++++++++++++++++++++++++++ pyo3-ffi/src/listobject.rs | 14 ++++++++++++++ src/types/dict.rs | 8 ++++---- src/types/list.rs | 7 +++---- 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/pyo3-ffi/src/dictobject.rs b/pyo3-ffi/src/dictobject.rs index b03fbb303a8..7dfe321fb6a 100644 --- a/pyo3-ffi/src/dictobject.rs +++ b/pyo3-ffi/src/dictobject.rs @@ -24,6 +24,8 @@ extern "C" { #[cfg_attr(PyPy, link_name = "PyPyDict_GetItem")] pub fn PyDict_GetItem(mp: *mut PyObject, key: *mut PyObject) -> *mut PyObject; pub fn PyDict_GetItemWithError(mp: *mut PyObject, key: *mut PyObject) -> *mut PyObject; + #[cfg_attr(Py_NOGIL, link_name = "PyDict_GetItemWithError2")] + pub fn _PyDict_FetchItemWithError(mp: *mut PyObject, key: *mut PyObject) -> *mut PyObject; #[cfg_attr(PyPy, link_name = "PyPyDict_SetItem")] pub fn PyDict_SetItem(mp: *mut PyObject, key: *mut PyObject, item: *mut PyObject) -> c_int; #[cfg_attr(PyPy, link_name = "PyPyDict_DelItem")] @@ -67,6 +69,33 @@ extern "C" { // skipped 3.10 / ex-non-limited PyObject_GenericGetDict } +#[inline] +#[cfg(not(Py_NOGIL))] +pub unsafe fn PyDict_FetchItem(mp: *mut PyObject, key: *mut PyObject) -> *mut PyObject { + _Py_XNewRef(PyDict_GetItem(mp, key)) +} + +#[inline] +#[cfg(Py_NOGIL)] +pub unsafe fn PyDict_FetchItem(mp: *mut PyObject, key: *mut PyObject) -> *mut PyObject { + use crate::PyErr_Clear; + let obj = _PyDict_FetchItemWithError(mp, key); + PyErr_Clear(); + obj +} + +#[inline] +#[cfg(not(Py_NOGIL))] +pub unsafe fn PyDict_FetchItemWithError(mp: *mut PyObject, key: *mut PyObject) -> *mut PyObject { + _Py_XNewRef(PyDict_GetItemWithError(mp, key)) +} + +#[inline] +#[cfg(Py_NOGIL)] +pub unsafe fn PyDict_FetchItemWithError(mp: *mut PyObject, key: *mut PyObject) -> *mut PyObject { + _PyDict_FetchItemWithError(mp, key) +} + #[cfg_attr(windows, link(name = "pythonXY"))] extern "C" { pub static mut PyDictKeys_Type: PyTypeObject; diff --git a/pyo3-ffi/src/listobject.rs b/pyo3-ffi/src/listobject.rs index 9b10a9174d0..5f710ef7bd8 100644 --- a/pyo3-ffi/src/listobject.rs +++ b/pyo3-ffi/src/listobject.rs @@ -27,6 +27,8 @@ extern "C" { pub fn PyList_Size(arg1: *mut PyObject) -> Py_ssize_t; #[cfg_attr(PyPy, link_name = "PyPyList_GetItem")] pub fn PyList_GetItem(arg1: *mut PyObject, arg2: Py_ssize_t) -> *mut PyObject; + #[cfg_attr(Py_NOGIL, link_name = "PyList_Item")] + pub fn _PyList_FetchItem(arg1: *mut PyObject, arg2: Py_ssize_t) -> *mut PyObject; #[cfg_attr(PyPy, link_name = "PyPyList_SetItem")] pub fn PyList_SetItem(arg1: *mut PyObject, arg2: Py_ssize_t, arg3: *mut PyObject) -> c_int; #[cfg_attr(PyPy, link_name = "PyPyList_Insert")] @@ -64,3 +66,15 @@ extern "C" { #[cfg_attr(PyPy, link_name = "PyPyList_SET_ITEM")] pub fn PyList_SET_ITEM(arg1: *mut PyObject, arg2: Py_ssize_t, arg3: *mut PyObject); } + +#[inline] +#[cfg(not(Py_NOGIL))] +pub unsafe fn PyList_FetchItem(list: *mut PyObject, index: Py_ssize_t) -> *mut PyObject { + _Py_XNewRef(PyList_GetItem(list, index)) +} + +#[inline] +#[cfg(Py_NOGIL)] +pub unsafe fn PyList_FetchItem(list: *mut PyObject, index: Py_ssize_t) -> *mut PyObject { + _PyList_FetchItem(list, index) +} diff --git a/src/types/dict.rs b/src/types/dict.rs index 35867ed1eb8..f73ff50c54b 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -146,10 +146,10 @@ impl PyDict { K: ToPyObject, { unsafe { - let ptr = ffi::PyDict_GetItem(self.as_ptr(), key.to_object(self.py()).as_ptr()); + let ptr = ffi::PyDict_FetchItem(self.as_ptr(), key.to_object(self.py()).as_ptr()); NonNull::new(ptr).map(|p| { // PyDict_GetItem return s borrowed ptr, must make it owned for safety (see #890). - self.py().from_owned_ptr(ffi::_Py_NewRef(p.as_ptr())) + self.py().from_owned_ptr(p.as_ptr()) }) } } @@ -166,12 +166,12 @@ impl PyDict { { unsafe { let ptr = - ffi::PyDict_GetItemWithError(self.as_ptr(), key.to_object(self.py()).as_ptr()); + ffi::PyDict_FetchItemWithError(self.as_ptr(), key.to_object(self.py()).as_ptr()); if !ffi::PyErr_Occurred().is_null() { return Err(PyErr::fetch(self.py())); } - Ok(NonNull::new(ptr).map(|p| self.py().from_owned_ptr(ffi::_Py_NewRef(p.as_ptr())))) + Ok(NonNull::new(ptr).map(|p| self.py().from_owned_ptr(p.as_ptr()))) } } diff --git a/src/types/list.rs b/src/types/list.rs index 2ef7bca8f36..5bec22182b7 100644 --- a/src/types/list.rs +++ b/src/types/list.rs @@ -130,9 +130,7 @@ impl PyList { /// ``` pub fn get_item(&self, index: usize) -> PyResult<&PyAny> { unsafe { - let item = ffi::PyList_GetItem(self.as_ptr(), index as Py_ssize_t); - // PyList_GetItem return borrowed ptr; must make owned for safety (see #890). - ffi::Py_XINCREF(item); + let item = ffi::PyList_FetchItem(self.as_ptr(), index as Py_ssize_t); self.py().from_owned_ptr_or_err(item) } } @@ -141,7 +139,8 @@ impl PyList { /// /// # Safety /// - /// Caller must verify that the index is within the bounds of the list. + /// Caller must verify that the index is within the bounds of the list and that no other + /// thread is concurrently modifying the list. #[cfg(not(Py_LIMITED_API))] pub unsafe fn get_item_unchecked(&self, index: usize) -> &PyAny { let item = ffi::PyList_GET_ITEM(self.as_ptr(), index as Py_ssize_t);