Improve compilation times for projects using PyO3

CHANGELOG.md

@@ -42,6 +42,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
   - `PyNumberProtocol::__complex__` and `PyNumberProtocol::__round__`
   - `PyAsyncProtocol::__aenter__` and `PyAsyncProtocol::__aexit__`
 - Deprecate `#[name = "..."]` attributes in favor of `#[pyo3(name = "...")]`. [#1567](https://github.com/PyO3/pyo3/pull/1567)
+- Improve compilation times for projects using PyO3 [#1604](https://github.com/PyO3/pyo3/pull/1604)
 
 ### Removed
 - Remove deprecated exception names `BaseException` etc. [#1426](https://github.com/PyO3/pyo3/pull/1426)

guide/src/class.md

@@ -757,7 +757,7 @@ impl pyo3::class::impl_::PyClassImpl for MyClass {
     type BaseType = PyAny;
     type ThreadChecker = pyo3::class::impl_::ThreadCheckerStub<MyClass>;
 
-    fn for_each_method_def(visitor: impl FnMut(&pyo3::class::PyMethodDefType)) {
+    fn for_each_method_def(visitor: &mut dyn FnMut(&pyo3::class::PyMethodDefType)) {
         use pyo3::class::impl_::*;
         let collector = PyClassImplCollector::<MyClass>::new();
         collector.py_methods().iter()
@@ -780,7 +780,7 @@ impl pyo3::class::impl_::PyClassImpl for MyClass {
         let collector = PyClassImplCollector::<Self>::new();
         collector.call_impl()
     }
-    fn for_each_proto_slot(visitor: impl FnMut(&pyo3::ffi::PyType_Slot)) {
+    fn for_each_proto_slot(visitor: &mut dyn FnMut(&pyo3::ffi::PyType_Slot)) {
         // Implementation which uses dtolnay specialization to load all slots.
         use pyo3::class::impl_::*;
         let collector = PyClassImplCollector::<Self>::new();

pyo3-macros-backend/src/pyclass.rs

@@ -436,7 +436,7 @@ fn impl_class(
             type BaseType = #base;
             type ThreadChecker = #thread_checker;
 
-            fn for_each_method_def(visitor: impl FnMut(&pyo3::class::PyMethodDefType)) {
+            fn for_each_method_def(visitor: &mut dyn FnMut(&pyo3::class::PyMethodDefType)) {
                 use pyo3::class::impl_::*;
                 let collector = PyClassImplCollector::<Self>::new();
                 #iter_py_methods
@@ -460,7 +460,7 @@ fn impl_class(
                 collector.call_impl()
             }
 
-            fn for_each_proto_slot(visitor: impl FnMut(&pyo3::ffi::PyType_Slot)) {
+            fn for_each_proto_slot(visitor: &mut dyn FnMut(&pyo3::ffi::PyType_Slot)) {
                 // Implementation which uses dtolnay specialization to load all slots.
                 use pyo3::class::impl_::*;
                 let collector = PyClassImplCollector::<Self>::new();

pyo3-macros-backend/src/pymethod.rs

@@ -487,19 +487,28 @@ fn impl_arg_param(
     let arg_value = quote_arg_span!(#args_array[#option_pos]);
     *option_pos += 1;
 
-    let default = match (spec.default_value(name), arg.optional.is_some()) {
-        (Some(default), true) if default.to_string() != "None" => {
-            quote_arg_span! { Some(#default) }
-        }
-        (Some(default), _) => quote_arg_span! { #default },
-        (None, true) => quote_arg_span! { None },
-        (None, false) => quote_arg_span! { panic!("Failed to extract required method argument") },
-    };
-
     let extract = if let Some(FromPyWithAttribute(expr_path)) = &arg.attrs.from_py_with {
-        quote_arg_span! { #expr_path(_obj).map_err(#transform_error)?}
+        quote_arg_span! { #expr_path(_obj).map_err(#transform_error) }
     } else {
-        quote_arg_span! { _obj.extract().map_err(#transform_error)?}
+        quote_arg_span! { _obj.extract().map_err(#transform_error) }
+    };
+
+    let arg_value_or_default = match (spec.default_value(name), arg.optional.is_some()) {
+        (Some(default), true) if default.to_string() != "None" => {
+            quote_arg_span! { #arg_value.map_or_else(|| Ok(Some(#default)), |_obj| #extract)? }
+        }
+        (Some(default), _) => {
+            quote_arg_span! { #arg_value.map_or_else(|| Ok(#default), |_obj| #extract)? }
+        }
+        (None, true) => quote_arg_span! { #arg_value.map_or(Ok(None), |_obj| #extract)? },
+        (None, false) => {
+            quote_arg_span! {
+                {
+                    let _obj = #arg_value.expect("Failed to extract required method argument");
+                    #extract?
+                }
+            }
+        }
     };
 
     return if let syn::Type::Reference(tref) = arg.optional.as_ref().unwrap_or(&ty) {
@@ -523,18 +532,12 @@ fn impl_arg_param(
         };
 
         Ok(quote_arg_span! {
-            let #mut_ _tmp: #target_ty = match #arg_value {
-                Some(_obj) => #extract,
-                None => #default,
-            };
+            let #mut_ _tmp: #target_ty = #arg_value_or_default;
             let #arg_name = #borrow_tmp;
         })
     } else {
         Ok(quote_arg_span! {
-            let #arg_name = match #arg_value {
-                Some(_obj) => #extract,
-                None => #default,
-            };
+            let #arg_name = #arg_value_or_default;
         })
     };
 

src/callback.rs

@@ -5,11 +5,13 @@
 use crate::err::{PyErr, PyResult};
 use crate::exceptions::PyOverflowError;
 use crate::ffi::{self, Py_hash_t};
-use crate::IntoPyPointer;
+use crate::panic::PanicException;
+use crate::{GILPool, IntoPyPointer};
 use crate::{IntoPy, PyObject, Python};
-use std::isize;
+use std::any::Any;
 use std::os::raw::c_int;
-use std::panic::UnwindSafe;
+use std::panic::{AssertUnwindSafe, UnwindSafe};
+use std::{isize, panic};
 
 /// A type which can be the return type of a python C-API callback
 pub trait PyCallbackOutput: Copy {
@@ -173,15 +175,6 @@ where
     value.convert(py)
 }
 
-#[doc(hidden)]
-#[inline]
-pub fn callback_error<T>() -> T
-where
-    T: PyCallbackOutput,
-{
-    T::ERR_VALUE
-}
-
 /// Use this macro for all internal callback functions which Python will call.
 ///
 /// It sets up the GILPool and converts the output into a Python object. It also restores
@@ -234,30 +227,33 @@ macro_rules! callback_body {
 #[doc(hidden)]
 pub unsafe fn handle_panic<F, R>(body: F) -> R
 where
-    F: FnOnce(Python) -> crate::PyResult<R> + UnwindSafe,
+    F: FnOnce(Python) -> PyResult<R> + UnwindSafe,
     R: PyCallbackOutput,
 {
-    let pool = crate::GILPool::new();
-    let unwind_safe_py = std::panic::AssertUnwindSafe(pool.python());
-    let result =
-        match std::panic::catch_unwind(move || -> crate::PyResult<_> { body(*unwind_safe_py) }) {
-            Ok(result) => result,
-            Err(e) => {
-                // Try to format the error in the same way panic does
-                if let Some(string) = e.downcast_ref::<String>() {
-                    Err(crate::panic::PanicException::new_err((string.clone(),)))
-                } else if let Some(s) = e.downcast_ref::<&str>() {
-                    Err(crate::panic::PanicException::new_err((s.to_string(),)))
-                } else {
-                    Err(crate::panic::PanicException::new_err((
-                        "panic from Rust code",
-                    )))
-                }
-            }
-        };
+    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)
+}
+
+fn panic_result_into_callback_output<R>(
+    py: Python,
+    panic_result: Result<PyResult<R>, Box<dyn Any + Send + 'static>>,
+) -> R
+where
+    R: PyCallbackOutput,
+{
+    let py_result = match panic_result {
+        Ok(py_result) => py_result,
+        Err(payload) => Err(PanicException::from_panic_payload(payload)),
+    };
 
-    result.unwrap_or_else(|e| {
-        e.restore(pool.python());
-        crate::callback::callback_error()
+    py_result.unwrap_or_else(|py_err| {
+        py_err.restore(py);
+        R::ERR_VALUE
     })
 }

src/class/impl_.rs

@@ -65,14 +65,14 @@ pub trait PyClassImpl: Sized {
     ///    can be accessed by multiple threads by `threading` module.
     type ThreadChecker: PyClassThreadChecker<Self>;
 
-    fn for_each_method_def(_visitor: impl FnMut(&PyMethodDefType)) {}
+    fn for_each_method_def(_visitor: &mut dyn FnMut(&PyMethodDefType)) {}
     fn get_new() -> Option<ffi::newfunc> {
         None
     }
     fn get_call() -> Option<ffi::PyCFunctionWithKeywords> {
         None
     }
-    fn for_each_proto_slot(_visitor: impl FnMut(&ffi::PyType_Slot)) {}
+    fn for_each_proto_slot(_visitor: &mut dyn FnMut(&ffi::PyType_Slot)) {}
     fn get_buffer() -> Option<&'static PyBufferProcs> {
         None
     }

src/panic.rs

@@ -1,4 +1,6 @@
 use crate::exceptions::PyBaseException;
+use crate::PyErr;
+use std::any::Any;
 
 pyo3_exception!(
     "
@@ -11,3 +13,16 @@ pyo3_exception!(
     PanicException,
     PyBaseException
 );
+
+impl PanicException {
+    // Try to format the error in the same way panic does
+    pub(crate) fn from_panic_payload(payload: Box<dyn Any + Send + 'static>) -> PyErr {
+        if let Some(string) = payload.downcast_ref::<String>() {
+            Self::new_err((string.clone(),))
+        } else if let Some(s) = payload.downcast_ref::<&str>() {
+            Self::new_err((s.to_string(),))
+        } else {
+            Self::new_err(("panic from Rust code",))
+        }
+    }
+}

src/pyclass.rs

@@ -206,20 +206,20 @@ where
     }
 
     // normal methods
-    let methods = py_class_method_defs::<T>();
+    let methods = py_class_method_defs(&T::for_each_method_def);
     if !methods.is_empty() {
         slots.push(ffi::Py_tp_methods, into_raw(methods));
     }
 
     // properties
-    let props = py_class_properties::<T>();
+    let props = py_class_properties(T::Dict::IS_DUMMY, &T::for_each_method_def);
     if !props.is_empty() {
         slots.push(ffi::Py_tp_getset, into_raw(props));
     }
 
     // protocol methods
     let mut has_gc_methods = false;
-    T::for_each_proto_slot(|slot| {
+    T::for_each_proto_slot(&mut |slot| {
         has_gc_methods |= slot.slot == ffi::Py_tp_clear;
         has_gc_methods |= slot.slot == ffi::Py_tp_traverse;
         slots.0.push(*slot);
@@ -230,7 +230,7 @@ where
         name: get_type_name::<T>(module_name)?,
         basicsize: std::mem::size_of::<T::Layout>() as c_int,
         itemsize: 0,
-        flags: py_class_flags::<T>(has_gc_methods),
+        flags: py_class_flags(has_gc_methods, T::IS_GC, T::IS_BASETYPE),
         slots: slots.0.as_mut_ptr(),
     };
 
@@ -299,22 +299,24 @@ fn tp_init_additional<T: PyClass>(type_object: *mut ffi::PyTypeObject) {
 #[cfg(any(Py_LIMITED_API, Py_3_10))]
 fn tp_init_additional<T: PyClass>(_type_object: *mut ffi::PyTypeObject) {}
 
-fn py_class_flags<T: PyClass + PyTypeInfo>(has_gc_methods: bool) -> c_uint {
-    let mut flags = if has_gc_methods || T::IS_GC {
+fn py_class_flags(has_gc_methods: bool, is_gc: bool, is_basetype: bool) -> c_uint {
+    let mut flags = if has_gc_methods || is_gc {
         ffi::Py_TPFLAGS_DEFAULT | ffi::Py_TPFLAGS_HAVE_GC
     } else {
         ffi::Py_TPFLAGS_DEFAULT
     };
-    if T::IS_BASETYPE {
+    if is_basetype {
         flags |= ffi::Py_TPFLAGS_BASETYPE;
     }
     flags.try_into().unwrap()
 }
 
-fn py_class_method_defs<T: PyClassImpl>() -> Vec<ffi::PyMethodDef> {
+fn py_class_method_defs(
+    for_each_method_def: &dyn Fn(&mut dyn FnMut(&PyMethodDefType)),
+) -> Vec<ffi::PyMethodDef> {
     let mut defs = Vec::new();
 
-    T::for_each_method_def(|def| match def {
+    for_each_method_def(&mut |def| match def {
         PyMethodDefType::Method(def)
         | PyMethodDefType::Class(def)
         | PyMethodDefType::Static(def) => {
@@ -381,10 +383,13 @@ const PY_GET_SET_DEF_INIT: ffi::PyGetSetDef = ffi::PyGetSetDef {
 };
 
 #[allow(clippy::clippy::collapsible_if)] // for if cfg!
-fn py_class_properties<T: PyClass>() -> Vec<ffi::PyGetSetDef> {
+fn py_class_properties(
+    is_dummy: bool,
+    for_each_method_def: &dyn Fn(&mut dyn FnMut(&PyMethodDefType)),
+) -> Vec<ffi::PyGetSetDef> {
     let mut defs = std::collections::HashMap::new();
 
-    T::for_each_method_def(|def| match def {
+    for_each_method_def(&mut |def| match def {
         PyMethodDefType::Getter(getter) => {
             getter.copy_to(defs.entry(getter.name).or_insert(PY_GET_SET_DEF_INIT));
         }
@@ -398,7 +403,7 @@ fn py_class_properties<T: PyClass>() -> Vec<ffi::PyGetSetDef> {
 
     // PyPy doesn't automatically adds __dict__ getter / setter.
     // PyObject_GenericGetDict not in the limited API until Python 3.10.
-    push_dict_getset::<T>(&mut props);
+    push_dict_getset(&mut props, is_dummy);
 
     if !props.is_empty() {
         props.push(unsafe { std::mem::zeroed() });
@@ -407,8 +412,8 @@ fn py_class_properties<T: PyClass>() -> Vec<ffi::PyGetSetDef> {
 }
 
 #[cfg(not(any(PyPy, all(Py_LIMITED_API, not(Py_3_10)))))]
-fn push_dict_getset<T: PyClass>(props: &mut Vec<ffi::PyGetSetDef>) {
-    if !T::Dict::IS_DUMMY {
+fn push_dict_getset(props: &mut Vec<ffi::PyGetSetDef>, is_dummy: bool) {
+    if !is_dummy {
         props.push(ffi::PyGetSetDef {
             name: "__dict__\0".as_ptr() as *mut c_char,
             get: Some(ffi::PyObject_GenericGetDict),
@@ -420,4 +425,4 @@ fn push_dict_getset<T: PyClass>(props: &mut Vec<ffi::PyGetSetDef>) {
 }
 
 #[cfg(any(PyPy, all(Py_LIMITED_API, not(Py_3_10))))]
-fn push_dict_getset<T: PyClass>(_: &mut Vec<ffi::PyGetSetDef>) {}
+fn push_dict_getset(_: &mut Vec<ffi::PyGetSetDef>, _is_dummy: bool) {}

src/type_object.rs

@@ -104,6 +104,17 @@ impl LazyStaticType {
             })
         });
 
+        self.ensure_init(py, type_object, T::NAME, &T::for_each_method_def);
+        type_object
+    }
+
+    fn ensure_init(
+        &self,
+        py: Python,
+        type_object: *mut ffi::PyTypeObject,
+        name: &str,
+        for_each_method_def: &dyn Fn(&mut dyn FnMut(&PyMethodDefType)),
+    ) {
         // We might want to fill the `tp_dict` with python instances of `T`
         // itself. In order to do so, we must first initialize the type object
         // with an empty `tp_dict`: now we can create instances of `T`.
@@ -117,7 +128,7 @@ impl LazyStaticType {
 
         if self.tp_dict_filled.get(py).is_some() {
             // `tp_dict` is already filled: ok.
-            return type_object;
+            return;
         }
 
         {
@@ -126,7 +137,7 @@ impl LazyStaticType {
             if threads.contains(&thread_id) {
                 // Reentrant call: just return the type object, even if the
                 // `tp_dict` is not filled yet.
-                return type_object;
+                return;
             }
             threads.push(thread_id);
         }
@@ -136,7 +147,7 @@ impl LazyStaticType {
         // means that another thread can continue the initialization in the
         // meantime: at worst, we'll just make a useless computation.
         let mut items = vec![];
-        T::for_each_method_def(|def| {
+        for_each_method_def(&mut |def| {
             if let PyMethodDefType::ClassAttribute(attr) = def {
                 items.push((
                     extract_cstr_or_leak_cstring(
@@ -162,10 +173,8 @@ impl LazyStaticType {
 
         if let Err(err) = result {
             err.clone_ref(py).print(py);
-            panic!("An error occured while initializing `{}.__dict__`", T::NAME);
+            panic!("An error occured while initializing `{}.__dict__`", name);
         }
-
-        type_object
     }
 }