Merge pull request RustPython#4721 from youknowone/opt-getattr

type.__name__ & getattr optimization

Author: youknowone

Lib/test/test_socket.py

@@ -1650,8 +1650,6 @@ def test_dealloc_warn(self):
             f = None
             support.gc_collect()
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_name_closed_socketio(self):
         with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
             fp = sock.makefile("rb")

derive-impl/src/pymodule.rs

@@ -458,6 +458,7 @@ impl ModuleItem for ClassItem {
         let set_attr = match py_names.len() {
             0 => quote! {
                 let _ = new_class;  // suppress warning
+                let _ = vm.ctx.intern_str(#class_name);
             },
             1 => {
                 let py_name = &py_names[0];

src/shell/helper.rs

@@ -78,7 +78,8 @@ impl<'vm> ShellHelper<'vm> {
             let mut current = self.globals.get_item_opt(first.as_str(), self.vm).ok()??;
 
             for attr in parents {
-                current = current.get_attr(attr.as_str(), self.vm).ok()?;
+                let attr = self.vm.ctx.new_str(attr.as_str());
+                current = current.get_attr(&attr, self.vm).ok()?;
             }
 
             let current_iter = str_iter_method(current, identifier!(self.vm, __dir__)).ok()?;

stdlib/src/sqlite.rs

@@ -766,7 +766,7 @@ mod _sqlite {
 
     pub(super) fn setup_module(module: &PyObject, vm: &VirtualMachine) {
         for (name, code) in ERROR_CODES {
-            let name = vm.ctx.new_str(*name);
+            let name = vm.ctx.intern_str(*name);
             let code = vm.new_pyobj(*code);
             module.set_attr(name, code, vm).unwrap();
         }

stdlib/src/ssl.rs

@@ -230,7 +230,7 @@ mod _ssl {
     /// SSL/TLS connection terminated abruptly.
     #[pyattr(name = "SSLEOFError", once)]
     fn ssl_eof_error(vm: &VirtualMachine) -> PyTypeRef {
-        PyType::new_simple_ref("ssl.SSLEOFError", &ssl_error(vm), &vm.ctx).unwrap()
+        PyType::new_simple_heap("ssl.SSLEOFError", &ssl_error(vm), &vm.ctx).unwrap()
     }
 
     type OpensslVersionInfo = (u8, u8, u8, u8, u8);

vm/src/builtins/builtin_func.rs

@@ -259,7 +259,7 @@ impl PyBuiltinMethod {
         vm: &VirtualMachine,
     ) -> (Option<PyObjectRef>, (Option<PyObjectRef>, PyStrRef)) {
         let builtins_getattr = vm.builtins.get_attr("getattr", vm).ok();
-        let classname = vm.builtins.get_attr(self.class.name().to_string(), vm).ok();
+        let classname = vm.builtins.get_attr(&self.class.__name__(vm), vm).ok();
         (builtins_getattr, (classname, self.value.name.clone()))
     }
 }

vm/src/builtins/function.rs

@@ -501,10 +501,10 @@ impl Comparable for PyBoundMethod {
 }
 
 impl GetAttr for PyBoundMethod {
-    fn getattro(zelf: &Py<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+    fn getattro(zelf: &Py<Self>, name: &Py<PyStr>, vm: &VirtualMachine) -> PyResult {
         let class_attr = vm
             .ctx
-            .interned_str(&*name)
+            .interned_str(name)
             .and_then(|attr_name| zelf.get_class_attr(attr_name));
         if let Some(obj) = class_attr {
             return vm.call_if_get_descriptor(obj, zelf.to_owned().into());

vm/src/builtins/genericalias.rs

@@ -3,7 +3,7 @@ use once_cell::sync::Lazy;
 use super::type_;
 use crate::{
     atomic_func,
-    builtins::{PyList, PyStr, PyStrRef, PyTuple, PyTupleRef, PyType, PyTypeRef},
+    builtins::{PyList, PyStr, PyTuple, PyTupleRef, PyType, PyTypeRef},
     class::PyClassImpl,
     common::hash,
     convert::ToPyObject,
@@ -214,9 +214,9 @@ impl PyGenericAlias {
     }
 }
 
-fn is_typevar(obj: &PyObjectRef, vm: &VirtualMachine) -> bool {
+pub(crate) fn is_typevar(obj: &PyObjectRef, vm: &VirtualMachine) -> bool {
     let class = obj.class();
-    class.slot_name() == "TypeVar"
+    "TypeVar" == &*class.slot_name()
         && class
             .get_attr(identifier!(vm, __module__))
             .and_then(|o| o.downcast_ref::<PyStr>().map(|s| s.as_str() == "typing"))
@@ -390,7 +390,7 @@ impl Hashable for PyGenericAlias {
 }
 
 impl GetAttr for PyGenericAlias {
-    fn getattro(zelf: &Py<Self>, attr: PyStrRef, vm: &VirtualMachine) -> PyResult {
+    fn getattro(zelf: &Py<Self>, attr: &Py<PyStr>, vm: &VirtualMachine) -> PyResult {
         for exc in ATTR_EXCEPTIONS.iter() {
             if *(*exc) == attr.to_string() {
                 return zelf.as_object().generic_getattr(attr, vm);

vm/src/builtins/module.rs

@@ -1,7 +1,6 @@
-use super::pystr::IntoPyStrRef;
 use super::{PyDictRef, PyStr, PyStrRef, PyType, PyTypeRef};
 use crate::{
-    builtins::PyStrInterned,
+    builtins::{pystr::AsPyStr, PyStrInterned},
     class::PyClassImpl,
     convert::ToPyObject,
     function::FuncArgs,
@@ -26,60 +25,36 @@ pub struct ModuleInitArgs {
     doc: Option<PyStrRef>,
 }
 
-#[pyclass(with(GetAttr, Initializer, Representable), flags(BASETYPE, HAS_DICT))]
 impl PyModule {
     // pub(crate) fn new(d: PyDictRef) -> Self {
     //     PyModule { dict: d.into() }
     // }
+}
 
-    // #[inline]
-    // pub fn dict(&self) -> PyDictRef {
-    //     self.dict.get()
-    // }
-
-    #[pyslot]
-    fn slot_new(cls: PyTypeRef, _args: FuncArgs, vm: &VirtualMachine) -> PyResult {
-        PyModule {}.into_ref_with_type(vm, cls).map(Into::into)
-    }
-
-    fn getattr_inner(zelf: &Py<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        if let Some(attr) = zelf
-            .as_object()
-            .generic_getattr_opt(name.clone(), None, vm)?
-        {
+impl Py<PyModule> {
+    fn getattr_inner(&self, name: &Py<PyStr>, vm: &VirtualMachine) -> PyResult {
+        if let Some(attr) = self.as_object().generic_getattr_opt(name, None, vm)? {
             return Ok(attr);
         }
-        if let Ok(getattr) = zelf.dict().get_item(identifier!(vm, __getattr__), vm) {
-            return getattr.call((name,), vm);
+        if let Ok(getattr) = self.dict().get_item(identifier!(vm, __getattr__), vm) {
+            return getattr.call((name.to_owned(),), vm);
         }
-        let module_name = if let Some(name) = Self::name(zelf.to_owned(), vm) {
+        let module_name = if let Some(name) = self.name(vm) {
             format!(" '{name}'")
         } else {
             "".to_owned()
         };
         Err(vm.new_attribute_error(format!("module{module_name} has no attribute '{name}'")))
     }
 
-    fn name(zelf: PyRef<Self>, vm: &VirtualMachine) -> Option<PyStrRef> {
-        let name = zelf
+    fn name(&self, vm: &VirtualMachine) -> Option<PyStrRef> {
+        let name = self
             .as_object()
-            .generic_getattr_opt(identifier!(vm, __name__).to_owned(), None, vm)
+            .generic_getattr_opt(identifier!(vm, __name__), None, vm)
             .unwrap_or_default()?;
         name.downcast::<PyStr>().ok()
     }
 
-    #[pymethod(magic)]
-    fn dir(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<Vec<PyObjectRef>> {
-        let dict = zelf
-            .as_object()
-            .dict()
-            .ok_or_else(|| vm.new_value_error("module has no dict".to_owned()))?;
-        let attrs = dict.into_iter().map(|(k, _v)| k).collect();
-        Ok(attrs)
-    }
-}
-
-impl Py<PyModule> {
     // TODO: to be replaced by the commented-out dict method above once dictoffsets land
     pub fn dict(&self) -> PyDictRef {
         self.as_object().dict().unwrap()
@@ -104,19 +79,38 @@ impl Py<PyModule> {
             .expect("Failed to set __spec__ on module");
     }
 
-    pub fn get_attr(&self, attr_name: impl IntoPyStrRef, vm: &VirtualMachine) -> PyResult {
-        PyModule::getattr_inner(self, attr_name.into_pystr_ref(vm), vm)
+    pub fn get_attr<'a>(&self, attr_name: impl AsPyStr<'a>, vm: &VirtualMachine) -> PyResult {
+        self.getattr_inner(attr_name.as_pystr(&vm.ctx), vm)
     }
-    pub fn set_attr(
+
+    pub fn set_attr<'a>(
         &self,
-        attr_name: impl IntoPyStrRef,
+        attr_name: impl AsPyStr<'a>,
         attr_value: impl Into<PyObjectRef>,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
         self.as_object().set_attr(attr_name, attr_value, vm)
     }
 }
 
+#[pyclass(with(GetAttr, Initializer, Representable), flags(BASETYPE, HAS_DICT))]
+impl PyModule {
+    #[pyslot]
+    fn slot_new(cls: PyTypeRef, _args: FuncArgs, vm: &VirtualMachine) -> PyResult {
+        PyModule {}.into_ref_with_type(vm, cls).map(Into::into)
+    }
+
+    #[pymethod(magic)]
+    fn dir(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<Vec<PyObjectRef>> {
+        let dict = zelf
+            .as_object()
+            .dict()
+            .ok_or_else(|| vm.new_value_error("module has no dict".to_owned()))?;
+        let attrs = dict.into_iter().map(|(k, _v)| k).collect();
+        Ok(attrs)
+    }
+}
+
 impl Initializer for PyModule {
     type Args = ModuleInitArgs;
 
@@ -136,8 +130,8 @@ impl Initializer for PyModule {
 }
 
 impl GetAttr for PyModule {
-    fn getattro(zelf: &Py<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        Self::getattr_inner(zelf, name, vm)
+    fn getattro(zelf: &Py<Self>, name: &Py<PyStr>, vm: &VirtualMachine) -> PyResult {
+        zelf.getattr_inner(name, vm)
     }
 }
 

vm/src/builtins/namespace.rs

@@ -53,7 +53,8 @@ impl Initializer for PyNamespace {
             return Err(vm.new_type_error("no positional arguments expected".to_owned()));
         }
         for (name, value) in args.kwargs.into_iter() {
-            zelf.as_object().set_attr(name, value, vm)?;
+            let name = vm.ctx.new_str(name);
+            zelf.as_object().set_attr(&name, value, vm)?;
         }
         Ok(())
     }
@@ -82,7 +83,7 @@ impl Representable for PyNamespace {
         let name = if o.class().is(vm.ctx.types.namespace_type) {
             "namespace".to_owned()
         } else {
-            o.class().slot_name()
+            o.class().slot_name().to_owned()
         };
 
         let repr = if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) {