Recent spinlock commit made scheduling unsafe for threading
authorSergey Sharybin <sergey.vfx@gmail.com>
Wed, 14 Aug 2013 09:24:15 +0000 (09:24 +0000)
committerSergey Sharybin <sergey.vfx@gmail.com>
Wed, 14 Aug 2013 09:24:15 +0000 (09:24 +0000)
Namely, it caused nodes be adding to the pool multiple times.

Returned spin back, but use it only in cases node valency is
zero. So now valency is decreasing without any locks, then
if it's zero spin lock happens, node color (which indicates
whether node is scheduled or not) happens. Actual new task
creation happens outside of locks.

This might sound a bit complicated, but it's straightforward
code which is free from any thread synchronization latency.

source/blender/blenkernel/BKE_depsgraph.h
source/blender/blenkernel/intern/blender.c
source/blender/blenkernel/intern/depsgraph.c
source/blender/windowmanager/intern/wm_playanim.c
source/creator/creator.c
source/gameengine/GamePlayer/ghost/GPG_ghost.cpp

index f58790c4a1f826a7fa45912891393838ceab842a..e8ebf88d98cd43041ca67c154f44335f062291d6 100644 (file)
@@ -118,6 +118,10 @@ void DAG_editors_update_cb(void (*id_func)(struct Main *bmain, struct ID *id),
 
 /* ** Threaded update ** */
 
+/* Global initialization/deinitialization */
+void DAG_threaded_init(void);
+void DAG_threaded_exit(void);
+
 /* Initialize the DAG for threaded update. */
 void DAG_threaded_update_begin(struct Scene *scene);
 
index 26f481e5341efb853e948622d1b066c93455cddb..71837cc41383615ff3f9b5bcd9d76aa05e3ea02e 100644 (file)
@@ -120,6 +120,7 @@ void free_blender(void)
        
        IMB_exit();
        BKE_images_exit();
+       DAG_threaded_exit();
 
        BKE_brush_system_exit();
 
index 507d1cf8d0b99f07f604160e9338087387249045..5995e2a9539674cab2ecc4a60469c5f4ecaf272f 100644 (file)
@@ -42,6 +42,7 @@
 #include "BLI_utildefines.h"
 #include "BLI_listbase.h"
 #include "BLI_ghash.h"
+#include "BLI_threads.h"
 
 #include "DNA_anim_types.h"
 #include "DNA_camera_types.h"
@@ -2658,6 +2659,18 @@ void DAG_pose_sort(Object *ob)
 
 /* ************************  DAG FOR THREADED UPDATE  ********************* */
 
+static SpinLock threaded_update_lock;
+
+void DAG_threaded_init(void)
+{
+       BLI_spin_init(&threaded_update_lock);
+}
+
+void DAG_threaded_exit(void)
+{
+       BLI_spin_end(&threaded_update_lock);
+}
+
 /* Initialize the DAG for threaded update.
  *
  * Sets up all the data needed for faster check whether DAG node is
@@ -2670,6 +2683,7 @@ void DAG_threaded_update_begin(Scene *scene)
        /* We reset valency to zero first... */
        for (node = scene->theDag->DagNode.first; node; node = node->next) {
                node->valency = 0;
+               node->color = DAG_WHITE;
        }
 
        /* ... and then iterate over all the nodes and
@@ -2698,7 +2712,16 @@ void DAG_threaded_update_foreach_ready_node(Scene *scene,
 
        for (node = scene->theDag->DagNode.first; node; node = node->next) {
                if (node->valency == 0) {
-                       func(node, user_data);
+                       bool need_schedule;
+
+                       BLI_spin_lock(&threaded_update_lock);
+                       need_schedule = node->color == DAG_WHITE;
+                       node->color = DAG_BLACK;
+                       BLI_spin_unlock(&threaded_update_lock);
+
+                       if (need_schedule) {
+                               func(node, user_data);
+                       }
                }
        }
 }
@@ -2738,11 +2761,21 @@ void DAG_threaded_update_handle_node_updated(void *node_v,
        DagAdjList *itA;
 
        for (itA = node->child; itA; itA = itA->next) {
-               if (itA->node != node) {
-                       atomic_sub_uint32(&itA->node->valency, 1);
+               DagNode *child_node = itA->node;
+               if (child_node != node) {
+                       atomic_sub_uint32(&child_node->valency, 1);
 
-                       if (itA->node->valency == 0) {
-                               func(itA->node, user_data);
+                       if (child_node->valency == 0) {
+                               bool need_schedule;
+
+                               BLI_spin_lock(&threaded_update_lock);
+                               need_schedule = child_node->color == DAG_WHITE;
+                               child_node->color = DAG_BLACK;
+                               BLI_spin_unlock(&threaded_update_lock);
+
+                               if (need_schedule) {
+                                       func(child_node, user_data);
+                               }
                        }
                }
        }
index e8ab1fef8b700a850f0910f3d2b6e09aa5d4cd43..31b44915ad4572f23d4661f5a8ff26630d3622df 100644 (file)
@@ -60,6 +60,7 @@
 #include "IMB_imbuf.h"
 
 #include "BKE_blender.h"
+#include "BKE_depsgraph.h"
 #include "BKE_global.h"
 #include "BKE_image.h"
 
@@ -1198,6 +1199,7 @@ static char *wm_main_playanim_intern(int argc, const char **argv)
        
        IMB_exit();
        BKE_images_exit();
+       DAG_threaded_exit();
 
        totblock = MEM_get_memory_blocks_in_use();
        if (totblock != 0) {
index 27fd9427da781270a3409d722d9d664bd87be04c..ff8b91f9c1e2f3d50651358e552820403e93b378 100644 (file)
@@ -1519,6 +1519,7 @@ int main(int argc, const char **argv)
        IMB_init();
        BKE_images_init();
        BKE_modifier_init();
+       DAG_threaded_init();
 
        BKE_brush_system_init();
 
index aa7d41cc410f575e1a044be1a0d2efbc5240e223..905c98eae562381b9ff2f43e1aad23ee7cb50951 100644 (file)
@@ -458,6 +458,7 @@ int main(int argc, char** argv)
        IMB_init();
        BKE_images_init();
        BKE_modifier_init();
+       DAG_threaded_init();
 
 #ifdef WITH_FFMPEG
        IMB_ffmpeg_init();
@@ -1081,6 +1082,7 @@ int main(int argc, char** argv)
 
        IMB_exit();
        BKE_images_exit();
+       DAG_threaded_exit();
 
        SYS_DeleteSystem(syshandle);