Merge pull request #2273 from PyO3/dog-food-intern

Make use of intern! macro for attribute names used internally
PyO3 · Apr 4, 2022 · 9774a7c · 9774a7c
2 parents d3ac71a + f02a060
commit 9774a7c
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 16 deletions.
diff --git a/pyo3-macros-backend/src/frompyobject.rs b/pyo3-macros-backend/src/frompyobject.rs
@@ -294,22 +294,25 @@ impl<'a> Container<'a> {
         let mut fields: Punctuated<TokenStream, syn::Token![,]> = Punctuated::new();
         for (ident, attrs) in tups {
             let getter = match &attrs.getter {
-                FieldGetter::GetAttr(Some(name)) => quote!(getattr(#name)),
-                FieldGetter::GetAttr(None) => quote!(getattr(stringify!(#ident))),
+                FieldGetter::GetAttr(Some(name)) => quote!(getattr(_pyo3::intern!(py, #name))),
+                FieldGetter::GetAttr(None) => {
+                    quote!(getattr(_pyo3::intern!(py, stringify!(#ident))))
+                }
                 FieldGetter::GetItem(Some(key)) => quote!(get_item(#key)),
                 FieldGetter::GetItem(None) => quote!(get_item(stringify!(#ident))),
             };
             let conversion_error_msg =
                 format!("failed to extract field {}.{}", quote!(#self_ty), ident);
             let get_field = quote!(obj.#getter?);
             let extractor = match &attrs.from_py_with {
-                None => quote!(
-                    #get_field.extract().map_err(|inner| {
+                None => quote!({
                     let py = _pyo3::PyNativeType::py(obj);
-                    let new_err = _pyo3::exceptions::PyTypeError::new_err(#conversion_error_msg);
-                    new_err.set_cause(py, ::std::option::Option::Some(inner));
-                    new_err
-                })?),
+                    #get_field.extract().map_err(|inner| {
+                        let new_err = _pyo3::exceptions::PyTypeError::new_err(#conversion_error_msg);
+                        new_err.set_cause(py, ::std::option::Option::Some(inner));
+                        new_err
+                    })?
+                }),
                 Some(FromPyWithAttribute {
                     value: expr_path, ..
                 }) => quote! (

diff --git a/src/instance.rs b/src/instance.rs
@@ -534,6 +534,25 @@ impl<T> Py<T> {
     /// Retrieves an attribute value.
     ///
     /// This is equivalent to the Python expression `self.attr_name`.
+    ///
+    /// If calling this method becomes performance-critical, the [`intern!`] macro can be used
+    /// to intern `attr_name`, thereby avoiding repeated temporary allocations of Python strings.
+    ///
+    /// # Example: `intern!`ing the attribute name
+    ///
+    /// ```
+    /// # use pyo3::{intern, pyfunction, types::PyModule, IntoPy, Py, Python, PyObject, PyResult};
+    /// #
+    /// #[pyfunction]
+    /// fn version(sys: Py<PyModule>, py: Python<'_>) -> PyResult<PyObject> {
+    ///     sys.getattr(py, intern!(py, "version"))
+    /// }
+    /// #
+    /// # Python::with_gil(|py| {
+    /// #    let sys = py.import("sys").unwrap().into_py(py);
+    /// #    version(sys, py).unwrap();
+    /// # });
+    /// ```
     pub fn getattr<N>(&self, py: Python<'_>, attr_name: N) -> PyResult<PyObject>
     where
         N: ToPyObject,
@@ -546,6 +565,25 @@ impl<T> Py<T> {
     /// Sets an attribute value.
     ///
     /// This is equivalent to the Python expression `self.attr_name = value`.
+    ///
+    /// If calling this method becomes performance-critical, the [`intern!`] macro can be used
+    /// to intern `attr_name`, thereby avoiding repeated temporary allocations of Python strings.
+    ///
+    /// # Example: `intern!`ing the attribute name
+    ///
+    /// ```
+    /// # use pyo3::{intern, pyfunction, types::PyModule, IntoPy, PyObject, Python, PyResult};
+    /// #
+    /// #[pyfunction]
+    /// fn set_answer(ob: PyObject, py: Python<'_>) -> PyResult<()> {
+    ///     ob.setattr(py, intern!(py, "answer"), 42)
+    /// }
+    /// #
+    /// # Python::with_gil(|py| {
+    /// #    let ob = PyModule::new(py, "empty").unwrap().into_py(py);
+    /// #    set_answer(ob, py).unwrap();
+    /// # });
+    /// ```
     pub fn setattr<N, V>(&self, py: Python<'_>, attr_name: N, value: V) -> PyResult<()>
     where
         N: ToPyObject,

diff --git a/src/types/any.rs b/src/types/any.rs
@@ -111,6 +111,25 @@ impl PyAny {
     /// Retrieves an attribute value.
     ///
     /// This is equivalent to the Python expression `self.attr_name`.
+    ///
+    /// If calling this method becomes performance-critical, the [`intern!`] macro can be used
+    /// to intern `attr_name`, thereby avoiding repeated temporary allocations of Python strings.
+    ///
+    /// # Example: `intern!`ing the attribute name
+    ///
+    /// ```
+    /// # use pyo3::{intern, pyfunction, types::PyModule, PyAny, Python, PyResult};
+    /// #
+    /// #[pyfunction]
+    /// fn version(sys: &PyModule) -> PyResult<&PyAny> {
+    ///     sys.getattr(intern!(sys.py(), "version"))
+    /// }
+    /// #
+    /// # Python::with_gil(|py| {
+    /// #    let sys = py.import("sys").unwrap();
+    /// #    version(sys).unwrap();
+    /// # });
+    /// ```
     pub fn getattr<N>(&self, attr_name: N) -> PyResult<&PyAny>
     where
         N: ToPyObject,
@@ -124,6 +143,25 @@ impl PyAny {
     /// Sets an attribute value.
     ///
     /// This is equivalent to the Python expression `self.attr_name = value`.
+    ///
+    /// If calling this method becomes performance-critical, the [`intern!`] macro can be used
+    /// to intern `attr_name`, thereby avoiding repeated temporary allocations of Python strings.
+    ///
+    /// # Example: `intern!`ing the attribute name
+    ///
+    /// ```
+    /// # use pyo3::{intern, pyfunction, types::PyModule, PyAny, Python, PyResult};
+    /// #
+    /// #[pyfunction]
+    /// fn set_answer(ob: &PyAny) -> PyResult<()> {
+    ///     ob.setattr(intern!(ob.py(), "answer"), 42)
+    /// }
+    /// #
+    /// # Python::with_gil(|py| {
+    /// #    let ob = PyModule::new(py, "empty").unwrap();
+    /// #    set_answer(ob).unwrap();
+    /// # });
+    /// ```
     pub fn setattr<N, V>(&self, attr_name: N, value: V) -> PyResult<()>
     where
         N: ToBorrowedObject,

diff --git a/src/types/module.rs b/src/types/module.rs
@@ -8,8 +8,7 @@ use crate::exceptions;
 use crate::ffi;
 use crate::pyclass::PyClass;
 use crate::type_object::PyTypeObject;
-use crate::types::PyCFunction;
-use crate::types::{PyAny, PyDict, PyList};
+use crate::types::{PyAny, PyCFunction, PyDict, PyList, PyString};
 use crate::{AsPyPointer, IntoPy, PyObject, Python};
 use std::ffi::{CStr, CString};
 use std::str;
@@ -168,12 +167,13 @@ impl PyModule {
     ///
     /// `__all__` declares the items that will be imported with `from my_module import *`.
     pub fn index(&self) -> PyResult<&PyList> {
-        match self.getattr("__all__") {
+        let __all__ = __all__(self.py());
+        match self.getattr(__all__) {
             Ok(idx) => idx.downcast().map_err(PyErr::from),
             Err(err) => {
                 if err.is_instance_of::<exceptions::PyAttributeError>(self.py()) {
                     let l = PyList::empty(self.py());
-                    self.setattr("__all__", l).map_err(PyErr::from)?;
+                    self.setattr(__all__, l).map_err(PyErr::from)?;
                     Ok(l)
                 } else {
                     Err(err)
@@ -202,7 +202,6 @@ impl PyModule {
     /// May fail if the module does not have a `__file__` attribute.
     #[cfg(not(all(windows, PyPy)))]
     pub fn filename(&self) -> PyResult<&str> {
-        use crate::types::PyString;
         unsafe {
             self.py()
                 .from_owned_ptr_or_err::<PyString>(ffi::PyModule_GetFilenameObject(self.as_ptr()))?
@@ -304,7 +303,7 @@ impl PyModule {
     {
         let py = self.py();
         let function = wrapper(py).convert(py)?;
-        let name = function.getattr(py, "__name__")?;
+        let name = function.getattr(py, __name__(py))?;
         let name = name.extract(py)?;
         self.add(name, function)
     }
@@ -389,11 +388,19 @@ impl PyModule {
     /// [1]: crate::prelude::pyfunction
     /// [2]: crate::wrap_pyfunction
     pub fn add_function<'a>(&'a self, fun: &'a PyCFunction) -> PyResult<()> {
-        let name = fun.getattr("__name__")?.extract()?;
+        let name = fun.getattr(__name__(self.py()))?.extract()?;
         self.add(name, fun)
     }
 }
 
+fn __all__(py: Python<'_>) -> &PyString {
+    intern!(py, "__all__")
+}
+
+fn __name__(py: Python<'_>) -> &PyString {
+    intern!(py, "__name__")
+}
+
 #[cfg(test)]
 mod tests {
     use crate::{types::PyModule, Python};

diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs
@@ -37,7 +37,7 @@ impl PyType {
 
     /// Gets the name of the `PyType`.
     pub fn name(&self) -> PyResult<&str> {
-        self.getattr("__qualname__")?.extract()
+        self.getattr(intern!(self.py(), "__qualname__"))?.extract()
     }
 
     /// Checks whether `self` is a subclass of `other`.