wip: measure impact of event stack as a CRTP

src/c4/yml/event_handler_stack.hpp

@@ -35,97 +35,17 @@ struct EventHandlerStack
 
     using state = HandlerState;
 
-public:
-
-    detail::stack<state> 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<HandlerImpl const* C4_RESTRICT>(this)->template _has_any__<bits>())
 
-    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_
-
 };
 
 /** @} */

src/c4/yml/event_handler_stack_methods.hpp

@@ -0,0 +1,75 @@
+
+#ifndef _has_any_
+#error "must define _has_any_()"
+#endif
+
+    detail::stack<state> 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);
+    }

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<EventHandlerTree, EventHandlerTreeState>
+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__<bits>()
+
+    // 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__<bits>(); _c4dbgpf("node[{}]: enable {}", m_curr->node_id, #bits)
@@ -51,17 +65,20 @@ struct EventHandlerTree : public EventHandlerStack<EventHandlerTree, EventHandle
     #define _enable_(bits) _enable__<bits>()
     #define _disable_(bits) _disable__<bits>()
     #endif
-    #define _has_any_(bits) _has_any__<bits>()
+
+    Tree *C4_RESTRICT m_tree;
+    id_type m_id;
+
     /** @endcond */
 
 public:
 
     /** @name construction and resetting
      * @{ */
 
-    EventHandlerTree() : EventHandlerStack(), m_tree(), m_id(NONE) {}
-    EventHandlerTree(Callbacks const& cb) : EventHandlerStack(cb), m_tree(), m_id(NONE) {}
-    EventHandlerTree(Tree *tree, id_type id) : EventHandlerStack(tree->callbacks()), 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);
     }

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<EventHandlerYamlStd, EventHandlerYamlStdState>
+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,27 +69,33 @@ struct EventHandlerYamlStd : public EventHandlerStack<EventHandlerYamlStd, Event
 public:
 
     /** @cond dev */
+
+    // undefined at the end
+    #define _enable_(bits) _enable__<bits>()
+    #define _disable_(bits) _disable__<bits>()
+    #define _has_any_(bits) _has_any__<bits>()
+
+    // 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<EventSink> 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__<bits>()
-    #define _disable_(bits) _disable__<bits>()
-    #define _has_any_(bits) _has_any__<bits>()
     /** @endcond */
 
 public:
 
     /** @name construction and resetting
      * @{ */
 
-    EventHandlerYamlStd() : EventHandlerStack(), m_sink(), m_val_buffers() {}
-    EventHandlerYamlStd(Callbacks const& cb) : EventHandlerStack(cb), m_sink(), m_val_buffers() {}
-    EventHandlerYamlStd(EventSink *sink, Callbacks const& cb) : EventHandlerStack(cb), m_sink(sink), m_val_buffers()
+    EventHandlerYamlStd() : m_stack(), m_curr(), m_parent(), m_sink(), m_val_buffers() {}
+    EventHandlerYamlStd(Callbacks const& cb) : m_stack(cb), m_curr(), m_parent(), m_sink(), m_val_buffers() {}
+    EventHandlerYamlStd(EventSink *sink, Callbacks const& cb) : m_stack(cb), m_curr(), m_parent(), m_sink(sink), m_val_buffers()
     {
         reset();
     }