diff --git a/Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonSerializationUtils.cpp b/Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonSerializationUtils.cpp index bc8ac9167c..3baf78b63d 100644 --- a/Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonSerializationUtils.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonSerializationUtils.cpp @@ -373,10 +373,6 @@ namespace AZ::Dom::Json bool RapidJsonReadHandler::Key(const char* str, rapidjson::SizeType length, [[maybe_unused]] bool copy) { AZStd::string_view key = AZStd::string_view(str, length); - if (!m_visitor->SupportsRawKeys()) - { - m_visitor->Key(AZ::Name(key)); - } const Lifetime lifetime = !copy ? m_stringLifetime : Lifetime::Temporary; return CheckResult(m_visitor->RawKey(key, lifetime)); } diff --git a/Code/Framework/AzCore/AzCore/DOM/DomValue.cpp b/Code/Framework/AzCore/AzCore/DOM/DomValue.cpp index 3836568f37..407d1e66a2 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomValue.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomValue.cpp @@ -17,13 +17,15 @@ namespace AZ::Dom template AZStd::shared_ptr& CheckCopyOnWrite(AZStd::shared_ptr& refCountedPointer) { - if (refCountedPointer.use_count() > 1) + if (refCountedPointer.use_count() == 1) { - AZStd::shared_ptr newPointer = AZStd::allocate_shared(StdValueAllocator()); - *newPointer = *refCountedPointer; - refCountedPointer = AZStd::move(newPointer); + return refCountedPointer; + } + else + { + refCountedPointer = AZStd::allocate_shared(StdValueAllocator(), *refCountedPointer); + return refCountedPointer; } - return refCountedPointer; } namespace Internal @@ -72,7 +74,7 @@ namespace AZ::Dom } Node::Node(AZ::Name name) - : m_name(name) + : m_name(AZStd::move(name)) { } @@ -83,7 +85,7 @@ namespace AZ::Dom void Node::SetName(AZ::Name name) { - m_name = name; + m_name = AZStd::move(name); } Object::ContainerType& Node::GetProperties() @@ -106,8 +108,8 @@ namespace AZ::Dom return m_children; } - Value::Value(AZStd::shared_ptr string) - : m_value(string) + Value::Value(SharedStringType sharedString) + : m_value(AZStd::move(sharedString)) { } @@ -122,18 +124,19 @@ namespace AZ::Dom Value::Value(Value&& value) noexcept { - operator=(value); + memcpy(this, &value, sizeof(Value)); + memset(&value, 0, sizeof(Value)); } - Value::Value(AZStd::string_view string, bool copy) + Value::Value(AZStd::string_view stringView, bool copy) { if (copy) { - CopyFromString(string); + CopyFromString(stringView); } else { - SetString(string); + SetString(stringView); } } @@ -227,7 +230,9 @@ namespace AZ::Dom Value& Value::operator=(Value&& other) noexcept { - m_value.swap(other.m_value); + SetNull(); + memcpy(this, &other, sizeof(Value)); + memset(&other, 0, sizeof(Value)); return *this; } @@ -265,7 +270,10 @@ namespace AZ::Dom void Value::Swap(Value& other) noexcept { - m_value.swap(other.m_value); + AZStd::aligned_storage_for_t temp; + memcpy(&temp, this, sizeof(Value)); + memcpy(this, &other, sizeof(Value)); + memcpy(&other, &temp, sizeof(Value)); } Type Dom::Value::GetType() const @@ -583,7 +591,7 @@ namespace AZ::Dom } else { - object.emplace_back(name, value); + object.emplace_back(AZStd::move(name), value); } return *this; } @@ -602,7 +610,7 @@ namespace AZ::Dom } else { - object.emplace_back(name, value); + object.emplace_back(AZStd::move(name), value); } return *this; } @@ -636,15 +644,12 @@ namespace AZ::Dom Object::Iterator Value::RemoveMember(Object::Iterator pos) { Object::ContainerType& object = GetObjectInternal(); - Object::Iterator nextIndex = object.end(); - auto lastEntry = object.end() - 1; - if (pos != lastEntry) + if (!object.empty()) { - AZStd::swap(*pos, *lastEntry); - nextIndex = pos; + AZStd::swap(*pos, object.back()); + object.pop_back(); } - object.resize(object.size() - 1); - return nextIndex; + return object.end(); } Object::Iterator Value::EraseMember(Object::ConstIterator pos) @@ -785,7 +790,7 @@ namespace AZ::Dom void Value::SetNode(AZ::Name name) { - m_value = AZStd::allocate_shared(StdValueAllocator(), name); + m_value = AZStd::allocate_shared(StdValueAllocator(), AZStd::move(name)); } void Value::SetNode(AZStd::string_view name) @@ -800,7 +805,7 @@ namespace AZ::Dom void Value::SetNodeName(AZ::Name name) { - GetNodeInternal().SetName(name); + GetNodeInternal().SetName(AZStd::move(name)); } void Value::SetNodeName(AZStd::string_view name) diff --git a/Code/Framework/AzCore/AzCore/DOM/DomValue.h b/Code/Framework/AzCore/AzCore/DOM/DomValue.h index 86e98030e2..a777640aaf 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomValue.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomValue.h @@ -190,8 +190,8 @@ namespace AZ::Dom Value(); Value(const Value&); Value(Value&&) noexcept; - Value(AZStd::string_view string, bool copy); - Value(AZStd::shared_ptr string); + Value(AZStd::string_view stringView, bool copy); + Value(SharedStringType sharedString); Value(int32_t value); Value(uint32_t value); diff --git a/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.cpp b/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.cpp index 9b814b1bff..3259579aaa 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.cpp @@ -82,6 +82,16 @@ namespace AZ::Dom return VisitorSuccess(); } + template + void MoveVectorMemory(AZStd::vector& dest, AZStd::vector& source) + { + dest.resize_no_construct(source.size()); + const size_t size = sizeof(T) * source.size(); + memcpy(dest.data(), source.data(), size); + memset(source.data(), 0, size); + source.resize(0); + } + Visitor::Result ValueWriter::EndContainer(Type containerType, AZ::u64 attributeCount, AZ::u64 elementCount) { const char* endMethodName; @@ -138,22 +148,12 @@ namespace AZ::Dom } if (buffer.m_attributes.size() > 0) { - container.MemberReserve(buffer.m_attributes.size()); - for (AZStd::pair& entry : buffer.m_attributes) - { - container.AddMember(AZStd::move(entry.first), AZStd::move(entry.second)); - } - buffer.m_attributes.clear(); + MoveVectorMemory(container.GetMutableObject(), buffer.m_attributes); } if(buffer.m_elements.size() > 0) { - container.Reserve(buffer.m_elements.size()); - for (Value& entry : buffer.m_elements) - { - container.PushBack(AZStd::move(entry)); - } - buffer.m_elements.clear(); + MoveVectorMemory(container.GetMutableArray(), buffer.m_elements); } m_entryStack.pop(); @@ -180,7 +180,7 @@ namespace AZ::Dom { AZ_Assert(!m_entryStack.empty(), "Attempmted to push a key with no object"); AZ_Assert(!m_entryStack.top().m_container.IsArray(), "Attempted to push a key to an array"); - m_entryStack.top().m_key = key; + m_entryStack.top().m_key = AZStd::move(key); return VisitorSuccess(); } diff --git a/Code/Framework/AzCore/Tests/DOM/DomJsonBenchmarks.cpp b/Code/Framework/AzCore/Tests/DOM/DomJsonBenchmarks.cpp index 477802dc37..8eda110e7b 100644 --- a/Code/Framework/AzCore/Tests/DOM/DomJsonBenchmarks.cpp +++ b/Code/Framework/AzCore/Tests/DOM/DomJsonBenchmarks.cpp @@ -120,6 +120,16 @@ namespace Benchmark AZ_Assert(result.IsSuccess(), "Failed to serialize generated JSON"); return serializedJson; } + + template + void TakeAndDiscardWithoutTimingDtor(T&& value, benchmark::State& state) + { + { + T instance = AZStd::move(value); + state.PauseTiming(); + } + state.ResumeTiming(); + } }; // Helper macro for registering JSON benchmarks @@ -148,7 +158,7 @@ namespace Benchmark return AZ::Dom::Utils::ReadFromStringInPlace(backend, payloadCopy, visitor); }); - benchmark::DoNotOptimize(result.GetValue()); + TakeAndDiscardWithoutTimingDtor(result.TakeValue(), state); } state.SetBytesProcessed(serializedPayload.size() * state.iterations()); @@ -172,7 +182,7 @@ namespace Benchmark return AZ::Dom::Utils::ReadFromStringInPlace(backend, payloadCopy, visitor); }); - benchmark::DoNotOptimize(result.GetValue()); + TakeAndDiscardWithoutTimingDtor(result.TakeValue(), state); } state.SetBytesProcessed(serializedPayload.size() * state.iterations()); @@ -192,7 +202,7 @@ namespace Benchmark return AZ::Dom::Utils::ReadFromString(backend, serializedPayload, AZ::Dom::Lifetime::Temporary, visitor); }); - benchmark::DoNotOptimize(result.GetValue()); + TakeAndDiscardWithoutTimingDtor(result.TakeValue(), state); } state.SetBytesProcessed(serializedPayload.size() * state.iterations()); @@ -212,7 +222,7 @@ namespace Benchmark return AZ::Dom::Utils::ReadFromString(backend, serializedPayload, AZ::Dom::Lifetime::Temporary, visitor); }); - benchmark::DoNotOptimize(result.GetValue()); + TakeAndDiscardWithoutTimingDtor(result.TakeValue(), state); } state.SetBytesProcessed(serializedPayload.size() * state.iterations()); @@ -228,7 +238,7 @@ namespace Benchmark { auto result = AZ::JsonSerializationUtils::ReadJsonString(serializedPayload); - benchmark::DoNotOptimize(result.GetValue()); + TakeAndDiscardWithoutTimingDtor(result.TakeValue(), state); } state.SetBytesProcessed(serializedPayload.size() * state.iterations()); @@ -239,7 +249,7 @@ namespace Benchmark { for (auto _ : state) { - benchmark::DoNotOptimize(GenerateDomJsonBenchmarkPayload(state.range(0), state.range(1))); + TakeAndDiscardWithoutTimingDtor(GenerateDomJsonBenchmarkPayload(state.range(0), state.range(1)), state); } state.SetItemsProcessed(state.range(0) * state.range(0) * state.iterations()); @@ -277,7 +287,7 @@ namespace Benchmark { rapidjson::Document copy; copy.CopyFrom(original, copy.GetAllocator(), true); - benchmark::DoNotOptimize(copy); + TakeAndDiscardWithoutTimingDtor(AZStd::move(copy), state); } state.SetItemsProcessed(state.iterations()); @@ -294,7 +304,7 @@ namespace Benchmark rapidjson::Document copy; copy.CopyFrom(original, copy.GetAllocator(), true); copy["entries"]["Key0"].PushBack(42, copy.GetAllocator()); - benchmark::DoNotOptimize(copy); + TakeAndDiscardWithoutTimingDtor(AZStd::move(copy), state); } state.SetItemsProcessed(state.iterations()); diff --git a/Code/Framework/AzCore/Tests/DOM/DomValueBenchmarks.cpp b/Code/Framework/AzCore/Tests/DOM/DomValueBenchmarks.cpp index af5b367d63..12684c64bc 100644 --- a/Code/Framework/AzCore/Tests/DOM/DomValueBenchmarks.cpp +++ b/Code/Framework/AzCore/Tests/DOM/DomValueBenchmarks.cpp @@ -98,13 +98,23 @@ namespace AZ::Dom::Benchmark return root; } + + template + void TakeAndDiscardWithoutTimingDtor(T&& value, benchmark::State& state) + { + { + T instance = AZStd::move(value); + state.PauseTiming(); + } + state.ResumeTiming(); + } }; BENCHMARK_DEFINE_F(DomValueBenchmark, AzDomValueMakeComplexObject)(benchmark::State& state) { for (auto _ : state) { - benchmark::DoNotOptimize(GenerateDomBenchmarkPayload(state.range(0), state.range(1))); + TakeAndDiscardWithoutTimingDtor(GenerateDomBenchmarkPayload(state.range(0), state.range(1)), state); } state.SetItemsProcessed(state.range(0) * state.range(0) * state.iterations()); @@ -143,7 +153,7 @@ namespace AZ::Dom::Benchmark { Value copy = original; copy["entries"]["Key0"].PushBack(42); - benchmark::DoNotOptimize(copy); + TakeAndDiscardWithoutTimingDtor(AZStd::move(copy), state); } state.SetItemsProcessed(state.iterations()); @@ -162,7 +172,7 @@ namespace AZ::Dom::Benchmark for (auto _ : state) { Value copy = original.DeepCopy(); - benchmark::DoNotOptimize(copy); + TakeAndDiscardWithoutTimingDtor(AZStd::move(copy), state); } state.SetItemsProcessed(state.iterations());