[Bf-blender-cvs] [1388e9de8af] master: Geometry Nodes: improve node locking in evaluator
Jacques Lucke
noreply at git.blender.org
Thu Jun 17 10:48:26 CEST 2021
Commit: 1388e9de8afe187f85ad857e864d7463ff8ede55
Author: Jacques Lucke
Date: Thu Jun 17 10:43:32 2021 +0200
Branches: master
https://developer.blender.org/rB1388e9de8afe187f85ad857e864d7463ff8ede55
Geometry Nodes: improve node locking in evaluator
This makes the parts where a node is locked more explicit. Also, now the thread
is isolated when the node is locked. This prevents some kinds of deadlocks
(which haven't happened in practice yet).
===================================================================
M source/blender/modifiers/intern/MOD_nodes_evaluator.cc
===================================================================
diff --git a/source/blender/modifiers/intern/MOD_nodes_evaluator.cc b/source/blender/modifiers/intern/MOD_nodes_evaluator.cc
index 3de7497ae3f..01eebfea333 100644
--- a/source/blender/modifiers/intern/MOD_nodes_evaluator.cc
+++ b/source/blender/modifiers/intern/MOD_nodes_evaluator.cc
@@ -264,13 +264,10 @@ struct NodeWithState {
class GeometryNodesEvaluator;
/**
- * Utility class that locks the state of a node. Having this is a separate class is useful because
- * it allows methods to communicate that they expect the node to be locked.
+ * Utility class that wraps a node whose state is locked. Having this is a separate class is useful
+ * because it allows methods to communicate that they expect the node to be locked.
*/
class LockedNode : NonCopyable, NonMovable {
- private:
- GeometryNodesEvaluator &evaluator_;
-
public:
/**
* This is the node that is currently locked.
@@ -290,13 +287,9 @@ class LockedNode : NonCopyable, NonMovable {
Vector<DOutputSocket> delayed_unused_outputs;
Vector<DNode> delayed_scheduled_nodes;
- LockedNode(GeometryNodesEvaluator &evaluator, const DNode node, NodeState &node_state)
- : evaluator_(evaluator), node(node), node_state(node_state)
+ LockedNode(const DNode node, NodeState &node_state) : node(node), node_state(node_state)
{
- node_state.mutex.lock();
}
-
- ~LockedNode();
};
static const CPPType *get_socket_cpp_type(const DSocket socket)
@@ -570,9 +563,10 @@ class GeometryNodesEvaluator {
for (const DInputSocket &socket : params_.output_sockets) {
const DNode node = socket.node();
NodeState &node_state = this->get_node_state(node);
- LockedNode locked_node{*this, node, node_state};
- /* Setting an input as required will schedule any linked node. */
- this->set_input_required(locked_node, socket);
+ this->with_locked_node(node, node_state, [&](LockedNode &locked_node) {
+ /* Setting an input as required will schedule any linked node. */
+ this->set_input_required(locked_node, socket);
+ });
}
}
@@ -634,30 +628,33 @@ class GeometryNodesEvaluator {
bool node_task_preprocessing(const DNode node, NodeState &node_state)
{
- LockedNode locked_node{*this, node, node_state};
- BLI_assert(node_state.schedule_state == NodeScheduleState::Scheduled);
- node_state.schedule_state = NodeScheduleState::Running;
+ bool do_execute_node = false;
+ this->with_locked_node(node, node_state, [&](LockedNode &locked_node) {
+ BLI_assert(node_state.schedule_state == NodeScheduleState::Scheduled);
+ node_state.schedule_state = NodeScheduleState::Running;
- /* Early return if the node has finished already. */
- if (locked_node.node_state.node_has_finished) {
- return false;
- }
- /* Prepare outputs and check if actually any new outputs have to be computed. */
- if (!this->prepare_node_outputs_for_execution(locked_node)) {
- return false;
- }
- /* Initialize nodes that don't support laziness. This is done after at least one output is
- * required and before we check that all required inputs are provided. This reduces the
- * number of "round-trips" through the task pool by one for most nodes. */
- if (!node_state.non_lazy_node_is_initialized && !node_supports_laziness(node)) {
- this->initialize_non_lazy_node(locked_node);
- node_state.non_lazy_node_is_initialized = true;
- }
- /* Prepare inputs and check if all required inputs are provided. */
- if (!this->prepare_node_inputs_for_execution(locked_node)) {
- return false;
- }
- return true;
+ /* Early return if the node has finished already. */
+ if (locked_node.node_state.node_has_finished) {
+ return;
+ }
+ /* Prepare outputs and check if actually any new outputs have to be computed. */
+ if (!this->prepare_node_outputs_for_execution(locked_node)) {
+ return;
+ }
+ /* Initialize nodes that don't support laziness. This is done after at least one output is
+ * required and before we check that all required inputs are provided. This reduces the
+ * number of "round-trips" through the task pool by one for most nodes. */
+ if (!node_state.non_lazy_node_is_initialized && !node_supports_laziness(node)) {
+ this->initialize_non_lazy_node(locked_node);
+ node_state.non_lazy_node_is_initialized = true;
+ }
+ /* Prepare inputs and check if all required inputs are provided. */
+ if (!this->prepare_node_inputs_for_execution(locked_node)) {
+ return;
+ }
+ do_execute_node = true;
+ });
+ return do_execute_node;
}
/* A node is finished when it has computed all outputs that may be used. */
@@ -898,18 +895,18 @@ class GeometryNodesEvaluator {
void node_task_postprocessing(const DNode node, NodeState &node_state)
{
- LockedNode locked_node{*this, node, node_state};
-
- const bool node_has_finished = this->finish_node_if_possible(locked_node);
- const bool reschedule_requested = node_state.schedule_state ==
- NodeScheduleState::RunningAndRescheduled;
- node_state.schedule_state = NodeScheduleState::NotScheduled;
- if (reschedule_requested && !node_has_finished) {
- /* Either the node rescheduled itself or another node tried to schedule it while it ran. */
- this->schedule_node(locked_node);
- }
+ this->with_locked_node(node, node_state, [&](LockedNode &locked_node) {
+ const bool node_has_finished = this->finish_node_if_possible(locked_node);
+ const bool reschedule_requested = node_state.schedule_state ==
+ NodeScheduleState::RunningAndRescheduled;
+ node_state.schedule_state = NodeScheduleState::NotScheduled;
+ if (reschedule_requested && !node_has_finished) {
+ /* Either the node rescheduled itself or another node tried to schedule it while it ran. */
+ this->schedule_node(locked_node);
+ }
- this->assert_expected_outputs_have_been_computed(locked_node);
+ this->assert_expected_outputs_have_been_computed(locked_node);
+ });
}
void assert_expected_outputs_have_been_computed(LockedNode &locked_node)
@@ -1097,15 +1094,16 @@ class GeometryNodesEvaluator {
NodeState &node_state = this->get_node_state(node);
OutputState &output_state = node_state.outputs[socket->index()];
- LockedNode locked_node{*this, node, node_state};
- if (output_state.output_usage == ValueUsage::Required) {
- /* Output is marked as required already. So the node is scheduled already. */
- return;
- }
- /* The origin node needs to be scheduled so that it provides the requested input
- * eventually. */
- output_state.output_usage = ValueUsage::Required;
- this->schedule_node(locked_node);
+ this->with_locked_node(node, node_state, [&](LockedNode &locked_node) {
+ if (output_state.output_usage == ValueUsage::Required) {
+ /* Output is marked as required already. So the node is scheduled already. */
+ return;
+ }
+ /* The origin node needs to be scheduled so that it provides the requested input
+ * eventually. */
+ output_state.output_usage = ValueUsage::Required;
+ this->schedule_node(locked_node);
+ });
}
void send_output_unused_notification(const DOutputSocket socket)
@@ -1114,14 +1112,15 @@ class GeometryNodesEvaluator {
NodeState &node_state = this->get_node_state(node);
OutputState &output_state = node_state.outputs[socket->index()];
- LockedNode locked_node{*this, node, node_state};
- output_state.potential_users -= 1;
- if (output_state.potential_users == 0) {
- /* The output socket has no users anymore. */
- output_state.output_usage = ValueUsage::Unused;
- /* Schedule the origin node in case it wants to set its inputs as unused as well. */
- this->schedule_node(locked_node);
- }
+ this->with_locked_node(node, node_state, [&](LockedNode &locked_node) {
+ output_state.potential_users -= 1;
+ if (output_state.potential_users == 0) {
+ /* The output socket has no users anymore. */
+ output_state.output_usage = ValueUsage::Unused;
+ /* Schedule the origin node in case it wants to set its inputs as unused as well. */
+ this->schedule_node(locked_node);
+ }
+ });
}
void add_node_to_task_pool(const DNode node)
@@ -1249,28 +1248,27 @@ class GeometryNodesEvaluator {
NodeState &node_state = this->get_node_state(node);
InputState &input_state = node_state.inputs[socket->index()];
- /* Lock the node because we want to change its state. */
- LockedNode locked_node{*this, node, node_state};
-
- if (socket->is_multi_input_socket()) {
- /* Add a new value to the multi-input. */
- MultiInputValue &multi_value = *input_state.value.multi;
- multi_value.items.append({origin, value.get()});
- }
- else {
- /* Assign the value to the input. */
- SingleInputValue &single_value = *input_state.value.single;
- BLI_assert(single_value.value == nullptr);
- single_value.value = value.get();
- }
+ this->with_locked_node(node, node_state, [&](LockedNode &locked_node) {
+ if (socket->is_multi_input_socket()) {
+ /* Add a new value to the multi-input. */
+ MultiInputValue &multi_value = *input_state.value.multi;
+ multi_value.items.append({origin, value.get()});
+ }
+ else {
+ /* Assign the value to the input. */
+ SingleInputValue &single_value = *input_state.value.single;
+ BLI_assert(single_value.value == nullptr);
+ single_value.value = value.get();
+ }
- if (input_state.usage == ValueUsage::Required) {
- node_state.missing_required_inputs--;
- if (node_state.missing_required_inputs == 0) {
- /* Schedule node if all the required inputs have been provided. */
- this->schedule_node(locked_node);
+ if (input_state.usage == ValueUsage::Required) {
+ node_state.missing_required_inputs--;
+ if (node_state.missing_required_inputs == 0) {
+ /* Schedule node if all the r
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list