diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp b/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp index 62e6209dcb..9be99d5a03 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomPath.cpp @@ -89,26 +89,6 @@ namespace AZ::Dom return !operator==(key); } - bool PathEntry::operator<(const PathEntry& rhs) const - { - return AZStd::visit( - [&](auto&& lhsValue) - { - using CurrentType = AZStd::decay_t; - if constexpr (AZStd::is_same_v) - { - const size_t* rhsValue = AZStd::get_if(&rhs.m_value); - return rhsValue == nullptr ? true : lhsValue < *rhsValue; - } - else if constexpr (AZStd::is_same_v) - { - const AZ::Name* rhsValue = AZStd::get_if(&rhs.m_value); - return rhsValue == nullptr ? false : lhsValue.GetStringView() < rhsValue->GetStringView(); - } - }, - m_value); - } - void PathEntry::SetEndOfArray() { m_value = EndOfArrayIndex; @@ -137,6 +117,36 @@ namespace AZ::Dom return AZStd::get(m_value); } + size_t PathEntry::GetHash() const + { + return AZStd::visit( + [&](auto&& value) -> size_t + { + using CurrentType = AZStd::decay_t; + if constexpr (AZStd::is_same_v) + { + AZStd::hash hasher; + return hasher(value); + } + else if constexpr (AZStd::is_same_v) + { + return value.GetHash(); + } + }, + m_value); + } +} // namespace AZ::Dom + +namespace AZStd +{ + size_t AZStd::hash::operator()(const AZ::Dom::PathEntry& entry) const + { + return entry.GetHash(); + } +} // namespace AZStd + +namespace AZ::Dom +{ const AZ::Name& PathEntry::GetKey() const { AZ_Assert(IsKey(), "Key called on PathEntry that is not a key"); diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPath.h b/Code/Framework/AzCore/AzCore/DOM/DomPath.h index 7317175490..416f3b6903 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPath.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomPath.h @@ -47,10 +47,6 @@ namespace AZ::Dom bool operator!=(const AZ::Name& key) const; bool operator!=(AZStd::string_view key) const; - //! Comparison operator used for storage in map structures - //! Compares hash order and not lexigraphic order, so not suitable for UI purposes - bool operator<(const PathEntry& rhs) const; - void SetEndOfArray(); bool IsEndOfArray() const; @@ -59,11 +55,24 @@ namespace AZ::Dom size_t GetIndex() const; const AZ::Name& GetKey() const; + size_t GetHash() const; private: AZStd::variant m_value; }; +} // namespace AZ::Dom +namespace AZStd +{ + template<> + struct hash + { + size_t operator()(const AZ::Dom::PathEntry& entry) const; + }; +} // namespace AZStd + +namespace AZ::Dom +{ //! Represents a path, represented as a series of PathEntry values, to a position in a Value. class Path final { @@ -139,7 +148,7 @@ namespace AZ::Dom AZStd::string ToString() const; void AppendToString(AZStd::string& output) const; - template + template void AppendToString(T& output) const { const size_t startIndex = output.length(); diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPrefixTree.h b/Code/Framework/AzCore/AzCore/DOM/DomPrefixTree.h index 42a93041a4..67034f2ec5 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPrefixTree.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomPrefixTree.h @@ -15,7 +15,7 @@ namespace AZ::Dom { - //! Specifies how a patch matches against a DomPrefixTree + //! Specifies how a path matches against a DomPrefixTree enum class PrefixTreeMatch { //! Only an exact path will match. @@ -44,17 +44,16 @@ namespace AZ::Dom explicit DomPrefixTree(AZStd::initializer_list> init); //! Visits a path and calls a visitor for each matching path and value. - void VisitPath(const Path& path, PrefixTreeMatch match, AZStd::function visitor) const; + void VisitPath(const Path& path, PrefixTreeMatch match, const AZStd::function& visitor) const; //! Visits a path and returns the most specific matching value, or null if none was found. T* ValueAtPath(const Path& path, PrefixTreeMatch match); //! \see ValueAtPath const T* ValueAtPath(const Path& path, PrefixTreeMatch match) const; //! Visits a path and returns the most specific matching value or some default value. - //! \note This returns a copy of a value. If T is expensive to copy, consider using ValueAtPath instead. - T ValueAtPathOrDefault(const Path& path, const T& defaultValue, PrefixTreeMatch match) const; + T ValueAtPathOrDefault(const Path& path, T&& defaultValue, PrefixTreeMatch match) const; //! Sets the value stored at path. - void SetValue(const Path& path, T value); + void SetValue(const Path& path, T&& value); //! Removes the value stored at path. If removeChildren is true, also removes any values stored at subpaths. void EraseValue(const Path& path, bool removedChildren = false); //! Removes all entries from this tree. @@ -63,7 +62,7 @@ namespace AZ::Dom private: struct Node { - AZStd::map m_values; + AZStd::unordered_map m_values; AZStd::optional m_data; }; diff --git a/Code/Framework/AzCore/AzCore/DOM/DomPrefixTree.inl b/Code/Framework/AzCore/AzCore/DOM/DomPrefixTree.inl index 6c0e342ead..41c0a02687 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomPrefixTree.inl +++ b/Code/Framework/AzCore/AzCore/DOM/DomPrefixTree.inl @@ -52,7 +52,7 @@ namespace AZ::Dom } template - void DomPrefixTree::VisitPath(const Path& path, PrefixTreeMatch match, AZStd::function visitor) const + void DomPrefixTree::VisitPath(const Path& path, PrefixTreeMatch match, const AZStd::function& visitor) const { const Node* rootNode = GetNodeForPath(path); if (rootNode == nullptr) @@ -166,14 +166,14 @@ namespace AZ::Dom } template - T DomPrefixTree::ValueAtPathOrDefault(const Path& path, const T& defaultValue, PrefixTreeMatch match) const + T DomPrefixTree::ValueAtPathOrDefault(const Path& path, T&& defaultValue, PrefixTreeMatch match) const { const T* value = ValueAtPath(path, match); - return value == nullptr ? defaultValue : *value; + return value == nullptr ? AZStd::forward(defaultValue) : *value; } template - void DomPrefixTree::SetValue(const Path& path, T value) + void DomPrefixTree::SetValue(const Path& path, T&& value) { Node* node = &m_rootNode; for (const PathEntry& entry : path) @@ -181,7 +181,7 @@ namespace AZ::Dom // Get or create an entry in this node node = &node->m_values[entry]; } - node->m_data = value; + node->m_data = AZStd::forward(value); } template diff --git a/Code/Framework/AzCore/Tests/DOM/DomPrefixTreeTests.cpp b/Code/Framework/AzCore/Tests/DOM/DomPrefixTreeTests.cpp index 715e06b603..49496b44f8 100644 --- a/Code/Framework/AzCore/Tests/DOM/DomPrefixTreeTests.cpp +++ b/Code/Framework/AzCore/Tests/DOM/DomPrefixTreeTests.cpp @@ -17,7 +17,7 @@ namespace AZ::Dom::Tests { DomPrefixTree tree; tree.SetValue(Path(), "root"); - EXPECT_EQ(*tree.ValueAtPath(Path(), PrefixTreeMatch::ExactPath), "root"); + EXPECT_EQ("root", *tree.ValueAtPath(Path(), PrefixTreeMatch::ExactPath)); } TEST_F(DomPrefixTreeTests, GetExactPath) @@ -29,14 +29,14 @@ namespace AZ::Dom::Tests tree.SetValue(Path("/foo/foo"), 1); tree.SetValue(Path("/foo/bar"), 2); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/0"), PrefixTreeMatch::ExactPath), 0); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/1"), PrefixTreeMatch::ExactPath), 42); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/foo"), PrefixTreeMatch::ExactPath), 1); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/bar"), PrefixTreeMatch::ExactPath), 2); + EXPECT_EQ(0, *tree.ValueAtPath(Path("/foo/0"), PrefixTreeMatch::ExactPath)); + EXPECT_EQ(42, *tree.ValueAtPath(Path("/foo/1"), PrefixTreeMatch::ExactPath)); + EXPECT_EQ(1, *tree.ValueAtPath(Path("/foo/foo"), PrefixTreeMatch::ExactPath)); + EXPECT_EQ(2, *tree.ValueAtPath(Path("/foo/bar"), PrefixTreeMatch::ExactPath)); - EXPECT_EQ(tree.ValueAtPath(Path(), PrefixTreeMatch::ExactPath), nullptr); - EXPECT_EQ(tree.ValueAtPath(Path("/foo"), PrefixTreeMatch::ExactPath), nullptr); - EXPECT_EQ(tree.ValueAtPath(Path("/foo/0/subpath"), PrefixTreeMatch::ExactPath), nullptr); + EXPECT_EQ(nullptr, tree.ValueAtPath(Path(), PrefixTreeMatch::ExactPath)); + EXPECT_EQ(nullptr, tree.ValueAtPath(Path("/foo"), PrefixTreeMatch::ExactPath)); + EXPECT_EQ(nullptr, tree.ValueAtPath(Path("/foo/0/subpath"), PrefixTreeMatch::ExactPath)); } TEST_F(DomPrefixTreeTests, GetSubpath) @@ -46,12 +46,12 @@ namespace AZ::Dom::Tests tree.SetValue(Path("/foo/0"), 0); tree.SetValue(Path("/foo/1"), 42); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/0/bar"), PrefixTreeMatch::SubpathsOnly), 0); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/0/bar/baz"), PrefixTreeMatch::SubpathsOnly), 0); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/1/0"), PrefixTreeMatch::SubpathsOnly), 42); + EXPECT_EQ(0, *tree.ValueAtPath(Path("/foo/0/bar"), PrefixTreeMatch::SubpathsOnly)); + EXPECT_EQ(0, *tree.ValueAtPath(Path("/foo/0/bar/baz"), PrefixTreeMatch::SubpathsOnly)); + EXPECT_EQ(42, *tree.ValueAtPath(Path("/foo/1/0"), PrefixTreeMatch::SubpathsOnly)); - EXPECT_EQ(tree.ValueAtPath(Path("/foo/0"), PrefixTreeMatch::SubpathsOnly), nullptr); - EXPECT_EQ(tree.ValueAtPath(Path("/foo/1"), PrefixTreeMatch::SubpathsOnly), nullptr); + EXPECT_EQ(nullptr, tree.ValueAtPath(Path("/foo/0"), PrefixTreeMatch::SubpathsOnly)); + EXPECT_EQ(nullptr, tree.ValueAtPath(Path("/foo/1"), PrefixTreeMatch::SubpathsOnly)); } TEST_F(DomPrefixTreeTests, GetPathOrSubpath) @@ -61,15 +61,15 @@ namespace AZ::Dom::Tests tree.SetValue(Path("/foo/0"), 0); tree.SetValue(Path("/foo/1"), 42); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/0"), PrefixTreeMatch::PathAndSubpaths), 0); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/0/bar"), PrefixTreeMatch::PathAndSubpaths), 0); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/0/bar/baz"), PrefixTreeMatch::PathAndSubpaths), 0); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/1"), PrefixTreeMatch::PathAndSubpaths), 42); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/1/0"), PrefixTreeMatch::PathAndSubpaths), 42); + EXPECT_EQ(0, *tree.ValueAtPath(Path("/foo/0"), PrefixTreeMatch::PathAndSubpaths)); + EXPECT_EQ(0, *tree.ValueAtPath(Path("/foo/0/bar"), PrefixTreeMatch::PathAndSubpaths)); + EXPECT_EQ(0, *tree.ValueAtPath(Path("/foo/0/bar/baz"), PrefixTreeMatch::PathAndSubpaths)); + EXPECT_EQ(42, *tree.ValueAtPath(Path("/foo/1"), PrefixTreeMatch::PathAndSubpaths)); + EXPECT_EQ(42, *tree.ValueAtPath(Path("/foo/1/0"), PrefixTreeMatch::PathAndSubpaths)); - EXPECT_EQ(tree.ValueAtPath(Path(), PrefixTreeMatch::PathAndSubpaths), nullptr); - EXPECT_EQ(tree.ValueAtPath(Path("/foo"), PrefixTreeMatch::PathAndSubpaths), nullptr); - EXPECT_EQ(tree.ValueAtPath(Path("/path/0"), PrefixTreeMatch::PathAndSubpaths), nullptr); + EXPECT_EQ(nullptr, tree.ValueAtPath(Path(), PrefixTreeMatch::PathAndSubpaths)); + EXPECT_EQ(nullptr, tree.ValueAtPath(Path("/foo"), PrefixTreeMatch::PathAndSubpaths)); + EXPECT_EQ(nullptr, tree.ValueAtPath(Path("/path/0"), PrefixTreeMatch::PathAndSubpaths)); } TEST_F(DomPrefixTreeTests, RemovePath) @@ -82,8 +82,8 @@ namespace AZ::Dom::Tests tree.EraseValue(Path("/foo")); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo"), PrefixTreeMatch::PathAndSubpaths), 20); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/0"), PrefixTreeMatch::PathAndSubpaths), 80); + EXPECT_EQ(20, *tree.ValueAtPath(Path("/foo"), PrefixTreeMatch::PathAndSubpaths)); + EXPECT_EQ(80, *tree.ValueAtPath(Path("/foo/0"), PrefixTreeMatch::PathAndSubpaths)); } TEST_F(DomPrefixTreeTests, RemovePathAndChildren) @@ -96,8 +96,8 @@ namespace AZ::Dom::Tests tree.EraseValue(Path("/foo"), true); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo"), PrefixTreeMatch::PathAndSubpaths), 20); - EXPECT_EQ(*tree.ValueAtPath(Path("/foo/0"), PrefixTreeMatch::PathAndSubpaths), 20); + EXPECT_EQ(20, *tree.ValueAtPath(Path("/foo"), PrefixTreeMatch::PathAndSubpaths)); + EXPECT_EQ(20, *tree.ValueAtPath(Path("/foo/0"), PrefixTreeMatch::PathAndSubpaths)); } TEST_F(DomPrefixTreeTests, ClearTree) @@ -109,7 +109,7 @@ namespace AZ::Dom::Tests tree.Clear(); - EXPECT_EQ(tree.ValueAtPathOrDefault(Path("/foo"), -10, PrefixTreeMatch::PathAndSubpaths), -10); + EXPECT_EQ(-10, tree.ValueAtPathOrDefault(Path("/foo"), -10, PrefixTreeMatch::PathAndSubpaths)); } TEST_F(DomPrefixTreeTests, Visit) @@ -141,27 +141,27 @@ namespace AZ::Dom::Tests tree.SetValue(Path("/bar/baz"), 2); tree.VisitPath(Path("/bar"), PrefixTreeMatch::ExactPath, visitorFn); - EXPECT_EQ(results.size(), 0); + EXPECT_EQ(0, results.size()); results.clear(); tree.VisitPath(Path("/foo/0"), PrefixTreeMatch::ExactPath, visitorFn); - EXPECT_EQ(results.size(), 1); + EXPECT_EQ(1, results.size()); EXPECT_TRUE(validateResult(Path("/foo/0"), 0)); results.clear(); tree.VisitPath(Path("/foo/1"), PrefixTreeMatch::ExactPath, visitorFn); - EXPECT_EQ(results.size(), 1); + EXPECT_EQ(1, results.size()); EXPECT_TRUE(validateResult(Path("/foo/1"), 42)); results.clear(); tree.VisitPath(Path("/foo"), PrefixTreeMatch::SubpathsOnly, visitorFn); - EXPECT_EQ(results.size(), 2); + EXPECT_EQ(2, results.size()); EXPECT_TRUE(validateResult(Path("/foo/0"), 0)); EXPECT_TRUE(validateResult(Path("/foo/1"), 42)); results.clear(); tree.VisitPath(Path("/foo"), PrefixTreeMatch::PathAndSubpaths, visitorFn); - EXPECT_EQ(results.size(), 3); + EXPECT_EQ(3, results.size()); EXPECT_TRUE(validateResult(Path("/foo"), 99)); EXPECT_TRUE(validateResult(Path("/foo/0"), 0)); EXPECT_TRUE(validateResult(Path("/foo/1"), 42));