From f72fdeeb59ad4d07a98f4cf89e9011a504d49897 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Mon, 30 Oct 2023 11:13:16 +0200 Subject: [PATCH] BAN: String now uses union for its sso storage This allows String to shrink by 8 bytes since Variant's 8 index is no longer stored in here. This required me to make Strings max size one bit less, but that should still be fine. There should never be strings with size of over half of the computer's address space. --- BAN/BAN/String.cpp | 69 ++++++++++++++++++++-------------------- BAN/include/BAN/String.h | 14 +++++--- 2 files changed, 43 insertions(+), 40 deletions(-) diff --git a/BAN/BAN/String.cpp b/BAN/BAN/String.cpp index 5f94a52b..07dc2657 100644 --- a/BAN/BAN/String.cpp +++ b/BAN/BAN/String.cpp @@ -1,6 +1,5 @@ #include #include -#include namespace BAN { @@ -32,8 +31,7 @@ namespace BAN String& String::operator=(const String& other) { clear(); - if (!other.fits_in_sso()) - MUST(ensure_capacity(other.size())); + MUST(ensure_capacity(other.size())); memcpy(data(), other.data(), other.size() + 1); m_size = other.size(); return *this; @@ -43,14 +41,18 @@ namespace BAN { clear(); - if (other.fits_in_sso()) + if (other.has_sso()) memcpy(data(), other.data(), other.size() + 1); else - m_storage = other.m_storage.get(); + { + m_storage.general_storage = other.m_storage.general_storage; + m_has_sso = false; + } m_size = other.m_size; other.m_size = 0; - other.m_storage = SSOStorage(); + other.m_storage.sso_storage = SSOStorage(); + other.m_has_sso = true; return *this; } @@ -58,8 +60,7 @@ namespace BAN String& String::operator=(StringView other) { clear(); - if (!fits_in_sso(other.size())) - MUST(ensure_capacity(other.size())); + MUST(ensure_capacity(other.size())); memcpy(data(), other.data(), other.size()); m_size = other.size(); data()[m_size] = '\0'; @@ -125,8 +126,9 @@ namespace BAN { if (!has_sso()) { - deallocator(m_storage.get().data); - m_storage = SSOStorage(); + deallocator(m_storage.general_storage.data); + m_storage.sso_storage = SSOStorage(); + m_has_sso = true; } m_size = 0; data()[m_size] = '\0'; @@ -166,15 +168,6 @@ namespace BAN data()[m_size] = '\0'; return {}; } - - // shrink general -> sso - if (!has_sso() && fits_in_sso(new_size)) - { - char* data = m_storage.get().data; - m_storage = SSOStorage(); - memcpy(m_storage.get().storage, data, new_size); - deallocator(data); - } m_size = new_size; data()[m_size] = '\0'; @@ -194,20 +187,21 @@ namespace BAN if (fits_in_sso()) { - char* data = m_storage.get().data; - m_storage = SSOStorage(); - memcpy(m_storage.get().storage, data, m_size + 1); + char* data = m_storage.general_storage.data; + m_storage.sso_storage = SSOStorage(); + m_has_sso = true; + memcpy(this->data(), data, m_size + 1); deallocator(data); return {}; } - GeneralStorage& storage = m_storage.get(); + GeneralStorage& storage = m_storage.general_storage; if (storage.capacity == m_size) return {}; char* new_data = (char*)allocator(m_size + 1); if (new_data == nullptr) - return BAN::Error::from_errno(ENOMEM); + return Error::from_errno(ENOMEM); memcpy(new_data, storage.data, m_size); deallocator(storage.data); @@ -222,40 +216,45 @@ namespace BAN { if (has_sso()) return sso_capacity; - return m_storage.get().capacity; + return m_storage.general_storage.capacity; } char* String::data() { if (has_sso()) - return m_storage.get().storage; - return m_storage.get().data; + return m_storage.sso_storage.data; + return m_storage.general_storage.data; } const char* String::data() const { if (has_sso()) - return m_storage.get().storage; - return m_storage.get().data; + return m_storage.sso_storage.data; + return m_storage.general_storage.data; } ErrorOr String::ensure_capacity(size_type new_size) { - if (m_size >= new_size || fits_in_sso(new_size)) + if (m_size >= new_size) + return {}; + if (has_sso() && fits_in_sso(new_size)) return {}; char* new_data = (char*)allocator(new_size + 1); if (new_data == nullptr) - return BAN::Error::from_errno(ENOMEM); + return Error::from_errno(ENOMEM); memcpy(new_data, data(), m_size + 1); if (has_sso()) - m_storage = GeneralStorage(); + { + m_storage.general_storage = GeneralStorage(); + m_has_sso = false; + } else - deallocator(m_storage.get().data); + deallocator(m_storage.general_storage.data); - auto& storage = m_storage.get(); + auto& storage = m_storage.general_storage; storage.capacity = new_size; storage.data = new_data; @@ -264,7 +263,7 @@ namespace BAN bool String::has_sso() const { - return m_storage.has(); + return m_has_sso; } } diff --git a/BAN/include/BAN/String.h b/BAN/include/BAN/String.h index 26685afd..3eecccd8 100644 --- a/BAN/include/BAN/String.h +++ b/BAN/include/BAN/String.h @@ -82,17 +82,21 @@ namespace BAN private: struct SSOStorage { - char storage[sso_capacity + 1] {}; + char data[sso_capacity + 1] {}; }; struct GeneralStorage { - size_type capacity { 0 }; - char* data; + size_type capacity { 0 }; + char* data { nullptr }; }; private: - Variant m_storage { SSOStorage() }; - size_type m_size { 0 }; + union { + SSOStorage sso_storage; + GeneralStorage general_storage; + } m_storage { .sso_storage = SSOStorage() }; + size_type m_size : sizeof(size_type) * 8 - 1 { 0 }; + size_type m_has_sso : 1 { true }; }; template