[Bf-blender-cvs] [d9d1a09] depsgraph_refactor: Scheduler fixes: Initialize mutexes correctly and make sure decrementing the pending_links count in nodes is an atomic operation.

Lukas Tönne noreply at git.blender.org
Wed Jun 4 13:01:18 CEST 2014


Commit: d9d1a09233c21a804a54487eef4669265d7457dc
Author: Lukas Tönne
Date:   Wed Jun 4 12:49:43 2014 +0200
https://developer.blender.org/rBd9d1a09233c21a804a54487eef4669265d7457dc

Scheduler fixes: Initialize mutexes correctly and make sure decrementing
the pending_links count in nodes is an atomic operation.

Also moved the child scheduling function into depsgraph eval, where it
is protected by a spin lock to avoid race conditions on
num_pending_links == 0.

NOTE: Apparently there is still a Heisenbug regarding this condition,
but reproducing only works sporadically, have to investigate ...

===================================================================

M	source/blender/depsgraph/CMakeLists.txt
M	source/blender/depsgraph/intern/depsgraph_eval.cpp
M	source/blender/depsgraph/intern/depsgraph_eval.h
M	source/blender/depsgraph/intern/depsnode_operation.h
M	source/blender/depsgraph/util/depsgraph_util_task.cpp
M	source/blender/depsgraph/util/depsgraph_util_task.h

===================================================================

diff --git a/source/blender/depsgraph/CMakeLists.txt b/source/blender/depsgraph/CMakeLists.txt
index 27846f4..faacc7a 100644
--- a/source/blender/depsgraph/CMakeLists.txt
+++ b/source/blender/depsgraph/CMakeLists.txt
@@ -34,6 +34,7 @@ set(INC
 	../makesrna
 	../modifiers
 	../windowmanager
+	../../../intern/atomic
 	../../../intern/guardedalloc
 )
 
diff --git a/source/blender/depsgraph/intern/depsgraph_eval.cpp b/source/blender/depsgraph/intern/depsgraph_eval.cpp
index 9d82486..3f263f5 100644
--- a/source/blender/depsgraph/intern/depsgraph_eval.cpp
+++ b/source/blender/depsgraph/intern/depsgraph_eval.cpp
@@ -57,6 +57,8 @@ extern "C" {
 #include "RNA_types.h"
 } /* extern "C" */
 
+#include "atomic_ops.h"
+
 #include "depsgraph.h"
 #include "depsnode.h"
 #include "depsnode_component.h"
@@ -88,10 +90,13 @@ void DEG_set_eval_mode(eDEG_EvalMode mode)
 /* *************************************************** */
 /* Multi-Threaded Evaluation Internals */
 
+static SpinLock threaded_update_lock;
+
 /* Initialise threading lock - called during application startup */
 void DEG_threaded_init(void)
 {
 	DepsgraphTaskScheduler::init();
+	BLI_spin_init(&threaded_update_lock);
 }
 
 /* Free threading lock - called during application shutdown */
@@ -100,6 +105,7 @@ void DEG_threaded_exit(void)
 	DepsgraphDebug::stats_free();
 	
 	DepsgraphTaskScheduler::exit();
+	BLI_spin_end(&threaded_update_lock);
 }
 
 
@@ -112,6 +118,7 @@ static void calculate_pending_parents(Depsgraph *graph)
 		OperationDepsNode *node = *it_op;
 		
 		node->num_links_pending = 0;
+		node->scheduled = false;
 		
 		/* count number of inputs that need updates */
 		if (node->flag & DEPSOP_FLAG_NEEDS_UPDATE) {
@@ -147,18 +154,40 @@ static void calculate_eval_priority(OperationDepsNode *node)
 		node->eval_priority = 0.0f;
 }
 
