From fa52f681425dbfd604b1c1635861d685a3710649 Mon Sep 17 00:00:00 2001 From: Dan Klishch Date: Sat, 28 Oct 2023 18:58:29 -0400 Subject: [PATCH] AK: Store data in FlyString as StringBase Unfortunately, it is not clear to me how to split this commit into several atomic ones. --- AK/FlyString.cpp | 62 ++++++------------------------------ AK/FlyString.h | 18 ++++------- AK/String.cpp | 76 ++++++-------------------------------------- AK/String.h | 11 ++----- AK/StringBase.cpp | 11 +++++++ AK/StringBase.h | 7 ++-- AK/StringInternals.h | 2 ++ 7 files changed, 42 insertions(+), 145 deletions(-) diff --git a/AK/FlyString.cpp b/AK/FlyString.cpp index b373cbedd8..d637289942 100644 --- a/AK/FlyString.cpp +++ b/AK/FlyString.cpp @@ -15,20 +15,10 @@ namespace AK { static auto& all_fly_strings() { - static Singleton> table; + static Singleton> table; return *table; } -FlyString::FlyString() - : m_data(String {}.to_fly_string_data({})) -{ -} - -FlyString::~FlyString() -{ - String::unref_fly_string_data({}, m_data); -} - ErrorOr FlyString::from_utf8(StringView string) { return FlyString { TRY(String::from_utf8(string)) }; @@ -37,21 +27,19 @@ ErrorOr FlyString::from_utf8(StringView string) FlyString::FlyString(String const& string) { if (string.is_short_string()) { - m_data = string.to_fly_string_data({}); + m_data = string; return; } auto it = all_fly_strings().find(string.bytes_as_string_view()); if (it == all_fly_strings().end()) { - m_data = string.to_fly_string_data({}); + m_data = string; all_fly_strings().set(string.bytes_as_string_view(), m_data); string.did_create_fly_string({}); } else { m_data = it->value; } - - String::ref_fly_string_data({}, m_data); } FlyString& FlyString::operator=(String const& string) @@ -60,36 +48,6 @@ FlyString& FlyString::operator=(String const& string) return *this; } -FlyString::FlyString(FlyString const& other) - : m_data(other.m_data) -{ - String::ref_fly_string_data({}, m_data); -} - -FlyString& FlyString::operator=(FlyString const& other) -{ - if (this != &other) { - m_data = other.m_data; - String::ref_fly_string_data({}, m_data); - } - - return *this; -} - -FlyString::FlyString(FlyString&& other) - : m_data(other.m_data) -{ - other.m_data = String {}.to_fly_string_data({}); -} - -FlyString& FlyString::operator=(FlyString&& other) -{ - m_data = other.m_data; - other.m_data = String {}.to_fly_string_data({}); - - return *this; -} - bool FlyString::is_empty() const { return bytes_as_string_view().is_empty(); @@ -97,7 +55,7 @@ bool FlyString::is_empty() const unsigned FlyString::hash() const { - return String::fly_string_data_to_hash({}, m_data); + return m_data.hash(); } u32 FlyString::ascii_case_insensitive_hash() const @@ -112,7 +70,8 @@ FlyString::operator String() const String FlyString::to_string() const { - return String::fly_string_data_to_string({}, m_data); + Detail::StringBase copy = m_data; + return String(move(copy)); } Utf8View FlyString::code_points() const @@ -127,7 +86,7 @@ ReadonlyBytes FlyString::bytes() const StringView FlyString::bytes_as_string_view() const { - return String::fly_string_data_to_string_view({}, m_data); + return m_data.bytes(); } bool FlyString::operator==(FlyString const& other) const @@ -137,10 +96,7 @@ bool FlyString::operator==(FlyString const& other) const bool FlyString::operator==(String const& other) const { - if (m_data == other.to_fly_string_data({})) - return true; - - return bytes_as_string_view() == other.bytes_as_string_view(); + return m_data == other; } bool FlyString::operator==(StringView string) const @@ -158,7 +114,7 @@ void FlyString::did_destroy_fly_string_data(Badge, StringVie all_fly_strings().remove(string_data); } -uintptr_t FlyString::data(Badge) const +Detail::StringBase FlyString::data(Badge) const { return m_data; } diff --git a/AK/FlyString.h b/AK/FlyString.h index 9fe8786a38..b0c9e18a05 100644 --- a/AK/FlyString.h +++ b/AK/FlyString.h @@ -16,9 +16,11 @@ namespace AK { class FlyString { + AK_MAKE_DEFAULT_MOVABLE(FlyString); + AK_MAKE_DEFAULT_COPYABLE(FlyString); + public: - FlyString(); - ~FlyString(); + FlyString() = default; static ErrorOr from_utf8(StringView); template @@ -28,12 +30,6 @@ public: FlyString(String const&); FlyString& operator=(String const&); - FlyString(FlyString const&); - FlyString& operator=(FlyString const&); - - FlyString(FlyString&&); - FlyString& operator=(FlyString&&); - [[nodiscard]] bool is_empty() const; [[nodiscard]] unsigned hash() const; [[nodiscard]] u32 ascii_case_insensitive_hash() const; @@ -53,7 +49,7 @@ public: [[nodiscard]] int operator<=>(FlyString const& other) const; static void did_destroy_fly_string_data(Badge, StringView); - [[nodiscard]] uintptr_t data(Badge) const; + [[nodiscard]] Detail::StringBase data(Badge) const; // This is primarily interesting to unit tests. [[nodiscard]] static size_t number_of_fly_strings(); @@ -80,9 +76,7 @@ public: } private: - // This will hold either the pointer to the Detail::StringData it represents or the raw bytes of - // an inlined short string. - uintptr_t m_data { 0 }; + Detail::StringBase m_data; }; template<> diff --git a/AK/String.cpp b/AK/String.cpp index 5f1aa2232a..6c5e41fa7b 100644 --- a/AK/String.cpp +++ b/AK/String.cpp @@ -41,12 +41,19 @@ StringData::StringData(StringData const& superstring, size_t start, size_t byte_ StringData::~StringData() { - if (m_is_fly_string) - FlyString::did_destroy_fly_string_data({}, bytes_as_string_view()); if (m_substring) substring_data().superstring->unref(); } +void StringData::unref() const +{ + if (m_is_fly_string && m_ref_count == 2) { + m_is_fly_string = false; // Otherwise unref from did_destory_fly_string_data will cause infinite recursion. + FlyString::did_destroy_fly_string_data({}, bytes_as_string_view()); + } + RefCounted::unref(); +} + constexpr size_t allocation_size_for_string_data(size_t length) { return sizeof(StringData) + (sizeof(char) * length); @@ -227,9 +234,7 @@ Optional String::find_byte_offset(StringView substring, size_t from_byte bool String::operator==(FlyString const& other) const { - if (reinterpret_cast(m_data) == other.data({})) - return true; - return bytes_as_string_view() == other.bytes_as_string_view(); + return static_cast(*this) == other.data({}); } bool String::operator==(StringView other) const @@ -367,67 +372,6 @@ unsigned Traits::hash(String const& string) return string.hash(); } -String String::fly_string_data_to_string(Badge, uintptr_t const& data) -{ - if (has_short_string_bit(data)) - return String { *reinterpret_cast(&data) }; - - auto const* string_data = reinterpret_cast(data); - return String { NonnullRefPtr(*string_data) }; -} - -StringView String::fly_string_data_to_string_view(Badge, uintptr_t const& data) -{ - if (has_short_string_bit(data)) { - auto const* short_string = reinterpret_cast(&data); - return short_string->bytes(); - } - - auto const* string_data = reinterpret_cast(data); - return string_data->bytes_as_string_view(); -} - -u32 String::fly_string_data_to_hash(Badge, uintptr_t const& data) -{ - if (has_short_string_bit(data)) { - auto const* short_string = reinterpret_cast(&data); - auto bytes = short_string->bytes(); - return string_hash(reinterpret_cast(bytes.data()), bytes.size()); - } - - auto const* string_data = reinterpret_cast(data); - return string_data->hash(); -} - -uintptr_t String::to_fly_string_data(Badge) const -{ - return reinterpret_cast(m_data); -} - -void String::ref_fly_string_data(Badge, uintptr_t data) -{ - if (has_short_string_bit(data)) - return; - - auto const* string_data = reinterpret_cast(data); - string_data->ref(); -} - -void String::unref_fly_string_data(Badge, uintptr_t data) -{ - if (has_short_string_bit(data)) - return; - - auto const* string_data = reinterpret_cast(data); - string_data->unref(); -} - -void String::did_create_fly_string(Badge) const -{ - VERIFY(!is_short_string()); - m_data->set_fly_string(true); -} - ByteString String::to_byte_string() const { return ByteString(bytes_as_string_view()); diff --git a/AK/String.h b/AK/String.h index cba273ac98..20a31b71ac 100644 --- a/AK/String.h +++ b/AK/String.h @@ -178,15 +178,6 @@ public: return builder.to_string(); } - [[nodiscard]] static String fly_string_data_to_string(Badge, uintptr_t const&); - [[nodiscard]] static StringView fly_string_data_to_string_view(Badge, uintptr_t const&); - [[nodiscard]] static u32 fly_string_data_to_hash(Badge, uintptr_t const&); - [[nodiscard]] uintptr_t to_fly_string_data(Badge) const; - - static void ref_fly_string_data(Badge, uintptr_t); - static void unref_fly_string_data(Badge, uintptr_t); - void did_create_fly_string(Badge) const; - // FIXME: Remove these once all code has been ported to String [[nodiscard]] ByteString to_byte_string() const; static ErrorOr from_byte_string(ByteString const&); @@ -195,6 +186,8 @@ public: static ErrorOr from_byte_string(T&&) = delete; private: + friend FlyString; + using ShortString = Detail::ShortString; explicit constexpr String(StringBase&& base) diff --git a/AK/StringBase.cpp b/AK/StringBase.cpp index e0199a1028..7c0b041f3a 100644 --- a/AK/StringBase.cpp +++ b/AK/StringBase.cpp @@ -4,6 +4,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include @@ -81,9 +82,19 @@ bool StringBase::operator==(StringBase const& other) const { if (is_short_string()) return m_data == other.m_data; + if (other.is_short_string()) + return false; + if (m_data->is_fly_string() && other.m_data->is_fly_string()) + return m_data == other.m_data; return bytes() == other.bytes(); } +void StringBase::did_create_fly_string(Badge) const +{ + VERIFY(!is_short_string()); + m_data->set_fly_string(true); +} + ErrorOr StringBase::replace_with_uninitialized_buffer(size_t byte_count) { if (byte_count <= MAX_SHORT_STRING_BYTE_COUNT) diff --git a/AK/StringBase.h b/AK/StringBase.h index f403d7785f..e362592496 100644 --- a/AK/StringBase.h +++ b/AK/StringBase.h @@ -68,15 +68,12 @@ public: [[nodiscard]] bool operator==(StringBase const&) const; + void did_create_fly_string(Badge) const; + protected: // NOTE: If the least significant bit of the pointer is set, this is a short string. static constexpr uintptr_t SHORT_STRING_FLAG = 1; - static constexpr bool has_short_string_bit(uintptr_t data) - { - return (data & SHORT_STRING_FLAG) != 0; - } - explicit StringBase(NonnullRefPtr); explicit constexpr StringBase(ShortString short_string) diff --git a/AK/StringInternals.h b/AK/StringInternals.h index 3a544fdd5c..99961eb779 100644 --- a/AK/StringInternals.h +++ b/AK/StringInternals.h @@ -25,6 +25,8 @@ public: void operator delete(void* ptr); + void unref() const; + ~StringData(); SubstringData const& substring_data() const