From 86094b80806edea46aa77d855accffae0be7b50c Mon Sep 17 00:00:00 2001 From: Connor McLaughlin Date: Fri, 28 Feb 2020 17:00:02 +1000 Subject: [PATCH] Common/String: Don't copy StaticStrings when not writing --- src/common/string.cpp | 8 +++++--- src/common/string.h | 34 +++++++++++++--------------------- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/common/string.cpp b/src/common/string.cpp index 8d32c8b73..6b57854af 100644 --- a/src/common/string.cpp +++ b/src/common/string.cpp @@ -124,7 +124,7 @@ static String::StringData* StringDataReallocate(String::StringData* pStringData, static bool StringDataIsSharable(const String::StringData* pStringData) { - return pStringData->ReferenceCount != -1; + return pStringData->ReadOnly || pStringData->ReferenceCount != -1; } static bool StringDataIsShared(const String::StringData* pStringData) @@ -145,7 +145,8 @@ String::String(const String& copyString) else if (StringDataIsSharable(copyString.m_pStringData)) { m_pStringData = copyString.m_pStringData; - StringDataAddRef(m_pStringData); + if (!m_pStringData->ReadOnly) + StringDataAddRef(m_pStringData); } // create a clone for ourselves else @@ -525,7 +526,8 @@ void String::Assign(const String& copyString) else if (StringDataIsSharable(copyString.m_pStringData)) { m_pStringData = copyString.m_pStringData; - StringDataAddRef(m_pStringData); + if (!m_pStringData->ReadOnly) + StringDataAddRef(m_pStringData); } // create a clone for ourselves else diff --git a/src/common/string.h b/src/common/string.h index 4543f9c49..a4a48ec6f 100644 --- a/src/common/string.h +++ b/src/common/string.h @@ -1,6 +1,6 @@ #pragma once #include "types.h" -#include +#include #include #include #include @@ -29,7 +29,7 @@ public: // Reference count of this data object. If set to -1, // it is considered noncopyable and any copies of the string // will always create their own copy. - std::atomic ReferenceCount; + s32 ReferenceCount; // Whether the memory pointed to by pBuffer is writable. bool ReadOnly; @@ -50,6 +50,9 @@ public: // Move constructor, take reference from other string. String(String&& moveString); + // Construct a string from a data object, does not increment the reference count on the string data, use carefully. + explicit String(StringData* pStringData) : m_pStringData(pStringData) {} + // Destructor. Child classes may not have any destructors, as this is not virtual. ~String(); @@ -251,10 +254,6 @@ public: bool operator>(const char* compString) const { return (NumericCompare(compString) > 0); } protected: - // Hidden constructor for creating string child classes. - // It does not increment the reference count on the string data, therefore dangerous to be public. - String(StringData* pStringData) : m_pStringData(pStringData) {} - // Internal append function. void InternalPrepend(const char* pString, u32 Length); void InternalAppend(const char* pString, u32 Length); @@ -267,21 +266,14 @@ protected: }; // static string, stored in .rodata -class StaticString : public String -{ -public: - StaticString(const char* Text) - { - m_sStringData.pBuffer = const_cast(Text); - m_sStringData.StringLength = static_cast(std::strlen(Text)); - m_sStringData.BufferSize = m_sStringData.StringLength + 1; - m_sStringData.ReadOnly = true; - m_sStringData.ReferenceCount = -1; - } - -private: - StringData m_sStringData; -}; +#define StaticString(Text) \ + []() noexcept -> String { \ + static constexpr u32 buffer_size = sizeof(Text); \ + static constexpr u32 length = buffer_size - 1; \ + static constexpr String::StringData data{const_cast(Text), length, buffer_size, static_cast(-1), \ + true}; \ + return String(const_cast(&data)); \ + }() // stack-allocated string template