From 8ce0671cd1dc55ec61379fb6cba5cb4449873684 Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Mon, 6 May 2024 00:51:00 +0200 Subject: [PATCH] wip: measure impact of event stack as a CRTP --- src/c4/yml/event_handler_stack.hpp | 88 +------------------- src/c4/yml/event_handler_stack_methods.hpp | 75 +++++++++++++++++ src/c4/yml/event_handler_tree.hpp | 57 ++++++++----- test/test_suite/test_suite_event_handler.hpp | 39 +++++---- 4 files changed, 137 insertions(+), 122 deletions(-) create mode 100644 src/c4/yml/event_handler_stack_methods.hpp diff --git a/src/c4/yml/event_handler_stack.hpp b/src/c4/yml/event_handler_stack.hpp index 34b0e4ea..bdb2d4cd 100644 --- a/src/c4/yml/event_handler_stack.hpp +++ b/src/c4/yml/event_handler_stack.hpp @@ -35,97 +35,17 @@ struct EventHandlerStack using state = HandlerState; -public: - - detail::stack m_stack; - state *C4_RESTRICT m_curr; ///< current stack level: top of the stack. cached here for easier access. - state *C4_RESTRICT m_parent; ///< parent of the current stack level. - -protected: - EventHandlerStack() : m_stack(), m_curr(), m_parent() {} EventHandlerStack(Callbacks const& cb) : m_stack(cb), m_curr(), m_parent() {} -protected: - - void _stack_reset_root() - { - m_stack.clear(); - m_stack.push({}); - m_parent = nullptr; - m_curr = &m_stack.top(); - } - - void _stack_reset_non_root() - { - m_stack.clear(); - m_stack.push({}); // parent - m_stack.push({}); // node - m_parent = &m_stack.top(1); - m_curr = &m_stack.top(); - } - - void _stack_push() - { - m_stack.push_top(); - m_parent = &m_stack.top(1); // don't use m_curr. watch out for relocations inside the prev push - m_curr = &m_stack.top(); - m_curr->reset_after_push(); - } - - void _stack_pop() - { - _RYML_CB_ASSERT(m_stack.m_callbacks, m_parent); - _RYML_CB_ASSERT(m_stack.m_callbacks, m_stack.size() > 1); - m_parent->reset_before_pop(*m_curr); - m_stack.pop(); - m_parent = m_stack.size() > 1 ? &m_stack.top(1) : nullptr; - m_curr = &m_stack.top(); - #ifdef RYML_DBG - if(m_parent) - _c4dbgpf("popped! top is now node={} (parent={})", m_curr->node_id, m_parent->node_id); - else - _c4dbgpf("popped! top is now node={} @ ROOT", m_curr->node_id); - #endif - } - -protected: - - // undefined at the end #define _has_any_(bits) (static_cast(this)->template _has_any__()) - bool _stack_should_push_on_begin_doc() const - { - const bool is_root = (m_stack.size() == 1u); - return is_root && (_has_any_(DOC|VAL|MAP|SEQ) || m_curr->has_children); - } - - bool _stack_should_pop_on_end_doc() const - { - const bool is_root = (m_stack.size() == 1u); - return !is_root && _has_any_(DOC); - } - -public: - - /** Check whether the current parse tokens are trailing on the - * previous doc, and raise an error if they are. This function is - * called by the parse engine (not the event handler) before a doc - * is started. */ - void check_trailing_doc_token() const - { - const bool is_root = (m_stack.size() == 1u); - const bool isndoc = (m_curr->flags & NDOC) != 0; - const bool suspicious = _has_any_(MAP|SEQ|VAL); - _c4dbgpf("target={} isroot={} suspicious={} ndoc={}", m_curr->node_id, is_root, suspicious, isndoc); - if((is_root || _has_any_(DOC)) && suspicious && !isndoc) - _RYML_CB_ERR_(m_stack.m_callbacks, "parse error", m_curr->pos); - } - -protected: + // We define the stack methods by including directly in here. + // Previously this was done using a CRTP, but ultimately that was + // costing in compilation time, so direct inclusion is preferred: + #include "./event_handler_stack_methods.hpp" #undef _has_any_ - }; /** @} */ diff --git a/src/c4/yml/event_handler_stack_methods.hpp b/src/c4/yml/event_handler_stack_methods.hpp new file mode 100644 index 00000000..ef849ee6 --- /dev/null +++ b/src/c4/yml/event_handler_stack_methods.hpp @@ -0,0 +1,75 @@ + +#ifndef _has_any_ +#error "must define _has_any_()" +#endif + + detail::stack m_stack; + state *C4_RESTRICT m_curr; ///< current stack level: top of the stack. cached here for easier access. + state *C4_RESTRICT m_parent; ///< parent of the current stack level. + + void _stack_reset_root() + { + m_stack.clear(); + m_stack.push({}); + m_parent = nullptr; + m_curr = &m_stack.top(); + } + + void _stack_reset_non_root() + { + m_stack.clear(); + m_stack.push({}); // parent + m_stack.push({}); // node + m_parent = &m_stack.top(1); + m_curr = &m_stack.top(); + } + + void _stack_push() + { + m_stack.push_top(); + m_parent = &m_stack.top(1); // don't use m_curr. watch out for relocations inside the prev push + m_curr = &m_stack.top(); + m_curr->reset_after_push(); + } + + void _stack_pop() + { + _RYML_CB_ASSERT(m_stack.m_callbacks, m_parent); + _RYML_CB_ASSERT(m_stack.m_callbacks, m_stack.size() > 1); + m_parent->reset_before_pop(*m_curr); + m_stack.pop(); + m_parent = m_stack.size() > 1 ? &m_stack.top(1) : nullptr; + m_curr = &m_stack.top(); + #ifdef RYML_DBG + if(m_parent) + _c4dbgpf("popped! top is now node={} (parent={})", m_curr->node_id, m_parent->node_id); + else + _c4dbgpf("popped! top is now node={} @ ROOT", m_curr->node_id); + #endif + } + + bool _stack_should_push_on_begin_doc() const + { + const bool is_root = (m_stack.size() == 1u); + return is_root && (_has_any_(DOC|VAL|MAP|SEQ) || m_curr->has_children); + } + + bool _stack_should_pop_on_end_doc() const + { + const bool is_root = (m_stack.size() == 1u); + return !is_root && _has_any_(DOC); + } + + /** Check whether the current parse tokens are trailing on the + * previous doc, and raise an error if they are. This function is + * called by the parse engine (not the event handler) before a doc + * is started. */ + void check_trailing_doc_token() const + { + const bool is_root = (m_stack.size() == 1u); + const bool isndoc = (m_curr->flags & NDOC) != 0; + const bool suspicious = _has_any_(MAP|SEQ|VAL); + _c4dbgpf("target={} isroot={} suspicious={} ndoc={}", m_curr->node_id, is_root, suspicious, isndoc); + if((is_root || _has_any_(DOC)) && suspicious && !isndoc) + _RYML_CB_ERR_(m_stack.m_callbacks, "parse error", m_curr->pos); + } diff --git a/src/c4/yml/event_handler_tree.hpp b/src/c4/yml/event_handler_tree.hpp index 5852e2c6..8454e20e 100644 --- a/src/c4/yml/event_handler_tree.hpp +++ b/src/c4/yml/event_handler_tree.hpp @@ -1,12 +1,26 @@ #ifndef _C4_YML_EVENT_HANDLER_TREE_HPP_ #define _C4_YML_EVENT_HANDLER_TREE_HPP_ -#ifndef _C4_YML_TREE_HPP_ -#include "c4/yml/tree.hpp" +#ifndef _C4_YML_DETAIL_STACK_HPP_ +#include "c4/yml/detail/stack.hpp" +#endif + +#ifndef _C4_YML_DETAIL_PARSER_DBG_HPP_ +#include "c4/yml/detail/parser_dbg.hpp" +#endif + +#ifndef _C4_YML_PARSER_STATE_HPP_ +#include "c4/yml/parser_state.hpp" +#endif + +#ifdef RYML_DBG +#ifndef _C4_YML_DETAIL_PRINT_HPP_ +#include "c4/yml/detail/print.hpp" +#endif #endif -#ifndef _C4_YML_EVENT_HANDLER_STACK_HPP_ -#include "c4/yml/event_handler_stack.hpp" +#ifndef _C4_YML_TREE_HPP_ +#include "c4/yml/tree.hpp" #endif C4_SUPPRESS_WARNING_MSVC_WITH_PUSH(4702) // unreachable code @@ -17,32 +31,32 @@ namespace yml { /** @addtogroup doc_event_handlers * @{ */ - -/** The stack state needed specifically by @ref EventHandlerTree */ -struct EventHandlerTreeState : public ParserState -{ - NodeData *tr_data; -}; - - /** The event handler to create a ryml @ref Tree. See the * documentation for @ref doc_event_handlers, which has important * notes about the event model used by rapidyaml. */ -struct EventHandlerTree : public EventHandlerStack +struct EventHandlerTree { /** @name types * @{ */ - using state = EventHandlerTreeState; + struct state : public ParserState + { + NodeData *tr_data; + }; /** @} */ public: /** @cond dev */ - Tree *C4_RESTRICT m_tree; - id_type m_id; + + #define _has_any_(bits) _has_any__() + + // We define the stack methods by including directly in here. + // Previously this was done using a CRTP, but ultimately that was + // costing in compilation time, so direct inclusion is preferred: + #include "./event_handler_stack_methods.hpp" #if RYML_DBG #define _enable_(bits) _enable__(); _c4dbgpf("node[{}]: enable {}", m_curr->node_id, #bits) @@ -51,7 +65,10 @@ struct EventHandlerTree : public EventHandlerStack() #define _disable_(bits) _disable__() #endif - #define _has_any_(bits) _has_any__() + + Tree *C4_RESTRICT m_tree; + id_type m_id; + /** @endcond */ public: @@ -59,9 +76,9 @@ struct EventHandlerTree : public EventHandlerStackcallbacks()), m_tree(tree), m_id(id) + EventHandlerTree() : m_stack(), m_curr(), m_parent(), m_tree(), m_id(NONE) {} + EventHandlerTree(Callbacks const& cb) : m_stack(cb), m_curr(), m_parent(), m_tree(), m_id(NONE) {} + EventHandlerTree(Tree *tree, id_type id) : m_stack(tree->callbacks()), m_curr(), m_parent(), m_tree(tree), m_id(id) { reset(tree, id); } diff --git a/test/test_suite/test_suite_event_handler.hpp b/test/test_suite/test_suite_event_handler.hpp index b1494ee0..1eb5d59e 100644 --- a/test/test_suite/test_suite_event_handler.hpp +++ b/test/test_suite/test_suite_event_handler.hpp @@ -28,30 +28,27 @@ namespace yml { /** @addtogroup doc_event_handlers * @{ */ -/** The stack state needed specifically by @ref EventHandlerYamlStd */ -struct EventHandlerYamlStdState : public ParserState -{ - NodeData ev_data; -}; - /** The event handler producing standard YAML events as used in the * [YAML test suite](https://github.com/yaml/yaml-test-suite). * See the documentation for @ref doc_event_handlers, which has * important notes about the event model used by rapidyaml. * - * This classe is used only in the CI of this project, and in the + * This class is used only in the CI of this project, and in the * application used as part of the [standard YAML - * playground](https://play.yaml.io/main/parser). This is not part of + * playground](https://play.yaml.io/main/parser). It is not part of * the library and is not installed. * */ -struct EventHandlerYamlStd : public EventHandlerStack +struct EventHandlerYamlStd { /** @name types * @{ */ // our internal state must inherit from parser state - using state = EventHandlerYamlStdState; + struct state : public ParserState + { + NodeData ev_data; + }; struct EventSink { @@ -72,17 +69,23 @@ struct EventHandlerYamlStd : public EventHandlerStack() + #define _disable_(bits) _disable__() + #define _has_any_(bits) _has_any__() + + // We define the stack methods by including directly in here. + // Previously this was done using a CRTP, but ultimately that was + // costing in compilation time, so direct inclusion is preferred: + #include "c4/yml/event_handler_stack_methods.hpp" + EventSink *C4_RESTRICT m_sink; std::vector m_val_buffers; char m_key_tag_buf[256]; char m_val_tag_buf[256]; TagDirective m_tag_directives[RYML_MAX_TAG_DIRECTIVES]; std::string m_arena; - - // undefined at the end - #define _enable_(bits) _enable__() - #define _disable_(bits) _disable__() - #define _has_any_(bits) _has_any__() /** @endcond */ public: @@ -90,9 +93,9 @@ struct EventHandlerYamlStd : public EventHandlerStack