[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