From bba5a816b0dd68c30fbe49d2a24ea368724812bc Mon Sep 17 00:00:00 2001 From: graehl Date: Wed, 16 Aug 2017 08:29:59 -0700 Subject: [PATCH] Node hash() and id() for faster is()-type sets A common use case is to transform each Node once even when *1 &1 DAG sharing exists in the input. This needs an 'already processed' set, which, using only is() would be quadratic in this use case. hashable ids (exposing the underlying pointer used by is) makes it linear time. For performance, part of node/detail/node.h is effectively exposed - the fact that it has a shared_ptr as its first member. Alternatively the implementation of id() could be made non-inline so the full definition of detail::node could be used. --- include/yaml-cpp/node/detail/node.h | 64 ++++++++++++++--------------- include/yaml-cpp/node/impl.h | 2 - include/yaml-cpp/node/node.h | 35 ++++++++++++++-- include/yaml-cpp/node/ptr.h | 20 +++++++++ 4 files changed, 81 insertions(+), 40 deletions(-) diff --git a/include/yaml-cpp/node/detail/node.h b/include/yaml-cpp/node/detail/node.h index 8a776f6..52bf90a 100644 --- a/include/yaml-cpp/node/detail/node.h +++ b/include/yaml-cpp/node/detail/node.h @@ -16,22 +16,19 @@ namespace YAML { namespace detail { -class node { +class node : public node_ref_id { public: - node() : m_pRef(new node_ref) {} - node(const node&) = delete; - node& operator=(const node&) = delete; + node() : node_ref_id(std::make_shared()) {} bool is(const node& rhs) const { return m_pRef == rhs.m_pRef; } - const node_ref* ref() const { return m_pRef.get(); } - bool is_defined() const { return m_pRef->is_defined(); } - const Mark& mark() const { return m_pRef->mark(); } - NodeType::value type() const { return m_pRef->type(); } + bool is_defined() const { return nref().is_defined(); } + const Mark& mark() const { return nref().mark(); } + NodeType::value type() const { return nref().type(); } - const std::string& scalar() const { return m_pRef->scalar(); } - const std::string& tag() const { return m_pRef->tag(); } - EmitterStyle::value style() const { return m_pRef->style(); } + const std::string& scalar() const { return nref().scalar(); } + const std::string& tag() const { return nref().tag(); } + EmitterStyle::value style() const { return nref().style(); } template bool equals(const T& rhs, shared_memory_holder pMemory); @@ -41,7 +38,7 @@ class node { if (is_defined()) return; - m_pRef->mark_defined(); + nref().mark_defined(); for (nodes::iterator it = m_dependencies.begin(); it != m_dependencies.end(); ++it) (*it)->mark_defined(); @@ -63,55 +60,55 @@ class node { void set_data(const node& rhs) { if (rhs.is_defined()) mark_defined(); - m_pRef->set_data(*rhs.m_pRef); + nref().set_data(rhs.nref()); } - void set_mark(const Mark& mark) { m_pRef->set_mark(mark); } + void set_mark(const Mark& mark) { nref().set_mark(mark); } void set_type(NodeType::value type) { if (type != NodeType::Undefined) mark_defined(); - m_pRef->set_type(type); + nref().set_type(type); } void set_null() { mark_defined(); - m_pRef->set_null(); + nref().set_null(); } void set_scalar(const std::string& scalar) { mark_defined(); - m_pRef->set_scalar(scalar); + nref().set_scalar(scalar); } void set_tag(const std::string& tag) { mark_defined(); - m_pRef->set_tag(tag); + nref().set_tag(tag); } // style void set_style(EmitterStyle::value style) { mark_defined(); - m_pRef->set_style(style); + nref().set_style(style); } // size/iterator - std::size_t size() const { return m_pRef->size(); } + std::size_t size() const { return nref().size(); } const_node_iterator begin() const { - return static_cast(*m_pRef).begin(); + return static_cast(nref()).begin(); } - node_iterator begin() { return m_pRef->begin(); } + node_iterator begin() { return nref().begin(); } const_node_iterator end() const { - return static_cast(*m_pRef).end(); + return static_cast(nref()).end(); } - node_iterator end() { return m_pRef->end(); } + node_iterator end() { return nref().end(); } // sequence void push_back(node& input, shared_memory_holder pMemory) { - m_pRef->push_back(input, pMemory); + nref().push_back(input, pMemory); input.add_dependency(*this); } void insert(node& key, node& value, shared_memory_holder pMemory) { - m_pRef->insert(key, value, pMemory); + nref().insert(key, value, pMemory); key.add_dependency(*this); value.add_dependency(*this); } @@ -122,44 +119,43 @@ class node { // NOTE: this returns a non-const node so that the top-level Node can wrap // it, and returns a pointer so that it can be NULL (if there is no such // key). - return static_cast(*m_pRef).get(key, pMemory); + return static_cast(nref()).get(key, pMemory); } template node& get(const Key& key, shared_memory_holder pMemory) { - node& value = m_pRef->get(key, pMemory); + node& value = nref().get(key, pMemory); value.add_dependency(*this); return value; } template bool remove(const Key& key, shared_memory_holder pMemory) { - return m_pRef->remove(key, pMemory); + return nref().remove(key, pMemory); } node* get(node& key, shared_memory_holder pMemory) const { // NOTE: this returns a non-const node so that the top-level Node can wrap // it, and returns a pointer so that it can be NULL (if there is no such // key). - return static_cast(*m_pRef).get(key, pMemory); + return static_cast(nref()).get(key, pMemory); } node& get(node& key, shared_memory_holder pMemory) { - node& value = m_pRef->get(key, pMemory); + node& value = nref().get(key, pMemory); key.add_dependency(*this); value.add_dependency(*this); return value; } bool remove(node& key, shared_memory_holder pMemory) { - return m_pRef->remove(key, pMemory); + return nref().remove(key, pMemory); } // map template void force_insert(const Key& key, const Value& value, shared_memory_holder pMemory) { - m_pRef->force_insert(key, value, pMemory); + nref().force_insert(key, value, pMemory); } private: - shared_node_ref m_pRef; typedef std::set nodes; nodes m_dependencies; }; diff --git a/include/yaml-cpp/node/impl.h b/include/yaml-cpp/node/impl.h index 20c487a..a68952a 100644 --- a/include/yaml-cpp/node/impl.h +++ b/include/yaml-cpp/node/impl.h @@ -441,8 +441,6 @@ inline void Node::force_insert(const Key& key, const Value& value) { m_pMemory); } -// free functions -inline bool operator==(const Node& lhs, const Node& rhs) { return lhs.is(rhs); } } #endif // NODE_IMPL_H_62B23520_7C8E_11DE_8A39_0800200C9A66 diff --git a/include/yaml-cpp/node/node.h b/include/yaml-cpp/node/node.h index 1ded7d2..675a58e 100644 --- a/include/yaml-cpp/node/node.h +++ b/include/yaml-cpp/node/node.h @@ -8,7 +8,7 @@ #endif #include - +#include #include "yaml-cpp/dll.h" #include "yaml-cpp/emitterstyle.h" #include "yaml-cpp/mark.h" @@ -21,11 +21,14 @@ namespace YAML { namespace detail { class node; class node_data; +class node_ref; struct iterator_value; } // namespace detail } // namespace YAML namespace YAML { + + class YAML_CPP_API Node { public: friend class NodeBuilder; @@ -76,8 +79,34 @@ class YAML_CPP_API Node { EmitterStyle::value Style() const; void SetStyle(EmitterStyle::value style); - // assignment + /// for defined nodes, a.id() == b.id() iff a.is(b) + /// (not part of standard yaml-cpp-0.5 api yet + void const* id() const { + assert(IsDefined()); + // node_ref_id is first base of detail::node + return ref_id(m_pNode)->id(); + } + + /// requires IsDefined for this and rhs (unlike 'is') + bool operator==(const Node& rhs) const { + assert(IsDefined()); + assert(rhs.IsDefined()); + return id() == rhs.id(); + } + + /// (not part of standard yaml-cpp-0.5 api yet + friend inline std::size_t hash_value(Node const& self) { + return self.hash(); + } + /// (not part of standard yaml-cpp-0.5 api yet + std::size_t hash() const { + assert(m_isValid); + return (std::size_t)id() >> (sizeof(std::shared_ptr) >= 8 ? 3 : 2); + } + bool is(const Node& rhs) const; + + // assignment template Node& operator=(const T& rhs); Node& operator=(const Node& rhs); @@ -134,8 +163,6 @@ class YAML_CPP_API Node { mutable detail::node* m_pNode; }; -YAML_CPP_API bool operator==(const Node& lhs, const Node& rhs); - YAML_CPP_API Node Clone(const Node& node); template diff --git a/include/yaml-cpp/node/ptr.h b/include/yaml-cpp/node/ptr.h index ce085dd..d92ea44 100644 --- a/include/yaml-cpp/node/ptr.h +++ b/include/yaml-cpp/node/ptr.h @@ -18,6 +18,26 @@ class node_data; class memory; class memory_holder; +class node_ref_id { +protected: + typedef std::shared_ptr Ptr; + Ptr m_pRef; + node_ref_id(Ptr && p) : m_pRef((Ptr &&)p) {} + node_ref_id(const node_ref_id&) = delete; + node_ref_id& operator=(const node_ref_id&) = delete; + detail::node_ref& nref() const { return *ref(); } +public: + detail::node_ref* ref() const { return (detail::node_ref*)m_pRef.get(); } + const void* id() const { + return m_pRef.get(); + } +}; + +inline node_ref_id const* ref_id(node const* node) { + // this could be static_cast if we pulled in the full definition of node + return reinterpret_cast(node); +} + typedef std::shared_ptr shared_node; typedef std::shared_ptr shared_node_ref; typedef std::shared_ptr shared_node_data;