-static bool is_node_ready(OperationDepsNode *node)
-{
-	return (node->flag & DEPSOP_FLAG_NEEDS_UPDATE) && node->num_links_pending == 0;
-}
 
 static void schedule_graph(DepsgraphTaskPool &pool, Depsgraph *graph, eEvaluationContextType context_type)
 {
+	BLI_spin_lock(&threaded_update_lock);
 	for (Depsgraph::OperationNodes::const_iterator it = graph->operations.begin(); it != graph->operations.end(); ++it) {
 		OperationDepsNode *node = *it;
 		
-		if (is_node_ready(node)) {
+		if ((node->flag & DEPSOP_FLAG_NEEDS_UPDATE) && node->num_links_pending == 0) {
 			pool.push(graph, node, context_type);
+			node->scheduled = true;
+		}
+	}
+	BLI_spin_unlock(&threaded_update_lock);
+}
+
+void deg_schedule_children(DepsgraphTaskPool &pool, Depsgraph *graph, eEvaluationContextType context_type, OperationDepsNode *node)
+{
+	for (OperationDepsNode::Relations::const_iterator it = node->outlinks.begin(); it != node->outlinks.end(); ++it) {
+		DepsRelation *rel = *it;
+		OperationDepsNode *child = rel->to;
+		
+		if (child->flag & DEPSOP_FLAG_NEEDS_UPDATE) {
+			BLI_assert(child->num_links_pending > 0);
+			atomic_sub_uint32(&child->num_links_pending, 1);
+			
+			if (child->num_links_pending == 0) {
+				BLI_spin_lock(&threaded_update_lock);
+				bool need_schedule = !child->scheduled;
+				child->scheduled = true;
+				BLI_spin_unlock(&threaded_update_lock);
+				
+				if (need_schedule)
+					pool.push(graph, child, context_type);
+			}
 		}
 	}
 }
diff --git a/source/blender/depsgraph/intern/depsgraph_eval.h b/source/blender/depsgraph/intern/depsgraph_eval.h
index 0eca9c1..912fe4f 100644
--- a/source/blender/depsgraph/intern/depsgraph_eval.h
+++ b/source/blender/depsgraph/intern/depsgraph_eval.h
@@ -128,4 +128,10 @@ typedef struct DEG_PoseContext {
 
 /* ****************************************** */
 
+struct DepsgraphTaskPool;
+struct Depsgraph;
+struct OperationDepsNode;
+
+void deg_schedule_children(DepsgraphTaskPool &pool, Depsgraph *graph, eEvaluationContextType context_type, OperationDepsNode *node);
+
 #endif // __DEPSGRAPH_EVAL_TYPES_H__
diff --git a/source/blender/depsgraph/intern/depsnode_operation.h b/source/blender/depsgraph/intern/depsnode_operation.h
index 74c8625..dfbf54d 100644
--- a/source/blender/depsgraph/intern/depsnode_operation.h
+++ b/source/blender/depsgraph/intern/depsnode_operation.h
@@ -78,6 +78,7 @@ struct OperationDepsNode : public DepsNode {
 	
 	uint32_t num_links_pending; /* how many inlinks are we still waiting on before we can be evaluated... */
 	float eval_priority;
+	bool scheduled;
 	
 	short optype;                 /* (eDepsOperation_Type) stage of evaluation */
 	short flag;                   /* (eDepsOperation_Flag) extra settings affecting evaluation */
diff --git a/source/blender/depsgraph/util/depsgraph_util_task.cpp b/source/blender/depsgraph/util/depsgraph_util_task.cpp
index 58a1897..ca82881 100644
--- a/source/blender/depsgraph/util/depsgraph_util_task.cpp
+++ b/source/blender/depsgraph/util/depsgraph_util_task.cpp
@@ -33,6 +33,7 @@ extern "C" {
 }
 
 #include "depsgraph_debug.h"
+#include "depsgraph_eval.h"
 #include "depsnode_component.h"
 
 #include "depsgraph_util_task.h"
@@ -101,21 +102,9 @@ void DepsgraphTask::run()
 	DepsgraphDebug::task_completed(*this, end_time - start_time);
 }
 
-void DepsgraphTask::schedule_children(DepsgraphTaskPool *pool)
+void DepsgraphTask::finish(DepsgraphTaskPool &pool)
 {
-	for (OperationDepsNode::Relations::const_iterator it = node->outlinks.begin(); it != node->outlinks.end(); ++it) {
-		DepsRelation *rel = *it;
-		OperationDepsNode *child = rel->to;
-		
-		if (child->flag & DEPSOP_FLAG_NEEDS_UPDATE) {
-			BLI_assert(child->num_links_pending > 0);
-			--child->num_links_pending;
-			
-			if (child->num_links_pending == 0) {
-				pool->push(graph, child, context_type);
-			}
-		}
-	}
+	deg_schedule_children(pool, graph, context_type, node);
 }
 
 
@@ -125,11 +114,15 @@ DepsgraphTaskPool::DepsgraphTaskPool()
 {
 	num = 0;
 	do_cancel = false;
+	
+	BLI_mutex_init(&num_mutex);
 }
 
 DepsgraphTaskPool::~DepsgraphTaskPool()
 {
 	stop();
+	
+	BLI_mutex_end(&num_mutex);
 }
 
 void DepsgraphTaskPool::push(Depsgraph *graph, OperationDepsNode *node, eEvaluationContextType context_type)
@@ -264,6 +257,8 @@ ThreadCondition DepsgraphTaskScheduler::queue_cond;
 
 void DepsgraphTaskScheduler::init(int num_threads)
 {
+	BLI_mutex_init(&queue_mutex);
+	
 	do_exit = false;
 	
 	if(num_threads == 0) {
@@ -294,6 +289,8 @@ void DepsgraphTaskScheduler::exit()
 	}
 	threads.clear();
 	
+	BLI_mutex_end(&queue_mutex);
+	
 	deg_eval_sim_free();
 }
 
@@ -333,7 +330,7 @@ void DepsgraphTaskScheduler::thread_run(int thread_id)
 		entry.pool->num_decrease(1);
 		
 		if (!entry.pool->canceled())
-			entry.task.schedule_children(entry.pool);
+			entry.task.finish(*entry.pool);
 	}
 }
 
diff --git a/source/blender/depsgraph/util/depsgraph_util_task.h b/source/blender/depsgraph/util/depsgraph_util_task.h
index 8156587..5051f46 100644
--- a/source/blender/depsgraph/util/depsgraph_util_task.h
+++ b/source/blender/depsgraph/util/depsgraph_util_task.h
@@ -55,8 +55,7 @@ public:
 	DepsgraphTask(Depsgraph *graph_, OperationDepsNode *node_, eEvaluationContextType context_type_);
 	
 	void run();
-	
-	void schedule_children(DepsgraphTaskPool *pool);
+	void finish(DepsgraphTaskPool &pool);
 };
 
 /* Task Pool




More information about the Bf-blender-cvs mailing list