Merge pull request #2235 from RustPython/coolreader18/tp_getattro

getattro slot

benchmarks/bench.rs

@@ -11,6 +11,7 @@ use rustpython_vm::Interpreter;
 const MINIDOM: &str = include_str!("./benchmarks/minidom.py");
 const NBODY: &str = include_str!("./benchmarks/nbody.py");
 const MANDELBROT: &str = include_str!("./benchmarks/mandelbrot.py");
+const PYSTONE: &str = include_str!("./benchmarks/pystone.py");
 
 #[bench]
 fn bench_tokenization(b: &mut Bencher) {
@@ -81,6 +82,11 @@ fn bench_cpython_mandelbrot(b: &mut Bencher) {
     bench_cpython(b, MANDELBROT)
 }
 
+#[bench]
+fn bench_cpython_pystone(b: &mut Bencher) {
+    bench_cpython(b, PYSTONE)
+}
+
 fn bench_rustpy(b: &mut Bencher, name: &str, source: &str) {
     // NOTE: Take long time.
     Interpreter::default().enter(|vm| {
@@ -105,3 +111,8 @@ fn bench_rustpy_nbody(b: &mut Bencher) {
 fn bench_rustpy_mandelbrot(b: &mut Bencher) {
     bench_rustpy(b, "mandelbrot.py", MANDELBROT)
 }
+
+#[bench]
+fn bench_rustpy_pystone(b: &mut Bencher) {
+    bench_rustpy(b, "pystone.py", PYSTONE)
+}

benchmarks/benchmarks/pystone.py

@@ -0,0 +1,281 @@
+#! /usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+"PYSTONE" Benchmark Program
+
+Version:        Python/1.1 (corresponds to C/1.1 plus 2 Pystone fixes)
+
+Author:         Reinhold P. Weicker,  CACM Vol 27, No 10, 10/84 pg. 1013.
+
+                Translated from ADA to C by Rick Richardson.
+                Every method to preserve ADA-likeness has been used,
+                at the expense of C-ness.
+
+                Translated from C to Python by Guido van Rossum.
+
+Version History:
+
+                Inofficial version 1.1.1 by Chris Arndt:
+
+                - Make it run under Python 2 and 3 by using
+                  "from __future__ import print_function".
+                - Change interpreter name in shebang line to plain
+                  "python".
+                - Add source code encoding declaration.
+
+                Version 1.1 corrects two bugs in version 1.0:
+
+                First, it leaked memory: in Proc1(), NextRecord ends
+                up having a pointer to itself.  I have corrected this
+                by zapping NextRecord.PtrComp at the end of Proc1().
+
+                Second, Proc3() used the operator != to compare a
+                record to None.  This is rather inefficient and not
+                true to the intention of the original benchmark (where
+                a pointer comparison to None is intended; the !=
+                operator attempts to find a method __cmp__ to do value
+                comparison of the record).  Version 1.1 runs 5-10
+                percent faster than version 1.0, so benchmark figures
+                of different versions can't be compared directly.
+
+"""
+
+from __future__ import print_function
+
+LOOPS = 50000
+
+from time import time as clock
+
+__version__ = "1.1.1"
+
+[Ident1, Ident2, Ident3, Ident4, Ident5] = range(1, 6)
+
+class Record:
+
+    def __init__(self, PtrComp = None, Discr = 0, EnumComp = 0,
+                       IntComp = 0, StringComp = 0):
+        self.PtrComp = PtrComp
+        self.Discr = Discr
+        self.EnumComp = EnumComp
+        self.IntComp = IntComp
+        self.StringComp = StringComp
+
+    def copy(self):
+        return Record(self.PtrComp, self.Discr, self.EnumComp,
+                      self.IntComp, self.StringComp)
+
+TRUE = 1
+FALSE = 0
+
+def main(loops=LOOPS):
+    benchtime, stones = pystones(loops)
+    print("Pystone(%s) time for %d passes = %f" % \
+          (__version__, loops, benchtime))
+    print("This machine benchmarks at %f pystones/second" % stones)
+
+
+def pystones(loops=LOOPS):
+    return Proc0(loops)
+
+IntGlob = 0
+BoolGlob = FALSE
+Char1Glob = '\0'
+Char2Glob = '\0'
+Array1Glob = [0]*51
+Array2Glob = [x[:] for x in [Array1Glob]*51]
+PtrGlb = None
+PtrGlbNext = None
+
+def Proc0(loops=LOOPS):
+    global IntGlob
+    global BoolGlob
+    global Char1Glob
+    global Char2Glob
+    global Array1Glob
+    global Array2Glob
+    global PtrGlb
+    global PtrGlbNext
+
+    starttime = clock()
+    for i in range(loops):
+        pass
+    nulltime = clock() - starttime
+
+    PtrGlbNext = Record()
+    PtrGlb = Record()
+    PtrGlb.PtrComp = PtrGlbNext
+    PtrGlb.Discr = Ident1
+    PtrGlb.EnumComp = Ident3
+    PtrGlb.IntComp = 40
+    PtrGlb.StringComp = "DHRYSTONE PROGRAM, SOME STRING"
+    String1Loc = "DHRYSTONE PROGRAM, 1'ST STRING"
+    Array2Glob[8][7] = 10
+
+    starttime = clock()
+
+    for i in range(loops):
+        Proc5()
+        Proc4()
+        IntLoc1 = 2
+        IntLoc2 = 3
+        String2Loc = "DHRYSTONE PROGRAM, 2'ND STRING"
+        EnumLoc = Ident2
+        BoolGlob = not Func2(String1Loc, String2Loc)
+        while IntLoc1 < IntLoc2:
+            IntLoc3 = 5 * IntLoc1 - IntLoc2
+            IntLoc3 = Proc7(IntLoc1, IntLoc2)
+            IntLoc1 = IntLoc1 + 1
+        Proc8(Array1Glob, Array2Glob, IntLoc1, IntLoc3)
+        PtrGlb = Proc1(PtrGlb)
+        CharIndex = 'A'
+        while CharIndex <= Char2Glob:
+            if EnumLoc == Func1(CharIndex, 'C'):
+                EnumLoc = Proc6(Ident1)
+            CharIndex = chr(ord(CharIndex)+1)
+        IntLoc3 = IntLoc2 * IntLoc1
+        IntLoc2 = IntLoc3 / IntLoc1
+        IntLoc2 = 7 * (IntLoc3 - IntLoc2) - IntLoc1
+        IntLoc1 = Proc2(IntLoc1)
+
+    benchtime = clock() - starttime - nulltime
+    if benchtime == 0.0:
+        loopsPerBenchtime = 0.0
+    else:
+        loopsPerBenchtime = (loops / benchtime)
+    return benchtime, loopsPerBenchtime
+
+def Proc1(PtrParIn):
+    PtrParIn.PtrComp = NextRecord = PtrGlb.copy()
+    PtrParIn.IntComp = 5
+    NextRecord.IntComp = PtrParIn.IntComp
+    NextRecord.PtrComp = PtrParIn.PtrComp
+    NextRecord.PtrComp = Proc3(NextRecord.PtrComp)
+    if NextRecord.Discr == Ident1:
+        NextRecord.IntComp = 6
+        NextRecord.EnumComp = Proc6(PtrParIn.EnumComp)
+        NextRecord.PtrComp = PtrGlb.PtrComp
+        NextRecord.IntComp = Proc7(NextRecord.IntComp, 10)
+    else:
+        PtrParIn = NextRecord.copy()
+    NextRecord.PtrComp = None
+    return PtrParIn
+
+def Proc2(IntParIO):
+    IntLoc = IntParIO + 10
+    while 1:
+        if Char1Glob == 'A':
+            IntLoc = IntLoc - 1
+            IntParIO = IntLoc - IntGlob
+            EnumLoc = Ident1
+        if EnumLoc == Ident1:
+            break
+    return IntParIO
+
+def Proc3(PtrParOut):
+    global IntGlob
+
+    if PtrGlb is not None:
+        PtrParOut = PtrGlb.PtrComp
+    else:
+        IntGlob = 100
+    PtrGlb.IntComp = Proc7(10, IntGlob)
+    return PtrParOut
+
+def Proc4():
+    global Char2Glob
+
+    BoolLoc = Char1Glob == 'A'
+    BoolLoc = BoolLoc or BoolGlob
+    Char2Glob = 'B'
+
+def Proc5():
+    global Char1Glob
+    global BoolGlob
+
+    Char1Glob = 'A'
+    BoolGlob = FALSE
+
+def Proc6(EnumParIn):
+    EnumParOut = EnumParIn
+    if not Func3(EnumParIn):
+        EnumParOut = Ident4
+    if EnumParIn == Ident1:
+        EnumParOut = Ident1
+    elif EnumParIn == Ident2:
+        if IntGlob > 100:
+            EnumParOut = Ident1
+        else:
+            EnumParOut = Ident4
+    elif EnumParIn == Ident3:
+        EnumParOut = Ident2
+    elif EnumParIn == Ident4:
+        pass
+    elif EnumParIn == Ident5:
+        EnumParOut = Ident3
+    return EnumParOut
+
+def Proc7(IntParI1, IntParI2):
+    IntLoc = IntParI1 + 2
+    IntParOut = IntParI2 + IntLoc
+    return IntParOut
+
+def Proc8(Array1Par, Array2Par, IntParI1, IntParI2):
+    global IntGlob
+
+    IntLoc = IntParI1 + 5
+    Array1Par[IntLoc] = IntParI2
+    Array1Par[IntLoc+1] = Array1Par[IntLoc]
+    Array1Par[IntLoc+30] = IntLoc
+    for IntIndex in range(IntLoc, IntLoc+2):
+        Array2Par[IntLoc][IntIndex] = IntLoc
+    Array2Par[IntLoc][IntLoc-1] = Array2Par[IntLoc][IntLoc-1] + 1
+    Array2Par[IntLoc+20][IntLoc] = Array1Par[IntLoc]
+    IntGlob = 5
+
+def Func1(CharPar1, CharPar2):
+    CharLoc1 = CharPar1
+    CharLoc2 = CharLoc1
+    if CharLoc2 != CharPar2:
+        return Ident1
+    else:
+        return Ident2
+
+def Func2(StrParI1, StrParI2):
+    IntLoc = 1
+    while IntLoc <= 1:
+        if Func1(StrParI1[IntLoc], StrParI2[IntLoc+1]) == Ident1:
+            CharLoc = 'A'
+            IntLoc = IntLoc + 1
+    if CharLoc >= 'W' and CharLoc <= 'Z':
+        IntLoc = 7
+    if CharLoc == 'X':
+        return TRUE
+    else:
+        if StrParI1 > StrParI2:
+            IntLoc = IntLoc + 7
+            return TRUE
+        else:
+            return FALSE
+
+def Func3(EnumParIn):
+    EnumLoc = EnumParIn
+    if EnumLoc == Ident3: return TRUE
+    return FALSE
+
+if __name__ == '__main__':
+    import sys
+    def error(msg):
+        print(msg, end=' ', file=sys.stderr)
+        print("usage: %s [number_of_loops]" % sys.argv[0], file=sys.stderr)
+        sys.exit(100)
+    nargs = len(sys.argv) - 1
+    if nargs > 1:
+        error("%d arguments are too many;" % nargs)
+    elif nargs == 1:
+        try: loops = int(sys.argv[1])
+        except ValueError:
+            error("Invalid argument %r;" % sys.argv[1])
+    else:
+        loops = LOOPS
+    main(loops)

bytecode/src/bytecode.rs

@@ -91,7 +91,7 @@ impl Label {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)]
 /// An indication where the name must be accessed.
 pub enum NameScope {
     /// The name will be in the local scope.

vm/src/frame.rs

@@ -10,19 +10,17 @@ use crate::common::cell::PyMutex;
 use crate::exceptions::{self, ExceptionCtor, PyBaseExceptionRef};
 use crate::function::PyFuncArgs;
 use crate::obj::objasyncgenerator::PyAsyncGenWrappedValue;
-use crate::obj::objbool;
 use crate::obj::objcode::PyCodeRef;
 use crate::obj::objcoroinner::Coro;
 use crate::obj::objcoroutine::PyCoroutine;
 use crate::obj::objdict::{PyDict, PyDictRef};
 use crate::obj::objgenerator::PyGenerator;
-use crate::obj::objiter;
-use crate::obj::objlist;
 use crate::obj::objslice::PySlice;
 use crate::obj::objstr::{self, PyStr, PyStrRef};
 use crate::obj::objtraceback::PyTraceback;
 use crate::obj::objtuple::PyTuple;
 use crate::obj::objtype::{self, PyTypeRef};
+use crate::obj::{objbool, objiter, objlist, objset};
 use crate::pyobject::{
     BorrowValue, IdProtocol, ItemProtocol, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject,
     TypeProtocol,
@@ -177,6 +175,7 @@ impl Frame {
 }
 
 impl FrameRef {
+    #[inline]
     fn with_exec<R>(&self, f: impl FnOnce(ExecutingFrame) -> R) -> R {
         let mut state = self.state.lock();
         let exec = ExecutingFrame {
@@ -350,14 +349,10 @@ impl ExecutingFrame<'_> {
             } => self.import(vm, name, symbols, *level),
             bytecode::Instruction::ImportStar => self.import_star(vm),
             bytecode::Instruction::ImportFrom { ref name } => self.import_from(vm, name),
-            bytecode::Instruction::LoadName {
-                ref name,
-                ref scope,
-            } => self.load_name(vm, name, scope),
-            bytecode::Instruction::StoreName {
-                ref name,
-                ref scope,
-            } => self.store_name(vm, name, scope),
+            bytecode::Instruction::LoadName { ref name, scope } => self.load_name(vm, name, *scope),
+            bytecode::Instruction::StoreName { ref name, scope } => {
+                self.store_name(vm, name, *scope)
+            }
             bytecode::Instruction::DeleteName { ref name } => self.delete_name(vm, name),
             bytecode::Instruction::Subscript => self.execute_subscript(vm),
             bytecode::Instruction::StoreSubscript => self.execute_store_subscript(vm),
@@ -393,11 +388,11 @@ impl ExecutingFrame<'_> {
             }
             bytecode::Instruction::BuildSet { size, unpack } => {
                 let elements = self.get_elements(vm, *size, *unpack)?;
-                let py_obj = vm.ctx.new_set();
+                let set = vm.ctx.new_set();
                 for item in elements {
-                    vm.call_method(&py_obj, "add", vec![item])?;
+                    set.add(item, vm)?;
                 }
-                self.push_value(py_obj);
+                self.push_value(set.into_object());
                 Ok(None)
             }
             bytecode::Instruction::BuildTuple { size, unpack } => {
@@ -421,14 +416,14 @@ impl ExecutingFrame<'_> {
             bytecode::Instruction::SetAdd { i } => {
                 let set_obj = self.nth_value(*i);
                 let item = self.pop_value();
-                vm.call_method(&set_obj, "add", vec![item])?;
+                objset::PySetRef::try_from_object(vm, set_obj)?.add(item, vm)?;
                 Ok(None)
             }
             bytecode::Instruction::MapAdd { i } => {
                 let dict_obj = self.nth_value(*i + 1);
                 let key = self.pop_value();
                 let value = self.pop_value();
-                vm.call_method(&dict_obj, "__setitem__", vec![key, value])?;
+                PyDictRef::try_from_object(vm, dict_obj)?.set_item(key, value, vm)?;
                 Ok(None)
             }
             bytecode::Instruction::MapAddRev { i } => {
@@ -436,7 +431,7 @@ impl ExecutingFrame<'_> {
                 let dict_obj = self.nth_value(*i + 1);
                 let value = self.pop_value();
                 let key = self.pop_value();
-                vm.call_method(&dict_obj, "__setitem__", vec![key, value])?;
+                PyDictRef::try_from_object(vm, dict_obj)?.set_item(key, value, vm)?;
                 Ok(None)
             }
             bytecode::Instruction::BinaryOperation { ref op, inplace } => {
@@ -882,7 +877,7 @@ impl ExecutingFrame<'_> {
         &mut self,
         vm: &VirtualMachine,
         name: &str,
-        name_scope: &bytecode::NameScope,
+        name_scope: bytecode::NameScope,
     ) -> FrameResult {
         let obj = self.pop_value();
         match name_scope {
@@ -910,11 +905,12 @@ impl ExecutingFrame<'_> {
     }
 
     #[cfg_attr(feature = "flame-it", flame("Frame"))]
+    #[inline]
     fn load_name(
         &mut self,
         vm: &VirtualMachine,
         name: &str,
-        name_scope: &bytecode::NameScope,
+        name_scope: bytecode::NameScope,
     ) -> FrameResult {
         let optional_value = match name_scope {
             bytecode::NameScope::Global => self.scope.load_global(vm, name),
@@ -1210,8 +1206,7 @@ impl ExecutingFrame<'_> {
 
     fn jump(&mut self, label: bytecode::Label) {
         let target_pc = self.code.label_map[&label];
-        #[cfg(feature = "vm-tracing-logging")]
-        trace!("jump from {:?} to {:?}", self.lasti(), target_pc);
+        vm_trace!("jump from {:?} to {:?}", self.lasti(), target_pc);
         self.lasti.store(target_pc, Ordering::Relaxed);
     }
 

vm/src/obj/objdict.rs

@@ -388,7 +388,7 @@ impl PyDict {
         Ok(PyDict { entries: dict })
     }
 
-    pub fn contains_key<T: IntoPyObject>(&self, key: T, vm: &VirtualMachine) -> bool {
+    pub fn contains_key<K: IntoPyObject>(&self, key: K, vm: &VirtualMachine) -> bool {
         let key = key.into_pyobject(vm);
         self.entries.contains(vm, &key).unwrap()
     }
@@ -421,6 +421,7 @@ impl Unhashable for PyDict {}
 
 impl PyDictRef {
     /// Return an optional inner item, or an error (can be key error as well)
+    #[inline]
     fn inner_getitem_option<K: DictKey>(
         &self,
         key: K,
@@ -453,9 +454,9 @@ impl PyDictRef {
     /// python value, or None.
     /// Note that we can pass any type which implements the DictKey
     /// trait. Notable examples are String and PyObjectRef.
-    pub fn get_item_option<T: IntoPyObject + DictKey>(
+    pub fn get_item_option<K: IntoPyObject + DictKey>(
         &self,
-        key: T,
+        key: K,
         vm: &VirtualMachine,
     ) -> PyResult<Option<PyObjectRef>> {
         // Test if this object is a true dict, or mabye a subclass?
@@ -492,15 +493,15 @@ impl PyDictRef {
     }
 }
 
-impl<T> ItemProtocol<T> for PyDictRef
+impl<K> ItemProtocol<K> for PyDictRef
 where
-    T: DictKey,
+    K: DictKey,
 {
-    fn get_item(&self, key: T, vm: &VirtualMachine) -> PyResult {
+    fn get_item(&self, key: K, vm: &VirtualMachine) -> PyResult {
         self.as_object().get_item(key, vm)
     }
 
-    fn set_item(&self, key: T, value: PyObjectRef, vm: &VirtualMachine) -> PyResult {
+    fn set_item(&self, key: K, value: PyObjectRef, vm: &VirtualMachine) -> PyResult {
         if self.lease_class().is(&vm.ctx.types.dict_type) {
             self.inner_setitem_fast(key, value, vm)
                 .map(|_| vm.ctx.none())
@@ -510,7 +511,7 @@ where
         }
     }
 
-    fn del_item(&self, key: T, vm: &VirtualMachine) -> PyResult {
+    fn del_item(&self, key: K, vm: &VirtualMachine) -> PyResult {
         if self.lease_class().is(&vm.ctx.types.dict_type) {
             self.entries.delete(vm, key).map(|_| vm.ctx.none())
         } else {

vm/src/obj/objframe.rs

@@ -46,7 +46,7 @@ impl FrameRef {
 
     #[pyproperty]
     fn f_locals(self) -> PyDictRef {
-        self.scope.get_locals()
+        self.scope.get_locals().clone()
     }
 
     #[pyproperty]

vm/src/obj/objfunction.rs

@@ -19,7 +19,7 @@ use crate::pyobject::{
     PyRef, PyResult, PyValue, TypeProtocol,
 };
 use crate::scope::Scope;
-use crate::slots::{Callable, Comparable, PyComparisonOp, SlotDescriptor};
+use crate::slots::{Callable, Comparable, PyComparisonOp, SlotDescriptor, SlotGetattro};
 use crate::VirtualMachine;
 use itertools::Itertools;
 #[cfg(feature = "jit")]
@@ -357,7 +357,16 @@ impl Comparable for PyBoundMethod {
     }
 }
 
-#[pyimpl(with(Callable, Comparable), flags(HAS_DICT))]
+impl SlotGetattro for PyBoundMethod {
+    fn getattro(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        if let Some(obj) = zelf.get_class_attr(name.borrow_value()) {
+            return vm.call_if_get_descriptor(obj, zelf.into_object());
+        }
+        vm.get_attribute(zelf.function.clone(), name)
+    }
+}
+
+#[pyimpl(with(Callable, Comparable, SlotGetattro), flags(HAS_DICT))]
 impl PyBoundMethod {
     pub fn new(object: PyObjectRef, function: PyObjectRef) -> Self {
         PyBoundMethod { object, function }
@@ -376,14 +385,6 @@ impl PyBoundMethod {
         vm.get_attribute(self.function.clone(), "__doc__")
     }
 
-    #[pymethod(magic)]
-    fn getattribute(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        if let Some(obj) = zelf.get_class_attr(name.borrow_value()) {
-            return vm.call_if_get_descriptor(obj, zelf.into_object());
-        }
-        vm.get_attribute(zelf.function.clone(), name)
-    }
-
     #[pyproperty(magic)]
     fn func(&self) -> PyObjectRef {
         self.function.clone()

vm/src/obj/objmodule.rs

@@ -6,6 +6,7 @@ use crate::pyobject::{
     BorrowValue, IntoPyObject, ItemProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult,
     PyValue,
 };
+use crate::slots::SlotGetattro;
 use crate::vm::VirtualMachine;
 
 #[pyclass(module = false, name = "module")]
@@ -42,8 +43,8 @@ pub fn init_module_dict(
         .expect("Failed to set __spec__ on module");
 }
 
-#[pyimpl(flags(BASETYPE, HAS_DICT))]
-impl PyModuleRef {
+#[pyimpl(with(SlotGetattro), flags(BASETYPE, HAS_DICT))]
+impl PyModule {
     #[pyslot]
     fn tp_new(cls: PyTypeRef, _args: PyFuncArgs, vm: &VirtualMachine) -> PyResult<PyModuleRef> {
         PyModule {}.into_ref_with_type(vm, cls)
@@ -51,27 +52,27 @@ impl PyModuleRef {
 
     #[pymethod(magic)]
     fn init(
-        self,
+        zelf: PyRef<Self>,
         name: PyStrRef,
         doc: OptionalOption<PyStrRef>,
         vm: &VirtualMachine,
     ) -> PyResult<()> {
-        debug_assert!(crate::pyobject::TypeProtocol::lease_class(self.as_object())
+        debug_assert!(crate::pyobject::TypeProtocol::lease_class(zelf.as_object())
             .slots
             .flags
             .has_feature(crate::slots::PyTpFlags::HAS_DICT));
         init_module_dict(
             vm,
-            &self.as_object().dict().unwrap(),
+            &zelf.as_object().dict().unwrap(),
             name.into_object(),
             doc.flatten().into_pyobject(vm),
         );
         Ok(())
     }
 
-    fn name(self, vm: &VirtualMachine) -> Option<String> {
+    fn name(zelf: PyRef<Self>, vm: &VirtualMachine) -> Option<String> {
         vm.generic_getattribute_opt(
-            self.as_object().clone(),
+            zelf.as_object().clone(),
             PyStr::from("__name__").into_ref(vm),
             None,
         )
@@ -80,10 +81,28 @@ impl PyModuleRef {
     }
 
     #[pymethod(magic)]
-    fn getattribute(self, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        vm.generic_getattribute_opt(self.as_object().clone(), name.clone(), None)?
+    fn repr(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult {
+        let importlib = vm.import("_frozen_importlib", &[], 0)?;
+        let module_repr = vm.get_attribute(importlib, "_module_repr")?;
+        vm.invoke(&module_repr, vec![zelf.into_object()])
+    }
+
+    #[pymethod(magic)]
+    fn dir(zelf: PyRef<Self>, vm: &VirtualMachine) -> PyResult<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(vm.ctx.new_list(attrs))
+    }
+}
+
+impl SlotGetattro for PyModule {
+    fn getattro(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        vm.generic_getattribute_opt(zelf.as_object().clone(), name.clone(), None)?
             .ok_or_else(|| {
-                let module_name = if let Some(name) = self.name(vm) {
+                let module_name = if let Some(name) = Self::name(zelf, vm) {
                     format!(" '{}'", name)
                 } else {
                     "".to_owned()
@@ -93,25 +112,8 @@ impl PyModuleRef {
                 )
             })
     }
-
-    #[pymethod(magic)]
-    fn repr(self, vm: &VirtualMachine) -> PyResult {
-        let importlib = vm.import("_frozen_importlib", &[], 0)?;
-        let module_repr = vm.get_attribute(importlib, "_module_repr")?;
-        vm.invoke(&module_repr, vec![self.into_object()])
-    }
-
-    #[pymethod(magic)]
-    fn dir(self, vm: &VirtualMachine) -> PyResult<PyObjectRef> {
-        let dict = self
-            .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(vm.ctx.new_list(attrs))
-    }
 }
 
 pub(crate) fn init(context: &PyContext) {
-    PyModuleRef::extend_class(&context, &context.types.module_type);
+    PyModule::extend_class(&context, &context.types.module_type);
 }

vm/src/obj/objobject.rs

@@ -242,8 +242,9 @@ impl PyBaseObject {
         }
     }
 
-    #[pymethod(magic)]
-    fn getattribute(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+    #[pymethod(name = "__getattribute__")]
+    #[pyslot]
+    fn getattro(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
         vm_trace!("object.__getattribute__({:?}, {:?})", obj, name);
         vm.generic_getattribute(obj, name)
     }

vm/src/obj/objsuper.rs

@@ -14,7 +14,7 @@ use crate::pyobject::{
     TryFromObject, TypeProtocol,
 };
 use crate::scope::NameProtocol;
-use crate::slots::SlotDescriptor;
+use crate::slots::{SlotDescriptor, SlotGetattro};
 use crate::vm::VirtualMachine;
 
 use itertools::Itertools;
@@ -34,7 +34,7 @@ impl PyValue for PySuper {
     }
 }
 
-#[pyimpl(with(SlotDescriptor))]
+#[pyimpl(with(SlotGetattro, SlotDescriptor))]
 impl PySuper {
     fn new(typ: PyTypeRef, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<Self> {
         let obj = if vm.is_none(&obj) {
@@ -55,29 +55,6 @@ impl PySuper {
         }
     }
 
-    #[pymethod(name = "__getattribute__")]
-    fn getattribute(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        let (inst, obj_type) = match zelf.obj.clone() {
-            Some(o) => o,
-            None => return vm.generic_getattribute(zelf.into_object(), name),
-        };
-        // skip the classes in obj_type.mro up to and including zelf.typ
-        let mut it = obj_type.iter_mro().peekable();
-        for _ in it.peeking_take_while(|cls| !cls.is(&zelf.typ)) {}
-        for cls in it.skip(1) {
-            if let Some(descr) = cls.get_attr(name.borrow_value()) {
-                return vm
-                    .call_get_descriptor_specific(
-                        descr.clone(),
-                        if inst.is(&obj_type) { None } else { Some(inst) },
-                        Some(obj_type.clone().into_object()),
-                    )
-                    .unwrap_or(Ok(descr));
-            }
-        }
-        vm.generic_getattribute(zelf.into_object(), name)
-    }
-
     #[pyslot]
     fn tp_new(
         cls: PyTypeRef,
@@ -130,6 +107,30 @@ impl PySuper {
     }
 }
 
+impl SlotGetattro for PySuper {
+    fn getattro(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        let (inst, obj_type) = match zelf.obj.clone() {
+            Some(o) => o,
+            None => return vm.generic_getattribute(zelf.into_object(), name),
+        };
+        // skip the classes in obj_type.mro up to and including zelf.typ
+        let mut it = obj_type.iter_mro().peekable();
+        for _ in it.peeking_take_while(|cls| !cls.is(&zelf.typ)) {}
+        for cls in it.skip(1) {
+            if let Some(descr) = cls.get_attr(name.borrow_value()) {
+                return vm
+                    .call_get_descriptor_specific(
+                        descr.clone(),
+                        if inst.is(&obj_type) { None } else { Some(inst) },
+                        Some(obj_type.clone().into_object()),
+                    )
+                    .unwrap_or(Ok(descr));
+            }
+        }
+        vm.generic_getattribute(zelf.into_object(), name)
+    }
+}
+
 impl SlotDescriptor for PySuper {
     fn descr_get(
         zelf: PyObjectRef,

vm/src/obj/objtype.rs

@@ -17,7 +17,7 @@ use crate::pyobject::{
     BorrowValue, Either, IdProtocol, PyAttributes, PyClassImpl, PyContext, PyIterable, PyLease,
     PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, TypeProtocol,
 };
-use crate::slots::{self, Callable, PyTpFlags, PyTypeSlots};
+use crate::slots::{self, Callable, PyTpFlags, PyTypeSlots, SlotGetattro};
 use crate::vm::VirtualMachine;
 use itertools::Itertools;
 use std::ops::Deref;
@@ -165,7 +165,14 @@ impl PyTypeRef {
                 } as _;
                 self.slots.cmp.store(Some(func))
             }
-            _ => (),
+            "__getattribute__" => {
+                let func: slots::GetattroFunc = |zelf, name, vm| {
+                    let magic = get_class_magic(&zelf, "__getattribute__");
+                    vm.invoke(&magic, vec![zelf, name.into_object()])
+                };
+                self.slots.getattro.store(Some(func))
+            }
+            _ => {}
         }
     }
 }
@@ -180,7 +187,7 @@ fn get_class_magic(zelf: &PyObjectRef, name: &str) -> PyObjectRef {
     // attrs.get(name).unwrap().clone()
 }
 
-#[pyimpl(with(Callable), flags(BASETYPE))]
+#[pyimpl(with(SlotGetattro, Callable), flags(BASETYPE))]
 impl PyType {
     #[pyproperty(name = "__mro__")]
     fn get_mro(zelf: PyRef<Self>) -> PyTuple {
@@ -266,53 +273,6 @@ impl PyType {
         vm.ctx.new_dict()
     }
 
-    #[pymethod(magic)]
-    fn getattribute(zelf: PyRef<Self>, name_ref: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        let name = name_ref.borrow_value();
-        vm_trace!("type.__getattribute__({:?}, {:?})", zelf, name);
-        let mcl = zelf.lease_class();
-
-        if let Some(attr) = mcl.get_attr(&name) {
-            let attr_class = attr.lease_class();
-            if attr_class.has_attr("__set__") {
-                if let Some(ref descr_get) =
-                    PyLease::into_pyref(attr_class).first_in_mro(|cls| cls.slots.descr_get.load())
-                {
-                    let mcl = PyLease::into_pyref(mcl).into_object();
-                    return descr_get(attr, Some(zelf.into_object()), Some(mcl), vm);
-                }
-            }
-        }
-
-        if let Some(attr) = zelf.get_attr(&name) {
-            let attr_class = attr.class();
-            if let Some(ref descr_get) = attr_class.first_in_mro(|cls| cls.slots.descr_get.load()) {
-                drop(mcl);
-                return descr_get(attr, None, Some(zelf.into_object()), vm);
-            }
-        }
-
-        if let Some(cls_attr) = zelf.get_attr(&name) {
-            Ok(cls_attr)
-        } else if let Some(attr) = mcl.get_attr(&name) {
-            drop(mcl);
-            vm.call_if_get_descriptor(attr, zelf.into_object())
-        } else if let Some(ref getter) = zelf.get_attr("__getattr__") {
-            vm.invoke(
-                getter,
-                vec![
-                    PyLease::into_pyref(mcl).into_object(),
-                    name_ref.into_object(),
-                ],
-            )
-        } else {
-            Err(vm.new_attribute_error(format!(
-                "type object '{}' has no attribute '{}'",
-                zelf, name
-            )))
-        }
-    }
-
     #[pymethod(magic)]
     fn setattr(
         zelf: PyRef<Self>,
@@ -510,6 +470,58 @@ impl PyType {
     }
 }
 
+impl SlotGetattro for PyType {
+    fn getattro(zelf: PyRef<Self>, name_ref: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        let name = name_ref.borrow_value();
+        vm_trace!("type.__getattribute__({:?}, {:?})", zelf, name);
+        let mcl = zelf.lease_class();
+
+        let mcl_attr = mcl.get_attr(name);
+
+        if let Some(ref attr) = mcl_attr {
+            let attr_class = attr.lease_class();
+            if attr_class.has_attr("__set__") {
+                if let Some(ref descr_get) =
+                    PyLease::into_pyref(attr_class).first_in_mro(|cls| cls.slots.descr_get.load())
+                {
+                    let mcl = PyLease::into_pyref(mcl).into_object();
+                    return descr_get(attr.clone(), Some(zelf.into_object()), Some(mcl), vm);
+                }
+            }
+        }
+
+        let zelf_attr = zelf.get_attr(name);
+
+        if let Some(ref attr) = zelf_attr {
+            let attr_class = attr.class();
+            if let Some(descr_get) = attr_class.first_in_mro(|cls| cls.slots.descr_get.load()) {
+                drop(mcl);
+                return descr_get(attr.clone(), None, Some(zelf.into_object()), vm);
+            }
+        }
+
+        if let Some(cls_attr) = zelf_attr {
+            Ok(cls_attr)
+        } else if let Some(attr) = mcl_attr {
+            drop(mcl);
+            vm.call_if_get_descriptor(attr, zelf.into_object())
+        } else if let Some(ref getter) = zelf.get_attr("__getattr__") {
+            vm.invoke(
+                getter,
+                vec![
+                    PyLease::into_pyref(mcl).into_object(),
+                    name_ref.into_object(),
+                ],
+            )
+        } else {
+            Err(vm.new_attribute_error(format!(
+                "type object '{}' has no attribute '{}'",
+                zelf, name
+            )))
+        }
+    }
+}
+
 impl Callable for PyType {
     fn call(zelf: &PyRef<Self>, args: PyFuncArgs, vm: &VirtualMachine) -> PyResult {
         vm_trace!("type_call: {:?}", zelf);

vm/src/pyobject.rs

@@ -27,7 +27,7 @@ use crate::obj::objiter;
 use crate::obj::objlist::PyList;
 use crate::obj::objnamespace::PyNamespace;
 use crate::obj::objobject;
-use crate::obj::objset::PySet;
+use crate::obj::objset;
 use crate::obj::objsingletons::{PyNone, PyNoneRef, PyNotImplemented, PyNotImplementedRef};
 use crate::obj::objslice::PyEllipsis;
 use crate::obj::objstaticmethod::PyStaticMethod;
@@ -256,10 +256,10 @@ impl PyContext {
         PyObject::new(PyList::from(elements), self.types.list_type.clone(), None)
     }
 
-    pub fn new_set(&self) -> PyObjectRef {
+    pub fn new_set(&self) -> objset::PySetRef {
         // Initialized empty, as calling __hash__ is required for adding each object to the set
         // which requires a VM context - this is done in the objset code itself.
-        PyObject::new(PySet::default(), self.types.set_type.clone(), None)
+        PyRef::new_ref(objset::PySet::default(), self.types.set_type.clone(), None)
     }
 
     pub fn new_dict(&self) -> PyDictRef {

vm/src/scope.rs

@@ -45,10 +45,10 @@ impl Scope {
         Scope::new(locals, globals, vm)
     }
 
-    pub fn get_locals(&self) -> PyDictRef {
+    pub fn get_locals(&self) -> &PyDictRef {
         match self.locals.first() {
-            Some(dict) => dict.clone(),
-            None => self.globals.clone(),
+            Some(dict) => dict,
+            None => &self.globals,
         }
     }
 

vm/src/slots.rs

@@ -3,6 +3,7 @@ use std::cmp::Ordering;
 use crate::common::cell::PyRwLock;
 use crate::common::hash::PyHash;
 use crate::function::{OptionalArg, PyFuncArgs, PyNativeFunc};
+use crate::obj::objstr::PyStrRef;
 use crate::pyobject::{
     Either, IdProtocol, PyComparisonValue, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject,
 };
@@ -51,6 +52,7 @@ pub(crate) type CmpFunc = fn(
     PyComparisonOp,
     &VirtualMachine,
 ) -> PyResult<Either<PyObjectRef, PyComparisonValue>>;
+pub(crate) type GetattroFunc = fn(PyObjectRef, PyStrRef, &VirtualMachine) -> PyResult;
 
 #[derive(Default)]
 pub struct PyTypeSlots {
@@ -62,6 +64,7 @@ pub struct PyTypeSlots {
     pub descr_get: AtomicCell<Option<DescrGetFunc>>,
     pub hash: AtomicCell<Option<HashFunc>>,
     pub cmp: AtomicCell<Option<CmpFunc>>,
+    pub getattro: AtomicCell<Option<GetattroFunc>>,
 }
 
 impl PyTypeSlots {
@@ -379,3 +382,23 @@ impl PyComparisonOp {
         }
     }
 }
+
+#[pyimpl]
+pub trait SlotGetattro: PyValue {
+    #[pyslot]
+    fn tp_getattro(obj: PyObjectRef, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        if let Ok(zelf) = obj.downcast::<Self>() {
+            Self::getattro(zelf, name, vm)
+        } else {
+            Err(vm.new_type_error("unexpected payload for __getattribute__".to_owned()))
+        }
+    }
+
+    // TODO: make zelf: &PyRef<Self>
+    fn getattro(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult;
+
+    #[pymethod]
+    fn __getattribute__(zelf: PyRef<Self>, name: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        Self::getattro(zelf, name, vm)
+    }
+}

vm/src/stdlib/thread.rs

@@ -9,6 +9,7 @@ use crate::pyobject::{
     BorrowValue, Either, IdProtocol, ItemProtocol, PyCallable, PyClassImpl, PyObjectRef, PyRef,
     PyResult, PyValue, TypeProtocol,
 };
+use crate::slots::SlotGetattro;
 use crate::vm::VirtualMachine;
 
 use parking_lot::{
@@ -296,7 +297,7 @@ impl PyValue for PyLocal {
     }
 }
 
-#[pyimpl(flags(BASETYPE))]
+#[pyimpl(with(SlotGetattro), flags(BASETYPE))]
 impl PyLocal {
     fn ldict(&self, vm: &VirtualMachine) -> PyDictRef {
         self.data.get_or(|| vm.ctx.new_dict()).clone()
@@ -310,20 +311,6 @@ impl PyLocal {
         .into_ref_with_type(vm, cls)
     }
 
-    #[pymethod(magic)]
-    fn getattribute(zelf: PyRef<Self>, attr: PyStrRef, vm: &VirtualMachine) -> PyResult {
-        let ldict = zelf.ldict(vm);
-        if attr.borrow_value() == "__dict__" {
-            Ok(ldict.into_object())
-        } else {
-            let zelf = zelf.into_object();
-            vm.generic_getattribute_opt(zelf.clone(), attr.clone(), Some(ldict))?
-                .ok_or_else(|| {
-                    vm.new_attribute_error(format!("{} has no attribute '{}'", zelf, attr))
-                })
-        }
-    }
-
     #[pymethod(magic)]
     fn setattr(
         zelf: PyRef<Self>,
@@ -356,6 +343,21 @@ impl PyLocal {
     }
 }
 
+impl SlotGetattro for PyLocal {
+    fn getattro(zelf: PyRef<Self>, attr: PyStrRef, vm: &VirtualMachine) -> PyResult {
+        let ldict = zelf.ldict(vm);
+        if attr.borrow_value() == "__dict__" {
+            Ok(ldict.into_object())
+        } else {
+            let zelf = zelf.into_object();
+            vm.generic_getattribute_opt(zelf.clone(), attr.clone(), Some(ldict))?
+                .ok_or_else(|| {
+                    vm.new_attribute_error(format!("{} has no attribute '{}'", zelf, attr))
+                })
+        }
+    }
+}
+
 pub fn make_module(vm: &VirtualMachine) -> PyObjectRef {
     let ctx = &vm.ctx;
 

vm/src/types.rs

@@ -180,7 +180,7 @@ impl TypeZoo {
             map_type: create_type!(objmap::PyMap),
             mappingproxy_type: create_type!(objmappingproxy::PyMappingProxy),
             memoryview_type: create_type!(objmemory::PyMemoryView),
-            module_type: create_type!(objmodule::PyModuleRef),
+            module_type: create_type!(objmodule::PyModule),
             namespace_type: create_type!(objnamespace::PyNamespace),
             property_type: create_type!(objproperty::PyProperty),
             range_type: create_type!(objrange::PyRange),

vm/src/vm.rs

@@ -700,7 +700,7 @@ impl VirtualMachine {
     }
 
     pub fn get_locals(&self) -> PyDictRef {
-        self.current_scope().get_locals()
+        self.current_scope().get_locals().clone()
     }
 
     // Container of the virtual machine state:
@@ -783,7 +783,7 @@ impl VirtualMachine {
 
                 let (locals, globals) = if let Some(frame) = self.current_frame() {
                     (
-                        frame.scope.get_locals().into_object(),
+                        frame.scope.get_locals().clone().into_object(),
                         frame.scope.globals.clone().into_object(),
                     )
                 } else {
@@ -971,7 +971,11 @@ impl VirtualMachine {
     {
         let attr_name = attr_name.try_into_ref(self)?;
         vm_trace!("vm.__getattribute__: {:?} {:?}", obj, attr_name);
-        self.call_method(&obj, "__getattribute__", vec![attr_name.into_object()])
+        let getattro = obj
+            .class()
+            .first_in_mro(|cls| cls.slots.getattro.load())
+            .unwrap();
+        getattro(obj, attr_name, self)
     }
 
     pub fn set_attr<K, V>(&self, obj: &PyObjectRef, attr_name: K, attr_value: V) -> PyResult
@@ -1083,9 +1087,11 @@ impl VirtualMachine {
         let name = name_str.borrow_value();
         let cls = obj.class();
 
-        if let Some(attr) = cls.get_attr(&name) {
+        let cls_attr = cls.get_attr(name);
+
+        if let Some(ref attr) = cls_attr {
             if attr.lease_class().has_attr("__set__") {
-                if let Some(r) = self.call_get_descriptor(attr, obj.clone()) {
+                if let Some(r) = self.call_get_descriptor(attr.clone(), obj.clone()) {
                     return r.map(Some);
                 }
             }
@@ -1101,7 +1107,7 @@ impl VirtualMachine {
 
         if let Some(obj_attr) = attr {
             Ok(Some(obj_attr))
-        } else if let Some(attr) = cls.get_attr(&name) {
+        } else if let Some(attr) = cls_attr {
             self.call_if_get_descriptor(attr, obj).map(Some)
         } else if let Some(getter) = cls.get_attr("__getattr__") {
             self.invoke(&getter, vec![obj, name_str.into_object()])