Merge pull request #193 from hhenson/Performance-enhancements

hhenson · web-flow · commit a15608ada6f5 · 2025-11-10T17:22:32.000+04:00
Improve performance by caching access to the engine_time and clock
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1554,3 +1554,9 @@ Version 0.5.6 (09-11-2025)
 --------------------------
 
 * Add observer support and a few bug fixes.
+
+Version 0.5.7 (10-11-2025)
+--------------------------
+
+* Refactoring cleanup to prepare for further C++ improvements.
+* Performance optimisation for accessing evaluation_time.
diff --git a/cpp/include/hgraph/runtime/evaluation_engine.h b/cpp/include/hgraph/runtime/evaluation_engine.h
@@ -41,6 +41,9 @@ namespace hgraph {
 
         virtual void reset_push_node_requires_scheduling() = 0;
 
+        // Performance: Direct access to evaluation time for caching
+        [[nodiscard]] virtual const engine_time_t* evaluation_time_ptr() const = 0;
+
         static void register_with_nanobind(nb::module_ &m);
     };
 
@@ -69,6 +72,8 @@ namespace hgraph {
 
         void reset_push_node_requires_scheduling() override;
 
+        [[nodiscard]] const engine_time_t* evaluation_time_ptr() const override;
+
         static void register_with_nanobind(nb::module_ &m);
 
     private:
@@ -261,6 +266,9 @@ namespace hgraph {
 
         void update_next_scheduled_evaluation_time(engine_time_t scheduled_time) override;
 
+        // Performance: Direct access to evaluation time for caching
+        [[nodiscard]] const engine_time_t* evaluation_time_ptr() const override;
+
         static void register_with_nanobind(nb::module_ &m);
 
     private:
diff --git a/cpp/include/hgraph/types/graph.h b/cpp/include/hgraph/types/graph.h
@@ -65,6 +65,10 @@ namespace hgraph {
 
         void dispose_subgraph(int64_t start, int64_t end);
 
+        // Performance: Cached clock pointer and evaluation time reference set during initialization
+        [[nodiscard]] EngineEvaluationClock* cached_engine_clock() const { return _cached_engine_clock; }
+        [[nodiscard]] const engine_time_t* cached_evaluation_time_ptr() const { return _cached_evaluation_time_ptr; }
+
     protected:
         void initialise() override;
 
@@ -85,6 +89,11 @@ namespace hgraph {
         SenderReceiverState _receiver;
         engine_time_t _last_evaluation_time{MIN_DT};
         int64_t _push_source_nodes_end{-1};
+
+        // Performance optimization: Cache clock pointer and evaluation time pointer at initialization
+        // Set once when evaluation engine is assigned, never changes
+        EngineEvaluationClock* _cached_engine_clock{nullptr};
+        const engine_time_t* _cached_evaluation_time_ptr{nullptr};
     };
 } // namespace hgraph
 
diff --git a/cpp/include/hgraph/types/node.h b/cpp/include/hgraph/types/node.h
@@ -221,7 +221,11 @@ namespace hgraph {
 
         void set_error_output(time_series_output_ptr value);
 
+        // Performance optimization: provide access to cached evaluation time pointer
+        [[nodiscard]] const engine_time_t* cached_evaluation_time_ptr() const { return _cached_evaluation_time_ptr; }
+
         friend struct Graph;
+        friend struct NodeScheduler;
 
         void add_start_input(nb::ref<TimeSeriesReferenceInput> input);
 
@@ -266,6 +270,10 @@ namespace hgraph {
         // Cache for these calculated values.
         std::vector<nb::ref<TimeSeriesInput> > _check_valid_inputs;
         std::vector<nb::ref<TimeSeriesInput> > _check_all_valid_inputs;
+
+        // Performance optimization: Cache evaluation time pointer from graph
+        // Set once when graph is assigned to node, never changes
+        const engine_time_t* _cached_evaluation_time_ptr{nullptr};
     };
 } // namespace hgraph
 
diff --git a/cpp/src/cpp/runtime/evaluation_engine.cpp b/cpp/src/cpp/runtime/evaluation_engine.cpp
@@ -86,6 +86,10 @@ namespace hgraph {
         _engine_evalaution_clock->reset_push_node_requires_scheduling();
     }
 
+    const engine_time_t* EngineEvaluationClockDelegate::evaluation_time_ptr() const {
+        return _engine_evalaution_clock->evaluation_time_ptr();
+    }
+
     void EngineEvaluationClockDelegate::register_with_nanobind(nb::module_ &m) {
         nb::class_ < EngineEvaluationClockDelegate, EngineEvaluationClock > (m, "EngineEvaluationClockDelegate")
                 .def(nb::init<EngineEvaluationClock::ptr>());
@@ -298,6 +302,8 @@ namespace hgraph {
                 std::max(next_cycle_evaluation_time(), std::min(_next_scheduled_evaluation_time, scheduled_time));
     }
 
+    const engine_time_t *BaseEvaluationClock::evaluation_time_ptr() const { return &_evaluation_time; }
+
     void BaseEvaluationClock::register_with_nanobind(nb::module_ &m) {
         nb::class_ < BaseEvaluationClock, EngineEvaluationClock > (m, "BaseEvaluationClock");
     }
diff --git a/cpp/src/cpp/types/base_time_series.cpp b/cpp/src/cpp/types/base_time_series.cpp
@@ -27,8 +27,12 @@ namespace hgraph {
     void BaseTimeSeriesOutput::reset_parent_or_node() { _parent_ts_or_node.reset(); }
     
     // Implement re_parent methods
-    void BaseTimeSeriesOutput::re_parent(const node_ptr &parent) { _parent_ts_or_node = parent; }
-    void BaseTimeSeriesOutput::re_parent(const TimeSeriesType::ptr &parent) { _parent_ts_or_node = parent; }
+    void BaseTimeSeriesOutput::re_parent(const node_ptr &parent) {
+        _parent_ts_or_node = parent;
+    }
+    void BaseTimeSeriesOutput::re_parent(const TimeSeriesType::ptr &parent) {
+        _parent_ts_or_node = parent;
+    }
 
     // TimeSeriesType helper methods
     TimeSeriesType::ptr &BaseTimeSeriesOutput::_parent_time_series() const {
@@ -119,9 +123,10 @@ namespace hgraph {
     }
 
     bool BaseTimeSeriesOutput::modified() const {
-        auto g = owning_graph();
-        if (!g) { return false; }
-        return g->evaluation_clock()->evaluation_time() == _last_modified_time;
+        auto n = owning_node();
+        if (n.get() == nullptr) { return false; }
+        // Use cached evaluation time pointer from node for performance
+        return *n->cached_evaluation_time_ptr() == _last_modified_time;
     }
 
     bool BaseTimeSeriesOutput::valid() const { return _last_modified_time > MIN_DT; }
@@ -135,24 +140,26 @@ namespace hgraph {
     void BaseTimeSeriesOutput::mark_invalid() {
         if (_last_modified_time > MIN_DT) {
             _last_modified_time = MIN_DT;
-            auto g = owning_graph();
-            if (g) {
-                _notify(g->evaluation_clock()->evaluation_time());
+            auto n = owning_node();
+            if (n.get() != nullptr) {
+                // Use cached evaluation time pointer from node for performance
+                _notify(*n->cached_evaluation_time_ptr());
             } else {
-                // Owning graph not yet attached; skip notify to avoid dereferencing null during start/recover
+                // Owning node not yet attached; skip notify to avoid dereferencing null during start/recover
             }
         }
     }
 
     void BaseTimeSeriesOutput::mark_modified() {
         if (has_parent_or_node()) {
-            auto g = owning_graph();
-            if (g != nullptr) {
-                mark_modified(g->evaluation_clock()->evaluation_time());
+            auto n = owning_node();
+            if (n.get() != nullptr) {
+                // Use cached evaluation time pointer from node for performance
+                mark_modified(*n->cached_evaluation_time_ptr());
             } else {
-                // Graph not yet attached; mark with a maximal time to preserve monotonicity without dereferencing
+                // Owning node not yet attached; mark with a maximal time to preserve monotonicity without dereferencing
                 // This is a bad situation, I would probably prefer to find out why,
-                // TODO: find the root cause of why this could be called without a bound graph.
+                // TODO: find the root cause of why this could be called without a bound node.
             }
         } else {
             mark_modified(MAX_ET);
@@ -203,8 +210,12 @@ namespace hgraph {
     void BaseTimeSeriesInput::reset_parent_or_node() { _parent_ts_or_node.reset(); }
     
     // Implement re_parent methods
-    void BaseTimeSeriesInput::re_parent(const node_ptr &parent) { _parent_ts_or_node = parent; }
-    void BaseTimeSeriesInput::re_parent(const TimeSeriesType::ptr &parent) { _parent_ts_or_node = parent; }
+    void BaseTimeSeriesInput::re_parent(const node_ptr &parent) {
+        _parent_ts_or_node = parent;
+    }
+    void BaseTimeSeriesInput::re_parent(const TimeSeriesType::ptr &parent) {
+        _parent_ts_or_node = parent;
+    }
 
     // TimeSeriesType helper methods
     TimeSeriesType::ptr &BaseTimeSeriesInput::_parent_time_series() const {
@@ -301,9 +312,10 @@ namespace hgraph {
         // - The input was previously bound (rebinding case), OR
         // - The new output is valid
         // This matches the Python implementation: (was_bound or self._output.valid)
-        if ((owning_node()->is_started() || owning_node()->is_starting()) && _output.get() && (was_bound || _output->
-                valid())) {
-            _sample_time = owning_graph()->evaluation_clock()->evaluation_time();
+        auto n = owning_node();
+        if ((n->is_started() || n->is_starting()) && _output.get() && (was_bound || _output->valid())) {
+            // Use cached evaluation time pointer from node for performance
+            _sample_time = *n->cached_evaluation_time_ptr();
             if (active()) {
                 notify(_sample_time);
                 // TODO: This might belong to make_active, or not? There is a race with setting sample_time too.
@@ -325,11 +337,13 @@ namespace hgraph {
         if (bound()) {
             do_un_bind_output(unbind_refs);
 
-            if (owning_node()->is_started() && was_valid) {
-                _sample_time = owning_graph()->evaluation_clock()->evaluation_time();
+            auto n = owning_node();
+            if (n->is_started() && was_valid) {
+                // Use cached evaluation time pointer from node for performance
+                _sample_time = *n->cached_evaluation_time_ptr();
                 if (active()) {
                     // Notify as the state of the node has changed from bound to un_bound
-                    owning_node()->notify(_sample_time);
+                    n->notify(_sample_time);
                 }
             }
         }
@@ -433,7 +447,10 @@ namespace hgraph {
     engine_time_t BaseTimeSeriesInput::sample_time() const { return _sample_time; }
 
     bool BaseTimeSeriesInput::sampled() const {
-        return _sample_time != MIN_DT && _sample_time == owning_graph()->evaluation_clock()->evaluation_time();
+        auto n = owning_node();
+        if (n.get() == nullptr) { return false; }
+        // Use cached evaluation time pointer from node for performance
+        return _sample_time != MIN_DT && _sample_time == *n->cached_evaluation_time_ptr();
     }
 
     time_series_reference_output_ptr BaseTimeSeriesInput::reference_output() const { return _reference_output; }
diff --git a/cpp/src/cpp/types/graph.cpp b/cpp/src/cpp/types/graph.cpp
@@ -48,6 +48,10 @@ namespace hgraph {
         }
         _evaluation_engine = std::move(value);
 
+        // Cache the clock pointer and evaluation time pointer once at initialization for performance
+        _cached_engine_clock = _evaluation_engine->engine_evaluation_clock().get();
+        _cached_evaluation_time_ptr = _cached_engine_clock->evaluation_time_ptr();
+
         if (_push_source_nodes_end > 0) { _receiver.set_evaluation_clock(evaluation_engine_clock()); }
     }
 
@@ -56,8 +60,8 @@ namespace hgraph {
     void Graph::schedule_node(int64_t node_ndx, engine_time_t when) { schedule_node(node_ndx, when, false); }
 
     void Graph::schedule_node(int64_t node_ndx, engine_time_t when, bool force_set) {
-        auto clock = this->evaluation_engine_clock();
-        auto et = clock->evaluation_time();
+        // Use cached evaluation time pointer (set at initialization) - direct memory access
+        auto et = *_cached_evaluation_time_ptr;
 
         // Match Python: just throw if scheduling in the past
         if (when < et) {
@@ -72,16 +76,17 @@ namespace hgraph {
 
         auto &st = this->_schedule[node_ndx];
         if (force_set || st <= et || st > when) { st = when; }
-        clock->update_next_scheduled_evaluation_time(when);
+        _cached_engine_clock->update_next_scheduled_evaluation_time(when);
     }
 
     std::vector<engine_time_t> &Graph::schedule() { return _schedule; }
 
     void Graph::evaluate_graph() {
         NotifyGraphEvaluation nge{evaluation_engine(), graph_ptr{this}};
 
-        engine_time_t now = evaluation_engine_clock()->evaluation_time();
-        auto clock = evaluation_engine_clock();
+        // Use cached pointers (set at initialization) for direct memory access
+        auto clock = _cached_engine_clock;
+        engine_time_t now = *_cached_evaluation_time_ptr;
         auto &nodes = _nodes;
         auto &schedule = _schedule;
 
diff --git a/cpp/src/cpp/types/node.cpp b/cpp/src/cpp/types/node.cpp
@@ -538,8 +538,10 @@ namespace hgraph {
     bool NodeScheduler::is_scheduled() const { return !_scheduled_events.empty() || !_alarm_tags.empty(); }
 
     bool NodeScheduler::is_scheduled_now() const {
-        return !_scheduled_events.empty() &&
-               _scheduled_events.begin()->first == _node->graph()->evaluation_clock()->evaluation_time();
+        if (_scheduled_events.empty()) return false;
+        // Use node's cached evaluation time pointer - direct memory access, no pointer chasing
+        auto eval_time = *_node->_cached_evaluation_time_ptr;
+        return _scheduled_events.begin()->first == eval_time;
     }
 
     bool NodeScheduler::has_tag(const std::string &tag) const { return _tags.contains(tag); }
@@ -578,7 +580,9 @@ namespace hgraph {
         }
 
         auto is_started{_node->is_started()};
-        auto now_{is_started ? _node->graph()->evaluation_clock()->evaluation_time() : MIN_DT};
+        // Use node's cached evaluation time pointer - direct memory access, no pointer chasing
+        engine_time_t now_ = is_started ? *_node->_cached_evaluation_time_ptr : MIN_DT;
+
         if (when > now_) {
             _tags[tag.value_or("")] = when;
             auto current_first = !_scheduled_events.empty() ? _scheduled_events.begin()->first : MAX_DT;
@@ -592,7 +596,9 @@ namespace hgraph {
     }
 
     void NodeScheduler::schedule(engine_time_delta_t when, std::optional<std::string> tag, bool on_wall_clock) {
-        auto when_{_node->graph()->evaluation_clock()->evaluation_time() + when};
+        // Use node's cached evaluation time pointer - direct memory access, no pointer chasing
+        auto eval_time = *_node->_cached_evaluation_time_ptr;
+        auto when_ = eval_time + when;
         schedule(when_, std::move(tag), on_wall_clock);
     }
 
@@ -622,7 +628,8 @@ namespace hgraph {
 
     void NodeScheduler::advance() {
         if (_scheduled_events.empty()) { return; }
-        auto until = _node->graph()->evaluation_clock()->evaluation_time();
+        // Use node's cached evaluation time pointer - direct memory access, no pointer chasing
+        auto until = *_node->_cached_evaluation_time_ptr;
         // Note: empty string is considered smallest in std::string comparison,
         // so upper_bound will correctly find elements <= until regardless of tag value
         _scheduled_events.erase(_scheduled_events.begin(), _scheduled_events.upper_bound({until, VERY_LARGE_STRING}));
@@ -690,19 +697,27 @@ namespace hgraph {
         if (is_started() || is_starting()) {
             // When a node is starting, it might be notified with a historical time (from inputs that ticked in the past).
             // We should schedule for MAX(modified_time, current_evaluation_time) to avoid scheduling in the past.
-            auto eval_time = graph()->evaluation_clock()->evaluation_time();
+            // Use node's cached evaluation time pointer - direct memory access, no pointer chasing
+            auto eval_time = *_cached_evaluation_time_ptr;
             auto schedule_time = std::max(modified_time, eval_time);
             graph()->schedule_node(node_ndx(), schedule_time);
         } else {
             scheduler()->schedule(MIN_ST, "start");
         }
     }
 
-    void Node::notify() { notify(graph()->evaluation_clock()->evaluation_time()); }
+    void Node::notify() {
+        // Use node's cached evaluation time pointer - direct memory access, no pointer chasing
+        auto eval_time = *_cached_evaluation_time_ptr;
+        notify(eval_time);
+    }
 
     void Node::notify_next_cycle() {
         if (is_started() || is_starting()) {
-            graph()->schedule_node(node_ndx(), graph()->evaluation_clock()->next_cycle_evaluation_time());
+            // Use node's cached evaluation time pointer and calculate next_cycle directly
+            // next_cycle_evaluation_time is just evaluation_time + MIN_TD
+            auto next_time = *_cached_evaluation_time_ptr + MIN_TD;
+            graph()->schedule_node(node_ndx(), next_time);
         } else {
             notify();
         }
@@ -729,7 +744,11 @@ namespace hgraph {
     graph_ptr Node::graph() { return _graph; }
     graph_ptr Node::graph() const { return _graph; }
 
-    void Node::set_graph(graph_ptr value) { _graph = value; }
+    void Node::set_graph(graph_ptr value) {
+        _graph = value;
+        // Cache the evaluation time pointer from the graph for performance
+        _cached_evaluation_time_ptr = value->cached_evaluation_time_ptr();
+    }
 
     time_series_bundle_input_ptr Node::input() { return _input; }
     time_series_bundle_input_ptr Node::input() const { return _input; }

Original file line number	Diff line number	Diff line change
`@@ -86,6 +86,10 @@ namespace hgraph {`
`86`	`86`	`_engine_evalaution_clock->reset_push_node_requires_scheduling();`
`87`	`87`	`}`
`88`	`88`
	`89`	`+ const engine_time_t* EngineEvaluationClockDelegate::evaluation_time_ptr() const {`
	`90`	`+ return _engine_evalaution_clock->evaluation_time_ptr();`
	`91`	`+ }`
	`92`	`+`
`89`	`93`	`void EngineEvaluationClockDelegate::register_with_nanobind(nb::module_ &m) {`
`90`	`94`	`nb::class_ < EngineEvaluationClockDelegate, EngineEvaluationClock > (m, "EngineEvaluationClockDelegate")`
`91`	`95`	`.def(nb::init<EngineEvaluationClock::ptr>());`
`@@ -298,6 +302,8 @@ namespace hgraph {`
`298`	`302`	`std::max(next_cycle_evaluation_time(), std::min(_next_scheduled_evaluation_time, scheduled_time));`
`299`	`303`	`}`
`300`	`304`
	`305`	`+ const engine_time_t *BaseEvaluationClock::evaluation_time_ptr() const { return &_evaluation_time; }`
	`306`	`+`
`301`	`307`	`void BaseEvaluationClock::register_with_nanobind(nb::module_ &m) {`
`302`	`308`	`nb::class_ < BaseEvaluationClock, EngineEvaluationClock > (m, "BaseEvaluationClock");`
`303`	`309`	`}`