Make VMFrame an AbstractVMObject and allow for more inlining (#54)

AbstractVMObject is simpler than VMObject, avoids some fields and other bits.

The rest is moving some bits to headers to allow the compilers to inline.

src/unitTests/CloneObjectsTest.cpp

@@ -215,11 +215,8 @@ void CloneObjectsTest::testCloneFrame() {
     VMFrame* clone = orig->CloneForMovingGC();
 
     CPPUNIT_ASSERT((intptr_t)orig != (intptr_t)clone);
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("class differs!!", orig->clazz, clone->clazz);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("objectSize differs!!", orig->totalObjectSize,
                                  clone->totalObjectSize);
-    CPPUNIT_ASSERT_EQUAL_MESSAGE("numberOfFields differs!!",
-                                 orig->numberOfFields, clone->numberOfFields);
     CPPUNIT_ASSERT_EQUAL_MESSAGE("GetPreviousFrame differs!!",
                                  orig->GetPreviousFrame(),
                                  clone->GetPreviousFrame());

src/vmobjects/ObjectFormats.h

@@ -141,7 +141,7 @@ typedef GCOop* gc_oop_t;
 // clang-format off
 class GCAbstractObject : public GCOop            { public: typedef AbstractVMObject Loaded; };
 class GCObject         : public GCAbstractObject { public: typedef VMObject         Loaded; };
-class GCFrame          : public GCObject         { public: typedef VMFrame          Loaded; };
+class GCFrame          : public GCAbstractObject { public: typedef VMFrame          Loaded; };
 class GCClass          : public GCObject         { public: typedef VMClass          Loaded; };
 class GCArray          : public GCObject         { public: typedef VMArray          Loaded; };
 class GCBlock          : public GCObject         { public: typedef VMBlock          Loaded; };

src/vmobjects/VMArray.cpp

@@ -48,26 +48,6 @@ VMArray::VMArray(size_t arraySize, size_t additionalBytes)
     nilInitializeFields();
 }
 
-vm_oop_t VMArray::GetIndexableField(size_t idx) const {
-    if (unlikely(idx > GetNumberOfIndexableFields())) {
-        ErrorExit(("Array index out of bounds: Accessing " + to_string(idx) +
-                   ", but array size is only " +
-                   to_string(GetNumberOfIndexableFields()) + "\n")
-                      .c_str());
-    }
-    return GetField(idx);
-}
-
-void VMArray::SetIndexableField(size_t idx, vm_oop_t value) {
-    if (unlikely(idx > GetNumberOfIndexableFields())) {
-        ErrorExit(("Array index out of bounds: Accessing " + to_string(idx) +
-                   ", but array size is only " +
-                   to_string(GetNumberOfIndexableFields()) + "\n")
-                      .c_str());
-    }
-    SetField(idx, value);
-}
-
 VMArray* VMArray::Copy() const {
     VMArray* copy = Universe::NewArray(GetNumberOfIndexableFields());
 
@@ -97,6 +77,12 @@ VMArray* VMArray::CloneForMovingGC() const {
     return clone;
 }
 
+void VMArray::IndexOutOfBounds(size_t idx) const {
+    ErrorExit(("Array index out of bounds: Accessing " + to_string(idx) +
+               ", but array size is only " + to_string(numberOfFields) + "\n")
+                  .c_str());
+}
+
 void VMArray::CopyIndexableFieldsTo(VMArray* to) const {
     const size_t numIndexableFields = GetNumberOfIndexableFields();
     for (size_t i = 0; i < numIndexableFields; ++i) {

src/vmobjects/VMArray.h

@@ -50,8 +50,24 @@ class VMArray : public VMObject {
     VMArray* Copy() const;
 
     VMArray* CopyAndExtendWith(vm_oop_t) const;
-    vm_oop_t GetIndexableField(size_t idx) const;
-    void SetIndexableField(size_t idx, vm_oop_t value);
+
+    inline vm_oop_t GetIndexableField(size_t idx) const {
+        if (unlikely(idx > numberOfFields)) {
+            IndexOutOfBounds(idx);
+        }
+        return GetField(idx);
+    }
+
+    inline void SetIndexableField(size_t idx, vm_oop_t value) {
+        if (unlikely(idx > GetNumberOfIndexableFields())) {
+            IndexOutOfBounds(idx);
+        }
+        SetField(idx, value);
+    }
+
+    __attribute__((noreturn)) __attribute__((noinline)) void IndexOutOfBounds(
+        size_t idx) const;
+
     void CopyIndexableFieldsTo(VMArray*) const;
     VMArray* CloneForMovingGC() const override;
 

src/vmobjects/VMFrame.cpp

@@ -55,8 +55,6 @@ VMFrame* VMFrame::EmergencyFrameFrom(VMFrame* from, long extraLength) {
     VMFrame* result = new (GetHeap<HEAP_CLS>(), additionalBytes)
         VMFrame(additionalBytes, method, from->GetPreviousFrame());
 
-    result->clazz = nullptr;  // result->SetClass(from->GetClass());
-
     // set Frame members
     result->SetContext(from->GetContext());
     result->stack_ptr =

src/vmobjects/VMFrame.h

@@ -31,7 +31,7 @@
 
 class Universe;
 
-class VMFrame : public VMObject {
+class VMFrame : public AbstractVMObject {
     friend class Universe;
     friend class Interpreter;
     friend class Shell;
@@ -44,7 +44,7 @@ class VMFrame : public VMObject {
 
     explicit VMFrame(size_t additionalBytes, VMMethod* method,
                      VMFrame* previousFrame)
-        : VMObject(0, additionalBytes + sizeof(VMFrame)), bytecodeIndex(0),
+        : totalObjectSize(additionalBytes + sizeof(VMFrame)), bytecodeIndex(0),
           previousFrame(store_root(previousFrame)), context(nullptr),
           method(store_root(method)), arguments((gc_oop_t*)&(stack_ptr) + 1),
           locals(arguments + method->GetNumberOfArguments()),
@@ -60,6 +60,20 @@ class VMFrame : public VMObject {
         }
     }
 
+    int64_t GetHash() const final { return 0; /* should never be called */ }
+
+    inline VMClass* GetClass() const final { return nullptr; }
+
+    inline size_t GetObjectSize() const final { return totalObjectSize; }
+
+    void MarkObjectAsInvalid() final {
+        previousFrame = (GCFrame*)INVALID_GC_POINTER;
+    }
+
+    bool IsMarkedInvalid() const final {
+        return previousFrame == (GCFrame*)INVALID_GC_POINTER;
+    }
+
     inline VMFrame* GetPreviousFrame() const;
     inline void ClearPreviousFrame();
     inline bool HasPreviousFrame() const;
@@ -156,6 +170,7 @@ class VMFrame : public VMObject {
     make_testable(public);
 
     long bytecodeIndex;
+    size_t totalObjectSize;
 
 private:
     GCFrame* previousFrame;

src/vmobjects/VMInvokable.cpp

@@ -31,27 +31,18 @@
 #include "../vm/Print.h"
 #include "ObjectFormats.h"
 #include "VMClass.h"
-#include "VMSymbol.h"
 
 bool VMInvokable::IsPrimitive() const {
     return false;
 }
 
-VMSymbol* VMInvokable::GetSignature() const {
-    return load_ptr(signature);
-}
-
 void VMInvokable::WalkObjects(walk_heap_fn walk) {
     signature = static_cast<GCSymbol*>(walk(signature));
     if (holder) {
         holder = static_cast<GCClass*>(walk(holder));
     }
 }
 
-VMClass* VMInvokable::GetHolder() const {
-    return load_ptr(holder);
-}
-
 void VMInvokable::SetHolder(VMClass* hld) {
     store_ptr(holder, hld);
 }

src/vmobjects/VMInvokable.h

@@ -58,8 +58,11 @@ class VMInvokable : public AbstractVMObject {
     virtual size_t GetNumberOfArguments() const = 0;
 
     virtual bool IsPrimitive() const;
-    VMSymbol* GetSignature() const;
-    VMClass* GetHolder() const;
+
+    inline VMSymbol* GetSignature() const { return load_ptr(signature); }
+
+    VMClass* GetHolder() const { return load_ptr(holder); }
+
     virtual void SetHolder(VMClass* hld);
 
     void WalkObjects(walk_heap_fn) override;

src/vmobjects/VMObject.cpp

@@ -43,19 +43,6 @@
 // clazz is the only field of VMObject so
 const size_t VMObject::VMObjectNumberOfFields = 0;
 
-VMObject::VMObject(size_t numSubclassFields, size_t totalObjectSize)
-    : totalObjectSize(totalObjectSize),
-      numberOfFields(VMObjectNumberOfFields + numSubclassFields) {
-    assert(IS_PADDED_SIZE(totalObjectSize));
-    assert(totalObjectSize >= sizeof(VMObject));
-
-    // this line would be needed if the VMObject** is used instead of the macro:
-    // FIELDS = (VMObject**)&clazz;
-    hash = (size_t)this;
-
-    nilInitializeFields();
-}
-
 VMObject* VMObject::CloneForMovingGC() const {
     VMObject* clone =
         new (GetHeap<HEAP_CLS>(),

src/vmobjects/VMObject.h

@@ -64,14 +64,32 @@ class VMObject : public AbstractVMObject {
     /**
      * numberOfFields - including
      */
-    explicit VMObject(size_t numSubclassFields, size_t totalObjectSize);
+    explicit VMObject(size_t numSubclassFields, size_t totalObjectSize)
+        : totalObjectSize(totalObjectSize),
+          numberOfFields(VMObjectNumberOfFields + numSubclassFields) {
+        assert(IS_PADDED_SIZE(totalObjectSize));
+        assert(totalObjectSize >= sizeof(VMObject));
+
+        // this line would be needed if the VMObject** is used instead of the
+        // macro: FIELDS = (VMObject**)&clazz;
+        hash = (size_t)this;
+
+        nilInitializeFields();
+    }
+
     ~VMObject() override = default;
 
     int64_t GetHash() const override { return hash; }
-    inline VMClass* GetClass() const override;
+
+    inline VMClass* GetClass() const override {
+        assert(IsValidObject((VMObject*)load_ptr(clazz)));
+        return load_ptr(clazz);
+    }
+
     void SetClass(VMClass* cl) override;
     VMSymbol* GetFieldName(long index) const override;
-    inline long GetNumberOfFields() const override;
+
+    inline long GetNumberOfFields() const override { return numberOfFields; }
 
     inline vm_oop_t GetField(size_t index) const {
         assert(numberOfFields > index);
@@ -121,12 +139,3 @@ class VMObject : public AbstractVMObject {
 private:
     static const size_t VMObjectNumberOfFields;
 };
-
-VMClass* VMObject::GetClass() const {
-    assert(IsValidObject((VMObject*)load_ptr(clazz)));
-    return load_ptr(clazz);
-}
-
-long VMObject::GetNumberOfFields() const {
-    return numberOfFields;
-}

src/vmobjects/VMString.cpp

@@ -31,23 +31,13 @@
 
 #include "../memory/Heap.h"
 #include "../misc/defs.h"
-#include "AbstractObject.h"
 #include "ObjectFormats.h"
 
 extern GCClass* stringClass;
 
 // this macro could replace the chars member variable
 // #define CHARS ((char*)&clazz+sizeof(VMObject*))
 
-VMString::VMString(const size_t length, const char* str)
-    : AbstractVMObject(), length(length),
-      // set the chars-pointer to point at the position of the first character
-      chars((char*)&chars + sizeof(char*)) {
-    for (size_t i = 0; i < length; i++) {
-        chars[i] = str[i];
-    }
-}
-
 VMString* VMString::CloneForMovingGC() const {
     return new (GetHeap<HEAP_CLS>(), PADDED_SIZE(length) ALLOC_MATURE)
         VMString(length, chars);

src/vmobjects/VMString.h

@@ -32,7 +32,15 @@ class VMString : public AbstractVMObject {
 public:
     typedef GCString Stored;
 
-    VMString(const size_t length, const char* str);
+    VMString(const size_t length, const char* str)
+        : AbstractVMObject(), length(length),
+          // set the chars-pointer to point at the position of the first
+          // character
+          chars((char*)&chars + sizeof(char*)) {
+        for (size_t i = 0; i < length; i++) {
+            chars[i] = str[i];
+        }
+    }
 
     int64_t GetHash() const override {
         uint64_t hash = 5381U;

src/vmobjects/VMSymbol.cpp

@@ -83,17 +83,6 @@ std::string VMSymbol::AsDebugString() const {
     return "Symbol(" + GetStdString() + ")";
 }
 
-VMInvokable* VMSymbol::GetCachedInvokable(const VMClass* cls) const {
-    if (cls == load_ptr(cachedClass_invokable[0])) {
-        return load_ptr(cachedInvokable[0]);
-    } else if (cls == load_ptr(cachedClass_invokable[1])) {
-        return load_ptr(cachedInvokable[1]);
-    } else if (cls == load_ptr(cachedClass_invokable[2])) {
-        return load_ptr(cachedInvokable[2]);
-    }
-    return nullptr;
-}
-
 void VMSymbol::UpdateCachedInvokable(const VMClass* cls, VMInvokable* invo) {
     store_ptr(cachedInvokable[nextCachePos], invo);
     store_ptr(cachedClass_invokable[nextCachePos], const_cast<VMClass*>(cls));

src/vmobjects/VMSymbol.h

@@ -28,6 +28,9 @@
 
 #include <iostream>
 
+#include "../misc/defs.h"
+#include "ObjectFormats.h"
+#include "VMClass.h"
 #include "VMObject.h"
 #include "VMString.h"
 
@@ -47,7 +50,18 @@ class VMSymbol : public VMString {
     const GCClass* cachedClass_invokable[3];
     long nextCachePos;
     GCInvokable* cachedInvokable[3];
-    VMInvokable* GetCachedInvokable(const VMClass*) const;
+
+    inline VMInvokable* GetCachedInvokable(const VMClass* cls) const {
+        if (cls == load_ptr(cachedClass_invokable[0])) {
+            return load_ptr(cachedInvokable[0]);
+        } else if (cls == load_ptr(cachedClass_invokable[1])) {
+            return load_ptr(cachedInvokable[1]);
+        } else if (cls == load_ptr(cachedClass_invokable[2])) {
+            return load_ptr(cachedInvokable[2]);
+        }
+        return nullptr;
+    }
+
     void UpdateCachedInvokable(const VMClass* cls, VMInvokable* invo);
 
     friend class Signature;