From 5be5d775899122fbbeb4037631b2dbf0ab88136f Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Thu, 23 Dec 2021 21:17:33 +0000 Subject: [PATCH] opt: improve handle_panic generated code --- CHANGELOG.md | 1 + src/callback.rs | 11 +++-------- src/gil.rs | 10 +++++++--- src/impl_.rs | 1 + src/impl_/not_send.rs | 9 +++++++++ src/internal_tricks.rs | 6 ------ src/panic.rs | 1 + src/python.rs | 3 ++- tests/test_compile_error.rs | 1 + tests/ui/not_send.rs | 11 +++++++++++ tests/ui/not_send.stderr | 14 ++++++++++++++ 11 files changed, 50 insertions(+), 18 deletions(-) create mode 100644 src/impl_/not_send.rs create mode 100644 tests/ui/not_send.rs create mode 100644 tests/ui/not_send.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index ae6edaf7..feb477f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The `create_exception!` macro can now take an optional docstring. This docstring, if supplied, is visible to users (with `.__doc__` and `help()`) and accompanies your error type in your crate's documentation. - Improve performance and error messages for `#[derive(FromPyObject)]` for enums. [#2068](https://github.com/PyO3/pyo3/pull/2068) +- Tweak LLVM code for compile times for internal `handle_panic` helper. [#2073](https://github.com/PyO3/pyo3/pull/2073) ### Removed diff --git a/src/callback.rs b/src/callback.rs index e29f07e0..4a8598f3 100644 --- a/src/callback.rs +++ b/src/callback.rs @@ -10,7 +10,7 @@ use crate::{GILPool, IntoPyPointer}; use crate::{IntoPy, PyObject, Python}; use std::any::Any; use std::os::raw::c_int; -use std::panic::{AssertUnwindSafe, UnwindSafe}; +use std::panic::UnwindSafe; use std::{isize, panic}; /// A type which can be the return type of a python C-API callback @@ -241,13 +241,8 @@ where R: PyCallbackOutput, { let pool = GILPool::new(); - let unwind_safe_py = AssertUnwindSafe(pool.python()); - let panic_result = panic::catch_unwind(move || -> PyResult<_> { - let py = *unwind_safe_py; - body(py) - }); - - panic_result_into_callback_output(pool.python(), panic_result) + let py = pool.python(); + panic_result_into_callback_output(py, panic::catch_unwind(move || -> PyResult<_> { body(py) })) } fn panic_result_into_callback_output( diff --git a/src/gil.rs b/src/gil.rs index 3de82040..1c0fa03c 100644 --- a/src/gil.rs +++ b/src/gil.rs @@ -2,7 +2,8 @@ //! Interaction with Python's global interpreter lock -use crate::{ffi, internal_tricks::Unsendable, Python}; +use crate::impl_::not_send::{NotSend, NOT_SEND}; +use crate::{ffi, Python}; use parking_lot::{const_mutex, Mutex, Once}; use std::cell::{Cell, RefCell}; use std::{ @@ -157,6 +158,7 @@ where pub struct GILGuard { gstate: ffi::PyGILState_STATE, pool: ManuallyDrop>, + _not_send: NotSend, } impl GILGuard { @@ -230,6 +232,7 @@ impl GILGuard { GILGuard { gstate, pool: ManuallyDrop::new(pool), + _not_send: NOT_SEND, } } } @@ -332,7 +335,7 @@ pub struct GILPool { /// Initial length of owned objects and anys. /// `Option` is used since TSL can be broken when `new` is called from `atexit`. start: Option, - no_send: Unsendable, + _not_send: NotSend, } impl GILPool { @@ -351,11 +354,12 @@ impl GILPool { POOL.update_counts(Python::assume_gil_acquired()); GILPool { start: OWNED_OBJECTS.try_with(|o| o.borrow().len()).ok(), - no_send: Unsendable::default(), + _not_send: NOT_SEND, } } /// Gets the Python token associated with this [`GILPool`]. + #[inline] pub fn python(&self) -> Python { unsafe { Python::assume_gil_acquired() } } diff --git a/src/impl_.rs b/src/impl_.rs index 31826afb..586ce1a5 100644 --- a/src/impl_.rs +++ b/src/impl_.rs @@ -8,3 +8,4 @@ pub mod deprecations; pub mod freelist; #[doc(hidden)] pub mod frompyobject; +pub(crate) mod not_send; diff --git a/src/impl_/not_send.rs b/src/impl_/not_send.rs new file mode 100644 index 00000000..fca8a96c --- /dev/null +++ b/src/impl_/not_send.rs @@ -0,0 +1,9 @@ +use std::marker::PhantomData; + +use crate::Python; + +/// A marker type that makes the type !Send. +/// Workaround for lack of !Send on stable (https://github.com/rust-lang/rust/issues/68318). +pub(crate) struct NotSend(PhantomData<*mut Python<'static>>); + +pub(crate) const NOT_SEND: NotSend = NotSend(PhantomData); diff --git a/src/internal_tricks.rs b/src/internal_tricks.rs index 307f1bfc..06422871 100644 --- a/src/internal_tricks.rs +++ b/src/internal_tricks.rs @@ -1,11 +1,5 @@ use crate::ffi::{Py_ssize_t, PY_SSIZE_T_MAX}; use std::ffi::{CStr, CString}; -use std::marker::PhantomData; -use std::rc::Rc; - -/// A marker type that makes the type !Send. -/// Temporal hack until https://github.com/rust-lang/rust/issues/13231 is resolved. -pub(crate) type Unsendable = PhantomData>; pub struct PrivateMarker; diff --git a/src/panic.rs b/src/panic.rs index 0fe6c0bf..4fe6100d 100644 --- a/src/panic.rs +++ b/src/panic.rs @@ -17,6 +17,7 @@ Python interpreter to exit. impl PanicException { // Try to format the error in the same way panic does + #[cold] pub(crate) fn from_panic_payload(payload: Box) -> PyErr { if let Some(string) = payload.downcast_ref::() { Self::new_err((string.clone(),)) diff --git a/src/python.rs b/src/python.rs index 014e866e..c410d0d1 100644 --- a/src/python.rs +++ b/src/python.rs @@ -4,6 +4,7 @@ use crate::err::{self, PyDowncastError, PyErr, PyResult}; use crate::gil::{self, GILGuard, GILPool}; +use crate::impl_::not_send::NotSend; use crate::type_object::{PyTypeInfo, PyTypeObject}; use crate::types::{PyAny, PyDict, PyModule, PyType}; use crate::{ffi, AsPyPointer, FromPyPointer, IntoPyPointer, PyNativeType, PyObject, PyTryFrom}; @@ -184,7 +185,7 @@ impl PartialOrd<(u8, u8, u8)> for PythonVersionInfo<'_> { /// [`Py::clone_ref`]: crate::Py::clone_ref /// [Memory Management]: https://pyo3.rs/main/memory.html#gil-bound-memory #[derive(Copy, Clone)] -pub struct Python<'py>(PhantomData<&'py GILGuard>); +pub struct Python<'py>(PhantomData<(&'py GILGuard, NotSend)>); impl Python<'_> { /// Acquires the global interpreter lock, allowing access to the Python interpreter. The diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs index ce9288fa..daade52d 100644 --- a/tests/test_compile_error.rs +++ b/tests/test_compile_error.rs @@ -29,6 +29,7 @@ fn _test_compile_errors() { t.compile_fail("tests/ui/invalid_pymodule_args.rs"); t.compile_fail("tests/ui/missing_clone.rs"); t.compile_fail("tests/ui/reject_generics.rs"); + t.compile_fail("tests/ui/not_send.rs"); tests_rust_1_49(&t); tests_rust_1_55(&t); diff --git a/tests/ui/not_send.rs b/tests/ui/not_send.rs new file mode 100644 index 00000000..d7c47dec --- /dev/null +++ b/tests/ui/not_send.rs @@ -0,0 +1,11 @@ +use pyo3::prelude::*; + +fn test_not_send_allow_threads(py: Python) { + py.allow_threads(|| { drop(py); }); +} + +fn main() { + Python::with_gil(|py| { + test_not_send_allow_threads(py); + }) +} diff --git a/tests/ui/not_send.stderr b/tests/ui/not_send.stderr new file mode 100644 index 00000000..31340cd7 --- /dev/null +++ b/tests/ui/not_send.stderr @@ -0,0 +1,14 @@ +error[E0277]: `*mut pyo3::Python<'static>` cannot be shared between threads safely + --> tests/ui/not_send.rs:4:8 + | +4 | py.allow_threads(|| { drop(py); }); + | ^^^^^^^^^^^^^ `*mut pyo3::Python<'static>` cannot be shared between threads safely + | + = help: within `pyo3::Python<'_>`, the trait `Sync` is not implemented for `*mut pyo3::Python<'static>` + = note: required because it appears within the type `PhantomData<*mut pyo3::Python<'static>>` + = note: required because it appears within the type `pyo3::impl_::not_send::NotSend` + = note: required because it appears within the type `(&GILGuard, pyo3::impl_::not_send::NotSend)` + = note: required because it appears within the type `PhantomData<(&GILGuard, pyo3::impl_::not_send::NotSend)>` + = note: required because it appears within the type `pyo3::Python<'_>` + = note: required because of the requirements on the impl of `Send` for `&pyo3::Python<'_>` + = note: required because it appears within the type `[closure@$DIR/tests/ui/not_send.rs:4:22: 4:38]`