diff --git a/src/oscar/Utils/SharedPreHashedString.h b/src/oscar/Utils/SharedPreHashedString.h index 12ae156c30..da0c4dd2f0 100644 --- a/src/oscar/Utils/SharedPreHashedString.h +++ b/src/oscar/Utils/SharedPreHashedString.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -65,7 +66,10 @@ namespace osc using reverse_iterator = std::string_view::const_reverse_iterator; using const_reverse_iterator = std::string_view::const_reverse_iterator; - explicit SharedPreHashedString() : SharedPreHashedString{std::string_view{}} {} + explicit SharedPreHashedString() : + SharedPreHashedString{SharedPreHashedString::static_default_instance()} + {} + explicit SharedPreHashedString(std::string_view str) { const size_t num_bytes_allocated = @@ -95,26 +99,16 @@ namespace osc static_cast(ptr_)->num_owners.fetch_add(1, std::memory_order_relaxed); } - SharedPreHashedString(SharedPreHashedString&& tmp) noexcept : - ptr_{std::exchange(tmp.ptr_, nullptr)} - {} - - SharedPreHashedString& operator=(const SharedPreHashedString& src) + SharedPreHashedString& operator=(const SharedPreHashedString& src) noexcept { SharedPreHashedString copy{src}; swap(*this, copy); return *this; } - SharedPreHashedString& operator=(SharedPreHashedString&& tmp) noexcept - { - swap(*this, tmp); - return *this; - } - ~SharedPreHashedString() noexcept { - if (ptr_ and static_cast(ptr_)->num_owners.fetch_sub(1, std::memory_order_relaxed) == 1) { + if (static_cast(ptr_)->num_owners.fetch_sub(1, std::memory_order_relaxed) == 1) { ::operator delete(ptr_, std::align_val_t{alignof(Metadata)}); } } @@ -253,6 +247,11 @@ namespace osc return lhs <=> std::string_view{rhs}; } + friend std::ostream& operator<<(std::ostream& lhs, const SharedPreHashedString& rhs) + { + return lhs << std::string_view{rhs}; + } + // returns the number of different `SharedPreHashedString` instances (`this` included) // managing the same underlying string data. In a multithreaded environment, the value // returned by `use_count` is approximate (i.e. the implementation uses a @@ -264,6 +263,12 @@ namespace osc private: friend struct std::hash; + const SharedPreHashedString& static_default_instance() + { + static SharedPreHashedString s_default_instance{std::string_view{}}; + return s_default_instance; + } + struct Metadata final { Metadata(std::string_view str) noexcept : diff --git a/src/oscar/Utils/StringName.cpp b/src/oscar/Utils/StringName.cpp index 675b5a31de..0674c24875 100644 --- a/src/oscar/Utils/StringName.cpp +++ b/src/oscar/Utils/StringName.cpp @@ -1,106 +1,40 @@ #include "StringName.h" -#include - #include #include -#include +#include + #include -#include -#include #include -#include using namespace osc; -using osc::detail::StringNameData; namespace { - // this is what's stored in the lookup table - // - // `StringNameData` _must_ be reference-stable w.r.t. downstream - // code because users are reading/reference incrementing raw - // pointers - struct StringNameDataPtr final { - public: - explicit StringNameDataPtr(std::string_view str) : - ptr_{std::make_unique(str)} - {} - - friend bool operator==(std::string_view lhs, const StringNameDataPtr& rhs) - { - return lhs == rhs.ptr_->value(); - } - - StringNameData* operator->() const - { - return ptr_.get(); - } - - StringNameData& operator*() const - { - return *ptr_; - } - private: - std::unique_ptr ptr_; - }; - - struct TransparentHasher : public TransparentStringHasher { - using TransparentStringHasher::operator(); - - size_t operator()(const StringNameDataPtr& ptr) const noexcept - { - return ptr->hash(); - } - }; - - using StringNameLookup = ankerl::unordered_dense::set< - StringNameDataPtr, - TransparentHasher, - std::equal_to<> - >; + using FastStringLookup = ankerl::unordered_dense::set>; - SynchronizedValue& get_global_string_name_lut() + SynchronizedValue& get_global_lookup() { - static SynchronizedValue s_lut; - return s_lut; + static SynchronizedValue s_lookup; + return s_lookup; } +} - template - requires - std::constructible_from and - std::convertible_to - StringNameData& possibly_construct_then_get_data(StringLike&& input) - { - auto [it, inserted] = get_global_string_name_lut().lock()->emplace(std::forward(input)); - if (not inserted) { - (*it)->increment_owner_count(); - } - return **it; - } +osc::StringName::StringName(std::string_view sv) : + SharedPreHashedString{*get_global_lookup().lock()->emplace(sv).first} +{} - void decrement_then_possibly_destruct_data(StringNameData& data) - { - if (data.decrement_owner_count()) { - get_global_string_name_lut().lock()->erase(data.value()); - } +osc::StringName::~StringName() noexcept +{ + if (empty()) { + return; // special case: the default constructor doesn't use the global lookup } - // edge-case for blank string (common) - const StringName& get_cached_blank_string_data() - { - static const StringName s_blank_string{std::string_view{}}; - return s_blank_string; + if (use_count() > 2) { + return; // other `StringName`s with the same value are still using the data } -} -osc::StringName::StringName() : StringName{get_cached_blank_string_data()} {} -osc::StringName::StringName(std::string&& tmp) : data_{&possibly_construct_then_get_data(std::move(tmp))} {} -osc::StringName::StringName(std::string_view sv) : data_{&possibly_construct_then_get_data(sv)} {} -osc::StringName::~StringName() noexcept { decrement_then_possibly_destruct_data(*data_); } - -std::ostream& osc::operator<<(std::ostream& out, const StringName& s) -{ - return out << std::string_view{s}; + // else: clear it from the global table + get_global_lookup().lock()->erase(static_cast(*this)); } diff --git a/src/oscar/Utils/StringName.h b/src/oscar/Utils/StringName.h index 8837df37fa..1cc03c847d 100644 --- a/src/oscar/Utils/StringName.h +++ b/src/oscar/Utils/StringName.h @@ -1,214 +1,71 @@ #pragma once +#include #include +#include -#include #include #include #include -#include -#include #include namespace osc { - namespace detail - { - class StringNameData final { - public: - explicit StringNameData(std::string_view str) : - value_{str} - {} - StringNameData(StringNameData const&) = delete; - StringNameData(StringNameData&&) noexcept = delete; - StringNameData& operator=(const StringNameData&) = delete; - StringNameData& operator=(StringNameData&&) noexcept = delete; - ~StringNameData() noexcept = default; - - const std::string& value() const { return value_; } - - size_t hash() const { return hash_; } - - void increment_owner_count() - { - num_owners_.fetch_add(1, std::memory_order_relaxed); - } - - bool decrement_owner_count() - { - return num_owners_.fetch_sub(1, std::memory_order_relaxed) == 1; - } - private: - std::string value_; - size_t hash_ = std::hash{}(value_); - std::atomic num_owners_ = 1; - }; - } - // immutable, globally unique string with fast hashing+equality - class StringName final { + class StringName final : public SharedPreHashedString { public: - using traits_type = std::string::traits_type; - using value_type = std::string::value_type; - using size_type = std::string::size_type; - using difference_type = std::string::difference_type; - using reference = std::string::const_reference; - using const_reference = std::string::const_reference; - using iterator = std::string::const_iterator; - using const_iterator = std::string::const_iterator; - using reverse_iterator = std::string::const_reverse_iterator; - using const_reverse_iterator = std::string::const_reverse_iterator; - - explicit StringName(); - explicit StringName(std::string&&); + explicit StringName() = default; explicit StringName(std::string_view); explicit StringName(const char* s) : StringName{std::string_view{s}} {} explicit StringName(std::nullptr_t) = delete; - - StringName(const StringName& other) noexcept : - data_{other.data_} - { - data_->increment_owner_count(); - } - - StringName(StringName&& tmp) noexcept : - data_{tmp.data_} - { - data_->increment_owner_count(); - } - - StringName& operator=(const StringName& other) noexcept - { - if (other == *this) { - return *this; - } - - this->~StringName(); - data_ = other.data_; - data_->increment_owner_count(); - return *this; - } - - StringName& operator=(StringName&& strname) noexcept - { - if (strname == *this) { - return *this; - } - - this->~StringName(); - data_ = strname.data_; - data_->increment_owner_count(); - return *this; - } - + StringName(const StringName&) = default; + StringName(StringName&&) noexcept = default; + StringName& operator=(const StringName&) = default; + StringName& operator=(StringName&&) noexcept = default; ~StringName() noexcept; - friend void swap(StringName& a, StringName& b) noexcept - { - a.swap(b); - } - - const_reference at(size_t pos) const - { - return data_->value().at(pos); - } + using SharedPreHashedString::operator CStringView; + using SharedPreHashedString::operator std::string_view; + using SharedPreHashedString::operator[]; - const_reference operator[](size_t pos) const - { - return data_->value()[pos]; - } + friend bool operator==(const StringName&, const StringName&) = default; - const_reference front() const + template + requires std::constructible_from + friend bool operator==(const StringName& lhs, const StringViewLike& rhs) { - return data_->value().front(); + return std::string_view{lhs} == rhs; } - const_reference back() const + template + requires std::constructible_from + friend bool operator==(const StringViewLike& lhs, const StringName& rhs) { - return data_->value().back(); + return lhs == std::string_view{rhs}; } - const value_type* data() const noexcept - { - return data_->value().data(); - } + friend auto operator<=>(const StringName&, const StringName&) = default; - const value_type* c_str() const noexcept + template + requires std::constructible_from + friend auto operator<=>(const StringName& lhs, const StringViewLike& rhs) { - return data_->value().c_str(); + return std::string_view{lhs} <=> rhs; } - operator std::string_view() const noexcept + template + requires std::constructible_from + friend auto operator<=>(const StringViewLike& lhs, const StringName& rhs) { - return data_->value(); + return lhs <=> std::string_view{rhs}; } - - operator CStringView() const noexcept - { - return data_->value(); - } - - const_iterator begin() const noexcept - { - return data_->value().cbegin(); - } - - const_iterator cbegin() const noexcept - { - return data_->value().cbegin(); - } - - const_iterator end() const noexcept - { - return data_->value().cend(); - } - - const_iterator cend() const noexcept - { - return data_->value().cend(); - } - - [[nodiscard]] bool empty() const noexcept - { - return data_->value().empty(); - } - - size_type size() const noexcept - { - return data_->value().size(); - } - - void swap(StringName& other) noexcept - { - std::swap(data_, other.data_); - } - - friend bool operator==(const StringName&, const StringName&) noexcept = default; - - template StringLike> - friend bool operator==(const StringName& lhs, const StringLike& rhs) - { - return std::string_view{lhs} == std::string_view{rhs}; - } - - template StringLike> - friend auto operator<=>(const StringName& lhs, const StringLike& rhs) - { - return std::string_view{lhs} <=> std::string_view{rhs}; - } - - private: - friend struct std::hash; - detail::StringNameData* data_; }; - - std::ostream& operator<<(std::ostream&, const StringName&); } template<> struct std::hash final { - size_t operator()(const osc::StringName& str) const + size_t operator()(const osc::StringName& sn) const { - return str.data_->hash(); + return std::hash{}(sn); } }; diff --git a/src/oscar/Utils/TransparentStringHasher.h b/src/oscar/Utils/TransparentStringHasher.h index 40f7bfc67a..84975ee10e 100644 --- a/src/oscar/Utils/TransparentStringHasher.h +++ b/src/oscar/Utils/TransparentStringHasher.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -20,6 +21,12 @@ namespace osc return std::hash{}(sv); } + size_t operator()(const SharedPreHashedString& sn) const noexcept + { + // special case: `SharedPreHashedString`s are pre-hashed + return std::hash{}(sn); + } + size_t operator()(const StringName& sn) const noexcept { // special case: `StringName`s are pre-hashed diff --git a/tests/testoscar/Utils/TestSharedPreHashedString.cpp b/tests/testoscar/Utils/TestSharedPreHashedString.cpp index e10a629500..0f6568d9c9 100644 --- a/tests/testoscar/Utils/TestSharedPreHashedString.cpp +++ b/tests/testoscar/Utils/TestSharedPreHashedString.cpp @@ -4,7 +4,9 @@ #include #include +#include #include +#include #include #include @@ -27,14 +29,6 @@ TEST(SharedPreHashedString, default_constructed_size_is_zero) ASSERT_EQ(SharedPreHashedString{}.size(), 0); } -TEST(SharedPreHashedString, default_constructed_copy_makes_use_count_increment) -{ - const SharedPreHashedString str; - ASSERT_EQ(str.use_count(), 1); - const SharedPreHashedString copy = str; - ASSERT_EQ(str.use_count(), 2); -} - TEST(SharedPreHashedString, can_construct_from_cstring) { const SharedPreHashedString str{"some string"}; @@ -161,3 +155,24 @@ TEST(SharedPreHashedString, less_than_works_as_expected) rgs::sort(shared_strings); ASSERT_TRUE(rgs::equal(source_strings, shared_strings)); } + +TEST(SharedPreHashedString, can_stream_to_ostream) +{ + std::stringstream ss; + ss << SharedPreHashedString{"stream me"}; + ASSERT_EQ(ss.str(), "stream me"); +} + + +TEST(SharedPreHashedString, std_hash_returns_same_as_std_string_view) +{ + auto source_strings = std::to_array({ + "", + "str", + "hash me", + " etc.", + }); + for (const auto& source_string : source_strings) { + ASSERT_EQ(std::hash{}(source_string), std::hash{}(SharedPreHashedString{source_string})); + } +} diff --git a/tests/testoscar/Utils/TestStringName.cpp b/tests/testoscar/Utils/TestStringName.cpp index 329467d1ae..f9ef3a90a8 100644 --- a/tests/testoscar/Utils/TestStringName.cpp +++ b/tests/testoscar/Utils/TestStringName.cpp @@ -373,15 +373,6 @@ TEST(StringName, SizeReturnsExpectedValue) ASSERT_EQ(s.size(), c_LongStringToAvoidSSOData.size()-1); // minus nul } -TEST(StringName, SwapSwapsTheStringNamesContents) -{ - StringName a{c_LongStringToAvoidSSO}; - StringName b{c_AnotherStringToAvoidSSO}; - a.swap(b); - ASSERT_EQ(a, c_AnotherStringToAvoidSSO); - ASSERT_EQ(b, c_LongStringToAvoidSSO); -} - TEST(StringName, NonEmptyStringNameComapresEqualToAnotherLogicallyEquivalentNonEmptyStringName) { ASSERT_EQ(StringName{c_LongStringToAvoidSSO}, StringName{c_LongStringToAvoidSSO}); diff --git a/tests/testoscar/Utils/TestTransparentStringHasher.cpp b/tests/testoscar/Utils/TestTransparentStringHasher.cpp index 1bae9c244e..9b8b0a4dc9 100644 --- a/tests/testoscar/Utils/TestTransparentStringHasher.cpp +++ b/tests/testoscar/Utils/TestTransparentStringHasher.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -53,6 +54,7 @@ TEST(TransparentStringHasher, produces_same_hash_for_all_of_OSCs_string_types) TransparentStringHasher{}(CStringView{str}), TransparentStringHasher{}(std::string{str}), TransparentStringHasher{}(StringName{str}), + TransparentStringHasher{}(SharedPreHashedString{str}), }); ASSERT_TRUE(rgs::adjacent_find(hashes, rgs::not_equal_to{}) == hashes.end()); }