From 5a449439efddc53883063b59c90657a2eea04ab6 Mon Sep 17 00:00:00 2001 From: Jacques Lucke Date: Thu, 15 May 2025 21:18:23 +0200 Subject: [PATCH] Refactor: BLI: simplify compute context hash generation and make it lazy Currently, there was a lot of boilerplate to compute the compute context hash. Now, the complexity is abstracted away to make it a simple function call. Furthermore, this makes the compute context hash generation lazy. The goal here is to make it very cheap to construct the compute context hash in the first, while making it a little bit more expensive (still quite cheap overall) to access the hash when any data has to be logged. This trade-off makes sense when we want to pass the compute context to more lower-level places in order to be able to create better error messages with more contextual information. For example, we'd want to create error messages during multi-function evaluation which happens during field evaluation within a node. Pull Request: https://projects.blender.org/blender/blender/pulls/138912 --- .../blenkernel/BKE_compute_contexts.hh | 28 ++-- .../blenkernel/intern/compute_contexts.cc | 120 +++++++----------- source/blender/blenlib/BLI_compute_context.hh | 92 ++++++++++---- .../blender/blenlib/intern/compute_context.cc | 16 +-- .../intern/geometry_nodes_lazy_function.cc | 9 +- 5 files changed, 135 insertions(+), 130 deletions(-) diff --git a/source/blender/blenkernel/BKE_compute_contexts.hh b/source/blender/blenkernel/BKE_compute_contexts.hh index d1aa651acdb..3d335e69bda 100644 --- a/source/blender/blenkernel/BKE_compute_contexts.hh +++ b/source/blender/blenkernel/BKE_compute_contexts.hh @@ -34,8 +34,6 @@ namespace blender::bke { class ModifierComputeContext : public ComputeContext { private: - static constexpr const char *s_static_type = "MODIFIER"; - /** #ModifierData.persistent_uid. */ int modifier_uid_; /** The modifier data that this context is for. This may be null. */ @@ -56,13 +54,12 @@ class ModifierComputeContext : public ComputeContext { } private: + ComputeContextHash compute_hash() const override; void print_current_in_line(std::ostream &stream) const override; }; class GroupNodeComputeContext : public ComputeContext { private: - static constexpr const char *s_static_type = "NODE_GROUP"; - int32_t node_id_; /** * The caller node tree and group node are not always necessary or even available, but storing @@ -72,13 +69,10 @@ class GroupNodeComputeContext : public ComputeContext { const bNode *caller_group_node_ = nullptr; public: - GroupNodeComputeContext(const ComputeContext *parent, - int32_t node_id, - const std::optional &cached_hash = {}); + GroupNodeComputeContext(const ComputeContext *parent, int32_t node_id); GroupNodeComputeContext(const ComputeContext *parent, const bNode &caller_group_node, - const bNodeTree &caller_tree, - const std::optional &cached_hash = {}); + const bNodeTree &caller_tree); int32_t node_id() const { @@ -96,13 +90,12 @@ class GroupNodeComputeContext : public ComputeContext { } private: + ComputeContextHash compute_hash() const override; void print_current_in_line(std::ostream &stream) const override; }; class SimulationZoneComputeContext : public ComputeContext { private: - static constexpr const char *s_static_type = "SIMULATION_ZONE"; - int32_t output_node_id_; public: @@ -115,13 +108,12 @@ class SimulationZoneComputeContext : public ComputeContext { } private: + ComputeContextHash compute_hash() const override; void print_current_in_line(std::ostream &stream) const override; }; class RepeatZoneComputeContext : public ComputeContext { private: - static constexpr const char *s_static_type = "REPEAT_ZONE"; - int32_t output_node_id_; int iteration_; @@ -140,13 +132,12 @@ class RepeatZoneComputeContext : public ComputeContext { } private: + ComputeContextHash compute_hash() const override; void print_current_in_line(std::ostream &stream) const override; }; class ForeachGeometryElementZoneComputeContext : public ComputeContext { private: - static constexpr const char *s_static_type = "FOREACH_GEOMETRY_ELEMENT_ZONE"; - int32_t output_node_id_; int index_; @@ -169,13 +160,12 @@ class ForeachGeometryElementZoneComputeContext : public ComputeContext { } private: + ComputeContextHash compute_hash() const override; void print_current_in_line(std::ostream &stream) const override; }; class EvaluateClosureComputeContext : public ComputeContext { private: - static constexpr const char *s_static_type = "CLOSURE"; - int32_t node_id_; /** @@ -207,13 +197,12 @@ class EvaluateClosureComputeContext : public ComputeContext { } private: + ComputeContextHash compute_hash() const override; void print_current_in_line(std::ostream &stream) const override; }; class OperatorComputeContext : public ComputeContext { private: - static constexpr const char *s_static_type = "OPERATOR"; - /** The tree that is executed. May be null. */ const bNodeTree *tree_ = nullptr; @@ -228,6 +217,7 @@ class OperatorComputeContext : public ComputeContext { } private: + ComputeContextHash compute_hash() const override; void print_current_in_line(std::ostream &stream) const override; }; diff --git a/source/blender/blenkernel/intern/compute_contexts.cc b/source/blender/blenkernel/intern/compute_contexts.cc index 57b6407efac..9388de75c8c 100644 --- a/source/blender/blenkernel/intern/compute_contexts.cc +++ b/source/blender/blenkernel/intern/compute_contexts.cc @@ -21,10 +21,13 @@ ModifierComputeContext::ModifierComputeContext(const ComputeContext *parent, ModifierComputeContext::ModifierComputeContext(const ComputeContext *parent, const int modifier_uid) - : ComputeContext(s_static_type, parent), modifier_uid_(std::move(modifier_uid)) + : ComputeContext(parent), modifier_uid_(std::move(modifier_uid)) { - hash_.mix_in(s_static_type, strlen(s_static_type)); - hash_.mix_in(&modifier_uid_, sizeof(modifier_uid_)); +} + +ComputeContextHash ModifierComputeContext::compute_hash() const +{ + return ComputeContextHash::from(parent_, "MODIFIER", modifier_uid_); } void ModifierComputeContext::print_current_in_line(std::ostream &stream) const @@ -34,39 +37,26 @@ void ModifierComputeContext::print_current_in_line(std::ostream &stream) const } } -GroupNodeComputeContext::GroupNodeComputeContext( - const ComputeContext *parent, - const int32_t node_id, - const std::optional &cached_hash) - : ComputeContext(s_static_type, parent), node_id_(node_id) +GroupNodeComputeContext::GroupNodeComputeContext(const ComputeContext *parent, + const int32_t node_id) + : ComputeContext(parent), node_id_(node_id) { - if (cached_hash.has_value()) { - hash_ = *cached_hash; - } - else { - /* Mix static type and node id into a single buffer so that only a single call to #mix_in is - * necessary. */ - const int type_size = strlen(s_static_type); - const int buffer_size = type_size + 1 + sizeof(int32_t); - DynamicStackBuffer<64, 8> buffer_owner(buffer_size, 8); - char *buffer = static_cast(buffer_owner.buffer()); - memcpy(buffer, s_static_type, type_size + 1); - memcpy(buffer + type_size + 1, &node_id_, sizeof(int32_t)); - hash_.mix_in(buffer, buffer_size); - } } -GroupNodeComputeContext::GroupNodeComputeContext( - const ComputeContext *parent, - const bNode &caller_group_node, - const bNodeTree &caller_tree, - const std::optional &cached_hash) - : GroupNodeComputeContext(parent, caller_group_node.identifier, cached_hash) +GroupNodeComputeContext::GroupNodeComputeContext(const ComputeContext *parent, + const bNode &caller_group_node, + const bNodeTree &caller_tree) + : GroupNodeComputeContext(parent, caller_group_node.identifier) { caller_group_node_ = &caller_group_node; caller_tree_ = &caller_tree; } +ComputeContextHash GroupNodeComputeContext::compute_hash() const +{ + return ComputeContextHash::from(parent_, "NODE_GROUP", node_id_); +} + void GroupNodeComputeContext::print_current_in_line(std::ostream &stream) const { if (caller_group_node_ != nullptr) { @@ -77,17 +67,8 @@ void GroupNodeComputeContext::print_current_in_line(std::ostream &stream) const SimulationZoneComputeContext::SimulationZoneComputeContext(const ComputeContext *parent, const int32_t output_node_id) - : ComputeContext(s_static_type, parent), output_node_id_(output_node_id) + : ComputeContext(parent), output_node_id_(output_node_id) { - /* Mix static type and node id into a single buffer so that only a single call to #mix_in is - * necessary. */ - const int type_size = strlen(s_static_type); - const int buffer_size = type_size + 1 + sizeof(int32_t); - DynamicStackBuffer<64, 8> buffer_owner(buffer_size, 8); - char *buffer = static_cast(buffer_owner.buffer()); - memcpy(buffer, s_static_type, type_size + 1); - memcpy(buffer + type_size + 1, &output_node_id_, sizeof(int32_t)); - hash_.mix_in(buffer, buffer_size); } SimulationZoneComputeContext::SimulationZoneComputeContext(const ComputeContext *parent, @@ -96,6 +77,11 @@ SimulationZoneComputeContext::SimulationZoneComputeContext(const ComputeContext { } +ComputeContextHash SimulationZoneComputeContext::compute_hash() const +{ + return ComputeContextHash::from(parent_, "SIM_ZONE", output_node_id_); +} + void SimulationZoneComputeContext::print_current_in_line(std::ostream &stream) const { stream << "Simulation Zone ID: " << output_node_id_; @@ -104,18 +90,8 @@ void SimulationZoneComputeContext::print_current_in_line(std::ostream &stream) c RepeatZoneComputeContext::RepeatZoneComputeContext(const ComputeContext *parent, const int32_t output_node_id, const int iteration) - : ComputeContext(s_static_type, parent), output_node_id_(output_node_id), iteration_(iteration) + : ComputeContext(parent), output_node_id_(output_node_id), iteration_(iteration) { - /* Mix static type and node id into a single buffer so that only a single call to #mix_in is - * necessary. */ - const int type_size = strlen(s_static_type); - const int buffer_size = type_size + 1 + sizeof(int32_t) + sizeof(int); - DynamicStackBuffer<64, 8> buffer_owner(buffer_size, 8); - char *buffer = static_cast(buffer_owner.buffer()); - memcpy(buffer, s_static_type, type_size + 1); - memcpy(buffer + type_size + 1, &output_node_id_, sizeof(int32_t)); - memcpy(buffer + type_size + 1 + sizeof(int32_t), &iteration_, sizeof(int)); - hash_.mix_in(buffer, buffer_size); } RepeatZoneComputeContext::RepeatZoneComputeContext(const ComputeContext *parent, @@ -125,6 +101,11 @@ RepeatZoneComputeContext::RepeatZoneComputeContext(const ComputeContext *parent, { } +ComputeContextHash RepeatZoneComputeContext::compute_hash() const +{ + return ComputeContextHash::from(parent_, "REPEAT_ZONE", output_node_id_, iteration_); +} + void RepeatZoneComputeContext::print_current_in_line(std::ostream &stream) const { stream << "Repeat Zone ID: " << output_node_id_; @@ -132,18 +113,8 @@ void RepeatZoneComputeContext::print_current_in_line(std::ostream &stream) const ForeachGeometryElementZoneComputeContext::ForeachGeometryElementZoneComputeContext( const ComputeContext *parent, const int32_t output_node_id, const int index) - : ComputeContext(s_static_type, parent), output_node_id_(output_node_id), index_(index) + : ComputeContext(parent), output_node_id_(output_node_id), index_(index) { - /* Mix static type and node id into a single buffer so that only a single call to #mix_in is - * necessary. */ - const int type_size = strlen(s_static_type); - const int buffer_size = type_size + 1 + sizeof(int32_t) + sizeof(int); - DynamicStackBuffer<64, 8> buffer_owner(buffer_size, 8); - char *buffer = static_cast(buffer_owner.buffer()); - memcpy(buffer, s_static_type, type_size + 1); - memcpy(buffer + type_size + 1, &output_node_id_, sizeof(int32_t)); - memcpy(buffer + type_size + 1 + sizeof(int32_t), &index_, sizeof(int)); - hash_.mix_in(buffer, buffer_size); } ForeachGeometryElementZoneComputeContext::ForeachGeometryElementZoneComputeContext( @@ -152,6 +123,11 @@ ForeachGeometryElementZoneComputeContext::ForeachGeometryElementZoneComputeConte { } +ComputeContextHash ForeachGeometryElementZoneComputeContext::compute_hash() const +{ + return ComputeContextHash::from(parent_, "FOREACH_GEOMETRY_ELEMENT", output_node_id_, index_); +} + void ForeachGeometryElementZoneComputeContext::print_current_in_line(std::ostream &stream) const { stream << "Foreach Geometry Element Zone ID: " << output_node_id_; @@ -159,17 +135,8 @@ void ForeachGeometryElementZoneComputeContext::print_current_in_line(std::ostrea EvaluateClosureComputeContext::EvaluateClosureComputeContext(const ComputeContext *parent, const int32_t node_id) - : ComputeContext(s_static_type, parent), node_id_(node_id) + : ComputeContext(parent), node_id_(node_id) { - /* Mix static type and node id into a single buffer so that only a single call to #mix_in is - * necessary. */ - const int type_size = strlen(s_static_type); - const int buffer_size = type_size + 1 + sizeof(int32_t); - DynamicStackBuffer<64, 8> buffer_owner(buffer_size, 8); - char *buffer = static_cast(buffer_owner.buffer()); - memcpy(buffer, s_static_type, type_size + 1); - memcpy(buffer + type_size + 1, &node_id_, sizeof(int32_t)); - hash_.mix_in(buffer, buffer_size); } EvaluateClosureComputeContext::EvaluateClosureComputeContext( @@ -183,6 +150,11 @@ EvaluateClosureComputeContext::EvaluateClosureComputeContext( closure_source_location_ = closure_source_location; } +ComputeContextHash EvaluateClosureComputeContext::compute_hash() const +{ + return ComputeContextHash::from(parent_, "EVAL_CLOSURE", node_id_); +} + void EvaluateClosureComputeContext::print_current_in_line(std::ostream &stream) const { if (evaluate_node_ != nullptr) { @@ -194,9 +166,8 @@ void EvaluateClosureComputeContext::print_current_in_line(std::ostream &stream) OperatorComputeContext::OperatorComputeContext() : OperatorComputeContext(nullptr) {} OperatorComputeContext::OperatorComputeContext(const ComputeContext *parent) - : ComputeContext(s_static_type, parent) + : ComputeContext(parent) { - hash_.mix_in(s_static_type, strlen(s_static_type)); } OperatorComputeContext::OperatorComputeContext(const ComputeContext *parent, const bNodeTree &tree) @@ -205,6 +176,11 @@ OperatorComputeContext::OperatorComputeContext(const ComputeContext *parent, con tree_ = &tree; } +ComputeContextHash OperatorComputeContext::compute_hash() const +{ + return ComputeContextHash::from(parent_, "OPERATOR"); +} + void OperatorComputeContext::print_current_in_line(std::ostream &stream) const { stream << "Operator"; diff --git a/source/blender/blenlib/BLI_compute_context.hh b/source/blender/blenlib/BLI_compute_context.hh index ccfc239f623..037aedf64eb 100644 --- a/source/blender/blenlib/BLI_compute_context.hh +++ b/source/blender/blenlib/BLI_compute_context.hh @@ -34,11 +34,14 @@ * run on different threads. */ +#include "BLI_cache_mutex.hh" #include "BLI_string_ref.hh" #include "BLI_struct_equality_utils.hh" namespace blender { +class ComputeContext; + /** * A hash that uniquely identifies a specific (non-fixed-size) compute context. The hash has to * have enough bits to make collisions practically impossible. @@ -54,9 +57,30 @@ struct ComputeContextHash { BLI_STRUCT_EQUALITY_OPERATORS_2(ComputeContextHash, v1, v2) - void mix_in(const void *data, int64_t len); + /** + * Standard way to create a compute context hash. + * \param parent: The optional parent context. + * \param type_str: A string literal that identifies the context type. This is used to avoid hash + * collisions between different context types. + * \param args: Additional arguments that affect the hash. Note that only the shallow bytes of + * these types are used. So they generally should not contain any padding. + */ + template + static ComputeContextHash from(const ComputeContext *parent, + const char (&type_str)[N], + Args &&...args); friend std::ostream &operator<<(std::ostream &stream, const ComputeContextHash &hash); + + private: + /** + * Compute a context hash by packing all the arguments into a contiguous buffer and hashing + * that. + */ + template static ComputeContextHash from_shallow_bytes(Args &&...args); + + /** Compute a context hash from a contiguous buffer. */ + static ComputeContextHash from_bytes(const void *data, int64_t len); }; /** @@ -66,43 +90,33 @@ struct ComputeContextHash { * This class should be subclassed to implement specific contexts. */ class ComputeContext { - private: + protected: /** - * Only used for debugging currently. - */ - const char *static_type_; - /** - * Pointer to the context that this context is child of. That allows nesting compute contexts. + * Pointer to the context that this context is child of. That allows nesting compute + * contexts. */ const ComputeContext *parent_ = nullptr; - protected: /** * The hash that uniquely identifies this context. It's a combined hash of this context as well - * as all the parent contexts. + * as all the parent contexts. It's computed lazily to keep initial construction of compute + * contexts very cheap. */ - ComputeContextHash hash_; + mutable ComputeContextHash hash_; + + private: + mutable CacheMutex hash_mutex_; public: - ComputeContext(const char *static_type, const ComputeContext *parent) - : static_type_(static_type), parent_(parent) - { - if (parent != nullptr) { - hash_ = parent_->hash_; - } - } + ComputeContext(const ComputeContext *parent) : parent_(parent) {} virtual ~ComputeContext() = default; const ComputeContextHash &hash() const { + hash_mutex_.ensure([&]() { hash_ = this->compute_hash(); }); return hash_; } - const char *static_type() const - { - return static_type_; - } - const ComputeContext *parent() const { return parent_; @@ -119,6 +133,40 @@ class ComputeContext { virtual void print_current_in_line(std::ostream &stream) const = 0; friend std::ostream &operator<<(std::ostream &stream, const ComputeContext &compute_context); + + private: + /** Compute the hash of this context, usually using #ComputeContextHash::from. */ + virtual ComputeContextHash compute_hash() const = 0; }; +template +inline ComputeContextHash ComputeContextHash::from(const ComputeContext *parent, + const char (&type_str)[N], + Args &&...args) +{ + return ComputeContextHash::from_shallow_bytes( + parent ? parent->hash() : ComputeContextHash{0, 0}, type_str, args...); +} + +template +inline ComputeContextHash ComputeContextHash::from_shallow_bytes(Args &&...args) +{ + /* Copy all values into a contiguous buffer. Intentionally don't use std::tuple to avoid any + * potential padding. */ + constexpr int64_t size_sum = (sizeof(args) + ...); + char buffer[size_sum]; + int64_t offset = 0; + ( + [&] { + using Arg = std::remove_reference_t>; + static_assert(std::has_unique_object_representations_v); + const Arg &arg = args; + memcpy(buffer + offset, &arg, sizeof(Arg)); + offset += sizeof(Arg); + }(), + ...); + /* Compute the hash of that buffer. */ + return ComputeContextHash::from_bytes(buffer, offset); +} + } // namespace blender diff --git a/source/blender/blenlib/intern/compute_context.cc b/source/blender/blenlib/intern/compute_context.cc index c992b8c517c..1d171f6ed94 100644 --- a/source/blender/blenlib/intern/compute_context.cc +++ b/source/blender/blenlib/intern/compute_context.cc @@ -14,18 +14,14 @@ namespace blender { -void ComputeContextHash::mix_in(const void *data, int64_t len) +ComputeContextHash ComputeContextHash::from_bytes(const void *data, const int64_t len) { - const int64_t hash_size = sizeof(ComputeContextHash); - const int64_t buffer_len = hash_size + len; - DynamicStackBuffer<> buffer_owner(buffer_len, 8); - char *buffer = static_cast(buffer_owner.buffer()); - memcpy(buffer, this, hash_size); - memcpy(buffer + hash_size, data, len); - - const XXH128_hash_t hash = XXH3_128bits(buffer, buffer_len); - memcpy(this, &hash, sizeof(hash)); + ComputeContextHash final_hash; + const XXH128_hash_t hash = XXH3_128bits(data, len); + /* Cast to void * to avoid warnings. */ + memcpy(static_cast(&final_hash), &hash, sizeof(hash)); static_assert(sizeof(ComputeContextHash) == sizeof(hash)); + return final_hash; } std::ostream &operator<<(std::ostream &stream, const ComputeContextHash &hash) diff --git a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc index 41c79569baf..4532d1e2760 100644 --- a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc +++ b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc @@ -1118,8 +1118,6 @@ class LazyFunctionForGroupNode : public LazyFunction { struct Storage { void *group_storage = nullptr; - /* To avoid computing the hash more than once. */ - std::optional context_hash_cache; }; public: @@ -1174,11 +1172,8 @@ class LazyFunctionForGroupNode : public LazyFunction { Storage *storage = static_cast(context.storage); /* The compute context changes when entering a node group. */ - bke::GroupNodeComputeContext compute_context{user_data->compute_context, - group_node_, - group_node_.owner_tree(), - storage->context_hash_cache}; - storage->context_hash_cache = compute_context.hash(); + bke::GroupNodeComputeContext compute_context{ + user_data->compute_context, group_node_, group_node_.owner_tree()}; GeoNodesUserData group_user_data = *user_data; group_user_data.compute_context = &compute_context;