diff --git a/Code/Framework/AzCore/AzCore/DOM/DomValue.cpp b/Code/Framework/AzCore/AzCore/DOM/DomValue.cpp index dcecc67460..3dafc02c6e 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomValue.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomValue.cpp @@ -19,7 +19,7 @@ namespace AZ::Dom { if (refCountedPointer.use_count() > 1) { - AZStd::shared_ptr newPointer = AZStd::allocate_shared(AZStdAlloc()); + AZStd::shared_ptr newPointer = AZStd::allocate_shared(StdValueAllocator()); *newPointer = *refCountedPointer; refCountedPointer = AZStd::move(newPointer); } @@ -92,12 +92,12 @@ namespace AZ::Dom } } - Value::Value(AZStd::any* value) - : m_value(value) + Value::Value(const AZStd::any& value) + : m_value(AZStd::allocate_shared(StdValueAllocator(), value)) { } - Value Value::FromOpaqueValue(AZStd::any& value) + Value Value::FromOpaqueValue(const AZStd::any& value) { return Value(&value); } @@ -330,7 +330,7 @@ namespace AZ::Dom Value& Value::SetObject() { - m_value = AZStd::allocate_shared(AZStdAlloc()); + m_value = AZStd::allocate_shared(StdValueAllocator()); return *this; } @@ -634,7 +634,7 @@ namespace AZ::Dom Value& Value::SetArray() { - m_value = AZStd::allocate_shared(AZStdAlloc()); + m_value = AZStd::allocate_shared(StdValueAllocator()); return *this; } @@ -740,7 +740,7 @@ namespace AZ::Dom void Value::SetNode(AZ::Name name) { - m_value = AZStd::allocate_shared(AZStdAlloc(), name); + m_value = AZStd::allocate_shared(StdValueAllocator(), name); } void Value::SetNode(AZStd::string_view name) @@ -914,9 +914,9 @@ namespace AZ::Dom m_value = aznumeric_cast(value); } - void Value::SetString(AZStd::shared_ptr string) + void Value::SetString(SharedStringType sharedString) { - m_value = string; + m_value = sharedString; } AZStd::string_view Value::GetString() const @@ -925,12 +925,15 @@ namespace AZ::Dom { case 5: // AZStd::string_view return AZStd::get(m_value); - case 6: // AZStd::shared_ptr - return *AZStd::get>(m_value); + case 6: // AZStd::shared_ptr> + { + auto& buffer = *AZStd::get(m_value); + return { buffer.data(), buffer.size() }; + } case 7: // ShortStringType { - const ShortStringType& ShortString = AZStd::get(m_value); - return { ShortString.m_data.data(), ShortString.m_size }; + const ShortStringType& shortString = AZStd::get(m_value); + return { shortString.data(), shortString.size() }; } } AZ_Assert(false, "AZ::Dom::Value: Called GetString on a non-string type"); @@ -947,8 +950,8 @@ namespace AZ::Dom if (value.size() <= ShortStringSize) { ShortStringType buffer; - buffer.m_size = value.size(); - memcpy(buffer.m_data.data(), value.data(), buffer.m_size); + buffer.resize_no_construct(value.size()); + memcpy(buffer.data(), value.data(), value.size()); m_value = buffer; } m_value = value; @@ -962,18 +965,20 @@ namespace AZ::Dom } else { - m_value = AZStd::allocate_shared(AZStdAlloc(), value); + SharedStringType sharedString = + AZStd::allocate_shared(StdValueAllocator(), value.begin(), value.end()); + m_value = AZStd::move(sharedString); } } - AZStd::any& Value::GetOpaqueValue() const + const AZStd::any& Value::GetOpaqueValue() const { - return *AZStd::get(m_value); + return *AZStd::get>(m_value); } - void Value::SetOpaqueValue(AZStd::any& value) + void Value::SetOpaqueValue(const AZStd::any& value) { - m_value = &value; + m_value = AZStd::allocate_shared(StdValueAllocator(), value); } void Value::SetNull() @@ -1014,7 +1019,7 @@ namespace AZ::Dom { result = visitor.String(arg, copyStrings ? Lifetime::Temporary : Lifetime::Persistent); } - else if constexpr (AZStd::is_same_v>) + else if constexpr (AZStd::is_same_v) { result = visitor.RefCountedString(arg, copyStrings ? Lifetime::Temporary : Lifetime::Persistent); } @@ -1110,7 +1115,7 @@ namespace AZ::Dom if (IsString() && other.IsString()) { // If we both hold the same ref counted string we don't need to do a full comparison - if (AZStd::holds_alternative>(m_value) && m_value == other.m_value) + if (AZStd::holds_alternative(m_value) && m_value == other.m_value) { return true; } diff --git a/Code/Framework/AzCore/AzCore/DOM/DomValue.h b/Code/Framework/AzCore/AzCore/DOM/DomValue.h index 1b69c73ecc..bc1b790009 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomValue.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomValue.h @@ -55,13 +55,15 @@ namespace AZ::Dom } }; + using StdValueAllocator = AZStdAlloc; + class Value; //! Internal storage for a Value array: an ordered list of Values. class Array { public: - using ContainerType = AZStd::vector>; + using ContainerType = AZStd::vector; using Iterator = ContainerType::iterator; using ConstIterator = ContainerType::const_iterator; static constexpr const size_t ReserveIncrement = 4; @@ -80,7 +82,7 @@ namespace AZ::Dom { public: using EntryType = AZStd::pair; - using ContainerType = AZStd::vector>; + using ContainerType = AZStd::vector; using Iterator = ContainerType::iterator; using ConstIterator = ContainerType::const_iterator; static constexpr const size_t ReserveIncrement = 8; @@ -150,6 +152,13 @@ namespace AZ::Dom class Value final { public: + // Determine the short string buffer size based on the size of our largest internal type (string_view) + // minus the size of the short string size field. + static constexpr const size_t ShortStringSize = sizeof(AZStd::string_view) - 2; + using ShortStringType = AZStd::fixed_string; + using SharedStringContainer = AZStd::vector; + using SharedStringType = AZStd::shared_ptr; + // Constructors... Value(); Value(const Value&); @@ -167,7 +176,7 @@ namespace AZ::Dom explicit Value(Type type); - static Value FromOpaqueValue(AZStd::any& value); + static Value FromOpaqueValue(const AZStd::any& value); // Equality / comparison / swap... Value& operator=(const Value&); @@ -306,17 +315,17 @@ namespace AZ::Dom AZStd::string_view GetString() const; size_t GetStringLength() const; void SetString(AZStd::string_view); - void SetString(AZStd::shared_ptr); + void SetString(SharedStringType sharedString); void CopyFromString(AZStd::string_view); // Opaque type API... - AZStd::any& GetOpaqueValue() const; + const AZStd::any& GetOpaqueValue() const; //! This sets this Value to represent a value of an type that the DOM has //! no formal knowledge of. Where possible, it should be preferred to //! serialize an opaque type into a DOM value instead, as serializers //! and other systems will have no means of dealing with fully arbitrary //! values. - void SetOpaqueValue(AZStd::any&); + void SetOpaqueValue(const AZStd::any&); // Null API... void SetNull(); @@ -336,49 +345,37 @@ namespace AZ::Dom const Array::ContainerType& GetArrayInternal() const; Array::ContainerType& GetArrayInternal(); - explicit Value(AZStd::any* opaqueValue); - - // Determine the short string buffer size based on the size of our largest internal type (string_view) - // minus the size of the short string size field. - static constexpr const size_t ShortStringSize = sizeof(AZStd::string_view) - sizeof(size_t); - struct ShortStringType - { - AZStd::array m_data; - size_t m_size; - - bool operator==(const ShortStringType& other) const - { - return m_size == other.m_size ? memcmp(m_data.data(), other.m_data.data(), m_size) == 0 : false; - } - }; + explicit Value(const AZStd::any& opaqueValue); //! The internal storage type for Value. //! These types do not correspond one-to-one with the Value's external Type as there may be multiple storage classes //! for the same type in some instances, such as string storage. using ValueType = AZStd::variant< - // NullType + // Null AZStd::monostate, - // NumberType + // Int64 int64_t, + // Uint64 uint64_t, + // Double double, - // FalseType & TrueType + // Bool bool, // StringType AZStd::string_view, - AZStd::shared_ptr, + SharedStringType, ShortStringType, - // ObjectType + // Object ObjectPtr, - // ArrayType + // Array ArrayPtr, - // NodeType + // Node NodePtr, - // OpaqueType - AZStd::any*>; + // Opaque + AZStd::shared_ptr>; static_assert( - sizeof(ValueType) == sizeof(AZStd::variant), "ValueType should have no members larger than ShortStringType"); + sizeof(ValueType) == sizeof(ShortStringType) + sizeof(size_t), "ValueType should have no members larger than ShortStringType"); ValueType m_value; }; diff --git a/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.cpp b/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.cpp index 172f7f2a92..9b814b1bff 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.cpp @@ -68,7 +68,7 @@ namespace AZ::Dom return FinishWrite(); } - Visitor::Result ValueWriter::RefCountedString(AZStd::shared_ptr value, [[maybe_unused]] Lifetime lifetime) + Visitor::Result ValueWriter::RefCountedString(AZStd::shared_ptr> value, [[maybe_unused]] Lifetime lifetime) { CurrentValue().SetString(value); return FinishWrite(); diff --git a/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.h b/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.h index 488535ace6..4fdea293eb 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.h @@ -28,7 +28,7 @@ namespace AZ::Dom Result Double(double value) override; Result String(AZStd::string_view value, Lifetime lifetime) override; - Result RefCountedString(AZStd::shared_ptr value, Lifetime lifetime) override; + Result RefCountedString(AZStd::shared_ptr> value, Lifetime lifetime) override; Result StartObject() override; Result EndObject(AZ::u64 attributeCount) override; Result Key(AZ::Name key) override; diff --git a/Code/Framework/AzCore/AzCore/DOM/DomVisitor.cpp b/Code/Framework/AzCore/AzCore/DOM/DomVisitor.cpp index 4e13a2a95c..0da4bcd5d3 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomVisitor.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomVisitor.cpp @@ -105,9 +105,9 @@ namespace AZ::Dom return VisitorSuccess(); } - Visitor::Result Visitor::RefCountedString(AZStd::shared_ptr value, Lifetime lifetime) + Visitor::Result Visitor::RefCountedString(AZStd::shared_ptr> value, Lifetime lifetime) { - return String(*value, lifetime); + return String({ value->data(), value->size() }, lifetime); } Visitor::Result Visitor::OpaqueValue([[maybe_unused]] OpaqueType& value) diff --git a/Code/Framework/AzCore/AzCore/DOM/DomVisitor.h b/Code/Framework/AzCore/AzCore/DOM/DomVisitor.h index 3bf1b3419b..f9930f93c6 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomVisitor.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomVisitor.h @@ -11,8 +11,9 @@ #include #include #include -#include +#include #include +#include namespace AZ::Dom { @@ -176,7 +177,7 @@ namespace AZ::Dom //! Operates on a ref-counted string value. S //! \param lifetime Specifies the lifetime of this string. If the string has a temporary lifetime, it may not //! be safely stored as a reference, but may still be safely stored as a ref-counted shared_ptr. - virtual Result RefCountedString(AZStd::shared_ptr value, Lifetime lifetime); + virtual Result RefCountedString(AZStd::shared_ptr> value, Lifetime lifetime); //! Operates on an opaque value. As opaque values are a reference type, storage semantics are provided to //! indicate where the value may be stored persistently or requires a copy. //! The base implementation of OpaqueValue rejects the operation, as opaque values are meant for special diff --git a/Code/Framework/AzCore/Tests/DOM/DomValueBenchmarks.cpp b/Code/Framework/AzCore/Tests/DOM/DomValueBenchmarks.cpp index 51d2e5c10c..af5b367d63 100644 --- a/Code/Framework/AzCore/Tests/DOM/DomValueBenchmarks.cpp +++ b/Code/Framework/AzCore/Tests/DOM/DomValueBenchmarks.cpp @@ -220,4 +220,33 @@ namespace AZ::Dom::Benchmark } BENCHMARK_REGISTER_F(DomValueBenchmark, LookupMemberByString)->Arg(100)->Arg(1000)->Arg(10000)->Unit(benchmark::kMillisecond); + BENCHMARK_DEFINE_F(DomValueBenchmark, LookupMemberByStringComparison)(benchmark::State& state) + { + Value value(Type::Object); + AZStd::vector keys; + for (int64_t i = 0; i < state.range(0); ++i) + { + AZStd::string key(AZStd::string::format("key%" PRId64, i)); + keys.push_back(key); + value[key] = i; + } + + for (auto _ : state) + { + for (const AZStd::string& key : keys) + { + const Object::ContainerType& object = value.GetObject(); + benchmark::DoNotOptimize(AZStd::find_if( + object.cbegin(), object.cend(), + [&key](const Object::EntryType& entry) + { + return key == entry.first.GetStringView(); + })); + } + } + + state.SetItemsProcessed(state.iterations() * state.range(0)); + } + BENCHMARK_REGISTER_F(DomValueBenchmark, LookupMemberByStringComparison)->Arg(100)->Arg(1000)->Arg(10000)->Unit(benchmark::kMillisecond); + } // namespace AZ::Dom::Benchmark