birkeh/o3de
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Address some AZ::Dom::Value feedback

Browse Source 
- Use a vector for shared string storage (to avoid the double heap allocation for AZStd::string)
- Use a shared heap allocated any for opaque types (instead of an unsafe ref)
- Add a string comparison key lookup benchmark to measure the impact of Name

Signed-off-by: Nicholas Van Sickle <nvsickle@amazon.com>
This commit is contained in:
Nicholas Van Sickle
2021-12-20 16:54:06 -08:00
parent 748f2a5c05
commit bbd00adade
7 changed files with 91 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Code/Framework/AzCore/AzCore/DOM/DomValue.cpp
+27 -22
View File
@@ -19,7 +19,7 @@ namespace AZ::Dom
{
if (refCountedPointer.use_count() > 1)
{
AZStd::shared_ptr<T> newPointer = AZStd::allocate_shared<T>(AZStdAlloc<ValueAllocator>());
AZStd::shared_ptr<T> newPointer = AZStd::allocate_shared<T>(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<AZStd::any>(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<Object>(AZStdAlloc<ValueAllocator>());
m_value = AZStd::allocate_shared<Object>(StdValueAllocator());
return *this;
}
@@ -634,7 +634,7 @@ namespace AZ::Dom
Value& Value::SetArray()
{
m_value = AZStd::allocate_shared<Array>(AZStdAlloc<ValueAllocator>());
m_value = AZStd::allocate_shared<Array>(StdValueAllocator());
return *this;
}
@@ -740,7 +740,7 @@ namespace AZ::Dom
void Value::SetNode(AZ::Name name)
{
m_value = AZStd::allocate_shared<Node>(AZStdAlloc<ValueAllocator>(), name);
m_value = AZStd::allocate_shared<Node>(StdValueAllocator(), name);
}
void Value::SetNode(AZStd::string_view name)
@@ -914,9 +914,9 @@ namespace AZ::Dom
m_value = aznumeric_cast<double>(value);
}
void Value::SetString(AZStd::shared_ptr<const AZStd::string> 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<AZStd::string_view>(m_value);
case 6: // AZStd::shared_ptr<const AZStd::string>
return *AZStd::get<AZStd::shared_ptr<const AZStd::string>>(m_value);
case 6: // AZStd::shared_ptr<AZStd::vector<char>>
{
auto& buffer = *AZStd::get<SharedStringType>(m_value);
return { buffer.data(), buffer.size() };
}
case 7: // ShortStringType
{
const ShortStringType& ShortString = AZStd::get<ShortStringType>(m_value);
return { ShortString.m_data.data(), ShortString.m_size };
const ShortStringType& shortString = AZStd::get<ShortStringType>(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<const AZStd::string>(AZStdAlloc<ValueAllocator>(), value);
SharedStringType sharedString =
AZStd::allocate_shared<SharedStringContainer>(StdValueAllocator(), value.begin(), value.end());
m_value = AZStd::move(sharedString);
}
}
AZStd::any& Value::GetOpaqueValue() const
const AZStd::any& Value::GetOpaqueValue() const
{
return *AZStd::get<AZStd::any*>(m_value);
return *AZStd::get<AZStd::shared_ptr<AZStd::any>>(m_value);
}
void Value::SetOpaqueValue(AZStd::any& value)
void Value::SetOpaqueValue(const AZStd::any& value)
{
m_value = &value;
m_value = AZStd::allocate_shared<AZStd::any>(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<Alternative, AZStd::shared_ptr<const AZStd::string>>)
else if constexpr (AZStd::is_same_v<Alternative, SharedStringType>)
{
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<AZStd::shared_ptr<const AZStd::string>>(m_value) && m_value == other.m_value)
if (AZStd::holds_alternative<SharedStringType>(m_value) && m_value == other.m_value)
{
return true;
}
Code/Framework/AzCore/AzCore/DOM/DomValue.h
+28 -31
View File
@@ -55,13 +55,15 @@ namespace AZ::Dom
}
};
using StdValueAllocator = AZStdAlloc<ValueAllocator>;
class Value;
//! Internal storage for a Value array: an ordered list of Values.
class Array
{
public:
using ContainerType = AZStd::vector<Value, AZStdAlloc<ValueAllocator>>;
using ContainerType = AZStd::vector<Value, StdValueAllocator>;
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<KeyType, Value>;
using ContainerType = AZStd::vector<EntryType, AZStdAlloc<ValueAllocator>>;
using ContainerType = AZStd::vector<EntryType, StdValueAllocator>;
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<ShortStringSize>;
using SharedStringContainer = AZStd::vector<char>;
using SharedStringType = AZStd::shared_ptr<const SharedStringContainer>;
// 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<const AZStd::string>);
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<char, ShortStringSize> 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<const AZStd::string>,
SharedStringType,
ShortStringType,
// ObjectType
// Object
ObjectPtr,
// ArrayType
// Array
ArrayPtr,
// NodeType
// Node
NodePtr,
// OpaqueType
AZStd::any*>;
// Opaque
AZStd::shared_ptr<AZStd::any>>;
static_assert(
sizeof(ValueType) == sizeof(AZStd::variant<ShortStringType>), "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;
};
Code/Framework/AzCore/AzCore/DOM/DomValueWriter.cpp
+1 -1
View File
@@ -68,7 +68,7 @@ namespace AZ::Dom
return FinishWrite();
}
Visitor::Result ValueWriter::RefCountedString(AZStd::shared_ptr<const AZStd::string> value, [[maybe_unused]] Lifetime lifetime)
Visitor::Result ValueWriter::RefCountedString(AZStd::shared_ptr<const AZStd::vector<char>> value, [[maybe_unused]] Lifetime lifetime)
{
CurrentValue().SetString(value);
return FinishWrite();
Code/Framework/AzCore/AzCore/DOM/DomValueWriter.h
+1 -1
View File
@@ -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<const AZStd::string> value, Lifetime lifetime) override;
Result RefCountedString(AZStd::shared_ptr<const AZStd::vector<char>> value, Lifetime lifetime) override;
Result StartObject() override;
Result EndObject(AZ::u64 attributeCount) override;
Result Key(AZ::Name key) override;
Code/Framework/AzCore/AzCore/DOM/DomVisitor.cpp
+2 -2
View File
@@ -105,9 +105,9 @@ namespace AZ::Dom
return VisitorSuccess();
}
Visitor::Result Visitor::RefCountedString(AZStd::shared_ptr<const AZStd::string> value, Lifetime lifetime)
Visitor::Result Visitor::RefCountedString(AZStd::shared_ptr<const AZStd::vector<char>> value, Lifetime lifetime)
{
return String(*value, lifetime);
return String({ value->data(), value->size() }, lifetime);
}
Visitor::Result Visitor::OpaqueValue([[maybe_unused]] OpaqueType& value)
Code/Framework/AzCore/AzCore/DOM/DomVisitor.h
+3 -2
View File
@@ -11,8 +11,9 @@
#include <AzCore/Name/Name.h>
#include <AzCore/Outcome/Outcome.h>
#include <AzCore/std/any.h>
#include <AzCore/std/string/string.h>
#include <AzCore/std/containers/vector.h>
#include <AzCore/std/smart_ptr/shared_ptr.h>
#include <AzCore/std/string/string.h>
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<const AZStd::string> value, Lifetime lifetime);
virtual Result RefCountedString(AZStd::shared_ptr<const AZStd::vector<char>> 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
Code/Framework/AzCore/Tests/DOM/DomValueBenchmarks.cpp
+29
View File
@@ -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<AZStd::string> 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