From 142541c2796251979dd33fc3454e8316f2dfca0e Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Sat, 16 Sep 2023 10:07:27 +0200 Subject: [PATCH] Geometry Nodes: create local tree logger only when something is logged Currently, we create a logger for every compute context that is evaluated, even when we don't actually log anything in that context (due to other optimizations). Now, the logger is not created eagerly anymore. This can be especially benefitial when there are many compute contexts that should not log anything, e.g. if there is a repeat zone with many iterations. In an extrem case I measured a speedup for the modifier evaluation from 24ms to 14ms. --- source/blender/nodes/NOD_geometry_exec.hh | 2 +- .../nodes/NOD_geometry_nodes_lazy_function.hh | 26 +++++++++++--- .../intern/geometry_nodes_lazy_function.cc | 34 +++++++++++-------- 3 files changed, 42 insertions(+), 20 deletions(-) diff --git a/source/blender/nodes/NOD_geometry_exec.hh b/source/blender/nodes/NOD_geometry_exec.hh index cb2e24aafa8..7d98439359c 100644 --- a/source/blender/nodes/NOD_geometry_exec.hh +++ b/source/blender/nodes/NOD_geometry_exec.hh @@ -181,7 +181,7 @@ class GeoNodeExecParams { geo_eval_log::GeoTreeLogger *get_local_tree_logger() const { - return this->local_user_data()->tree_logger; + return this->local_user_data()->try_get_tree_logger(); } /** diff --git a/source/blender/nodes/NOD_geometry_nodes_lazy_function.hh b/source/blender/nodes/NOD_geometry_nodes_lazy_function.hh index a3992546377..032ab920a47 100644 --- a/source/blender/nodes/NOD_geometry_nodes_lazy_function.hh +++ b/source/blender/nodes/NOD_geometry_nodes_lazy_function.hh @@ -194,13 +194,31 @@ struct GeoNodesLFUserData : public lf::UserData { }; struct GeoNodesLFLocalUserData : public lf::LocalUserData { - public: + private: + GeoNodesLFUserData &user_data_; /** - * Thread-local logger for the current node tree in the current compute context. + * Thread-local logger for the current node tree in the current compute context. It is only + * instantiated when it is actually used and then cached for the current thread. */ - geo_eval_log::GeoTreeLogger *tree_logger = nullptr; + mutable std::optional tree_logger_; - GeoNodesLFLocalUserData(GeoNodesLFUserData &user_data); + public: + GeoNodesLFLocalUserData(GeoNodesLFUserData &user_data) : user_data_(user_data) {} + + /** + * Get the current tree logger. This method is not thread-safe, each thread is supposed to have + * a separate logger. + */ + geo_eval_log::GeoTreeLogger *try_get_tree_logger() const + { + if (!tree_logger_.has_value()) { + this->ensure_tree_logger(); + } + return *tree_logger_; + } + + private: + void ensure_tree_logger() const; }; /** diff --git a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc index e0fcf18e7f9..800934415fa 100644 --- a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc +++ b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc @@ -328,9 +328,8 @@ class LazyFunctionForGeometryNode : public LazyFunction { node_.typeinfo->geometry_node_execute(geo_params); geo_eval_log::TimePoint end_time = geo_eval_log::Clock::now(); - if (local_user_data.tree_logger) { - local_user_data.tree_logger->node_execution_times.append( - {node_.identifier, start_time, end_time}); + if (geo_eval_log::GeoTreeLogger *tree_logger = local_user_data.try_get_tree_logger()) { + tree_logger->node_execution_times.append({node_.identifier, start_time, end_time}); } } @@ -779,7 +778,8 @@ class LazyFunctionForViewerNode : public LazyFunction { void execute_impl(lf::Params ¶ms, const lf::Context &context) const override { const auto &local_user_data = *static_cast(context.local_user_data); - if (local_user_data.tree_logger == nullptr) { + geo_eval_log::GeoTreeLogger *tree_logger = local_user_data.try_get_tree_logger(); + if (tree_logger == nullptr) { return; } @@ -828,7 +828,7 @@ class LazyFunctionForViewerNode : public LazyFunction { } } - local_user_data.tree_logger->log_viewer_node(bnode_, std::move(geometry)); + tree_logger->log_viewer_node(bnode_, std::move(geometry)); } }; @@ -3919,7 +3919,8 @@ void GeometryNodesLazyFunctionLogger::log_socket_value( return; } auto &local_user_data = *static_cast(context.local_user_data); - if (local_user_data.tree_logger == nullptr) { + geo_eval_log::GeoTreeLogger *tree_logger = local_user_data.try_get_tree_logger(); + if (tree_logger == nullptr) { return; } @@ -3939,7 +3940,7 @@ void GeometryNodesLazyFunctionLogger::log_socket_value( if (bnode.is_reroute()) { continue; } - local_user_data.tree_logger->log_value(bsocket->owner_node(), *bsocket, value); + tree_logger->log_value(bsocket->owner_node(), *bsocket, value); } } @@ -4000,7 +4001,8 @@ Vector GeometryNodesLazyFunctionSideEffectProvider:: static thread_local const std::string thread_id_str = "Thread: " + std::to_string(thread_id); const auto &local_user_data = *static_cast(context.local_user_data); - if (local_user_data.tree_logger == nullptr) { + geo_eval_log::GeoTreeLogger *tree_logger = local_user_data.try_get_tree_logger(); + if (tree_logger == nullptr) { return; } @@ -4012,7 +4014,7 @@ Vector GeometryNodesLazyFunctionSideEffectProvider:: if (!bsockets.is_empty()) { const bNodeSocket &bsocket = *bsockets[0]; const bNode &bnode = bsocket.owner_node(); - local_user_data.tree_logger->debug_messages.append({bnode.identifier, thread_id_str}); + tree_logger->debug_messages.append({bnode.identifier, thread_id_str}); return true; } } @@ -4040,16 +4042,18 @@ destruct_ptr GeoNodesLFUserData::get_local(LinearAllocator<> return allocator.construct(*this); } -GeoNodesLFLocalUserData::GeoNodesLFLocalUserData(GeoNodesLFUserData &user_data) +void GeoNodesLFLocalUserData::ensure_tree_logger() const { - if (user_data.modifier_data == nullptr) { - this->tree_logger = nullptr; + if (user_data_.modifier_data == nullptr) { + tree_logger_.emplace(nullptr); return; } - if (user_data.modifier_data->eval_log != nullptr) { - this->tree_logger = &user_data.modifier_data->eval_log->get_local_tree_logger( - *user_data.compute_context); + if (user_data_.modifier_data->eval_log != nullptr) { + tree_logger_.emplace( + &user_data_.modifier_data->eval_log->get_local_tree_logger(*user_data_.compute_context)); + return; } + this->tree_logger_.emplace(nullptr); } std::optional find_nested_node_id(const GeoNodesLFUserData &user_data,