From 76da6a0f3ffb8a9246e0d132ca2ff69d8227c81e Mon Sep 17 00:00:00 2001 From: Stefan Marr Date: Mon, 12 Aug 2024 10:16:44 +0100 Subject: [PATCH 1/2] Improve VMString inlining and optimize strEqual Signed-off-by: Stefan Marr --- src/primitives/String.cpp | 21 +++++++++++++-------- src/vmobjects/VMString.cpp | 19 ------------------- src/vmobjects/VMString.h | 27 ++++++++++++++++++--------- 3 files changed, 31 insertions(+), 36 deletions(-) diff --git a/src/primitives/String.cpp b/src/primitives/String.cpp index 6999ccc1..e6a6e9f3 100644 --- a/src/primitives/String.cpp +++ b/src/primitives/String.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include "../misc/defs.h" @@ -74,7 +75,7 @@ static vm_oop_t strLength(vm_oop_t rcvr) { } static vm_oop_t strEqual(vm_oop_t leftObj, vm_oop_t op1) { - VMString* op2 = static_cast(leftObj); + VMString* left = static_cast(leftObj); if (IS_TAGGED(op1)) { return load_ptr(falseObject); @@ -85,14 +86,18 @@ static vm_oop_t strEqual(vm_oop_t leftObj, vm_oop_t op1) { } VMClass* otherClass = CLASS_OF(op1); - if (otherClass == load_ptr(stringClass) || - otherClass == load_ptr(symbolClass)) { - StdString s1 = static_cast(op1)->GetStdString(); - StdString s2 = op2->GetStdString(); + if (otherClass != load_ptr(stringClass) && + otherClass != load_ptr(symbolClass)) { + return load_ptr(falseObject); + } - if (s1 == s2) { - return load_ptr(trueObject); - } + VMString* right = static_cast(op1); + if (left->length != right->length) { + return load_ptr(falseObject); + } + + if (strncmp(left->chars, right->chars, left->length) == 0) { + return load_ptr(trueObject); } return load_ptr(falseObject); } diff --git a/src/vmobjects/VMString.cpp b/src/vmobjects/VMString.cpp index 0968e741..77ce6864 100644 --- a/src/vmobjects/VMString.cpp +++ b/src/vmobjects/VMString.cpp @@ -43,21 +43,6 @@ VMString* VMString::CloneForMovingGC() const { VMString(length, chars); } -void VMString::MarkObjectAsInvalid() { - for (size_t i = 0; i < length; i++) { - chars[i] = 'z'; - } - chars = (char*)INVALID_GC_POINTER; -} - -bool VMString::IsMarkedInvalid() const { - return chars == (char*)INVALID_GC_POINTER; -} - -void VMString::WalkObjects(walk_heap_fn) { - // nothing to do -} - size_t VMString::GetObjectSize() const { size_t size = sizeof(VMString) + PADDED_SIZE(length); return size; @@ -67,10 +52,6 @@ VMClass* VMString::GetClass() const { return load_ptr(stringClass); } -size_t VMString::GetStringLength() const { - return length; -} - std::string VMString::GetStdString() const { if (chars == 0) { return std::string(""); diff --git a/src/vmobjects/VMString.h b/src/vmobjects/VMString.h index 3401ab86..23836eae 100644 --- a/src/vmobjects/VMString.h +++ b/src/vmobjects/VMString.h @@ -52,17 +52,30 @@ class VMString : public AbstractVMObject { return (int64_t)hash; } - inline char* GetRawChars() const; + inline char* GetRawChars() const { return chars; } + StdString GetStdString() const; - size_t GetStringLength() const; + + inline size_t GetStringLength() const { return length; } VMString* CloneForMovingGC() const override; VMClass* GetClass() const override; size_t GetObjectSize() const override; - void WalkObjects(walk_heap_fn) override; - void MarkObjectAsInvalid() override; - bool IsMarkedInvalid() const override; + inline void WalkObjects(walk_heap_fn) override { + // nothing to do + } + + void MarkObjectAsInvalid() override { + for (size_t i = 0; i < length; i++) { + chars[i] = 'z'; + } + chars = (char*)INVALID_GC_POINTER; + } + + bool IsMarkedInvalid() const override { + return chars == (char*)INVALID_GC_POINTER; + } StdString AsDebugString() const override; @@ -80,7 +93,3 @@ class VMString : public AbstractVMObject { length(length), chars(adaptedCharsPointer) {}; // constructor to use by VMSymbol }; - -char* VMString::GetRawChars() const { - return chars; -} From 1e17f763dd1f34d0f99c14eb403d0eedd6e27452 Mon Sep 17 00:00:00 2001 From: Stefan Marr Date: Mon, 12 Aug 2024 10:17:59 +0100 Subject: [PATCH 2/2] Avoid nil-ing out VMArrays twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s done in VMObject already Signed-off-by: Stefan Marr --- src/vmobjects/VMArray.cpp | 8 -------- src/vmobjects/VMArray.h | 7 ++++++- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/vmobjects/VMArray.cpp b/src/vmobjects/VMArray.cpp index 210eede8..b94c7cd3 100644 --- a/src/vmobjects/VMArray.cpp +++ b/src/vmobjects/VMArray.cpp @@ -40,14 +40,6 @@ const size_t VMArray::VMArrayNumberOfFields = 0; -VMArray::VMArray(size_t arraySize, size_t additionalBytes) - : VMObject( - arraySize + 0 /* VMArray is not allowed to have any fields itself */, - additionalBytes + sizeof(VMArray)) { - assert(VMArrayNumberOfFields == 0); - nilInitializeFields(); -} - VMArray* VMArray::Copy() const { VMArray* copy = Universe::NewArray(GetNumberOfIndexableFields()); diff --git a/src/vmobjects/VMArray.h b/src/vmobjects/VMArray.h index 1467e95f..37f08121 100644 --- a/src/vmobjects/VMArray.h +++ b/src/vmobjects/VMArray.h @@ -39,7 +39,12 @@ class VMArray : public VMObject { public: typedef GCArray Stored; - explicit VMArray(size_t arraySize, size_t additionalBytes); + explicit VMArray(size_t arraySize, size_t additionalBytes) + : VMObject(arraySize + + 0 /* VMArray is not allowed to have any fields itself */, + additionalBytes + sizeof(VMArray)) { + assert(VMArrayNumberOfFields == 0); + } // VMArray doesn't need to customize `void WalkObjects(walk_heap_fn)`, // because it doesn't need anything special.