Task scheduler: Remove per-pool threads limit
authorSergey Sharybin <sergey.vfx@gmail.com>
Fri, 3 Mar 2017 09:48:13 +0000 (10:48 +0100)
committerSergey Sharybin <sergey.vfx@gmail.com>
Tue, 7 Mar 2017 16:32:01 +0000 (17:32 +0100)
This feature was adding extra complexity to task scheduling
which required yet extra variables to be worried about to be
modified in atomic manner, which resulted in following issues:

- More complex code to maintain, which increases risks of
  something going wrong when we modify the code.

- Extra barriers and/or locks during task scheduling, which
  causes extra threading overhead.

- Unable to use some other implementation (such as TBB) even for
  the comparison tests.

Notes about other changes.

There are two places where we really had to use that limit.

One of them is the single threaded dependency graph. This will
now construct a single-threaded scheduler at evaluation time.
This shouldn't be a problem because it only happens when using
debugging command line arguments and the code simply don't
run in regular Blender operation.

The code seems a bit duplicated here across old and new
depsgraph, but think it's OK since the old depsgraph is already
gone in 2.8 branch and i don't see where else we might want
to use such a single-threaded scheduler.

When/if we'll want to do so, we can move it to a centralized
single-threaded scheduler in threads.c.

OpenGL render was a bit more tricky to port, but basically we
are using conditional variables to wait background thread to
do all the job.

source/blender/blenkernel/intern/scene.c
source/blender/blenlib/BLI_task.h
source/blender/blenlib/intern/task.c
source/blender/depsgraph/intern/eval/deg_eval.cc
source/blender/editors/render/render_opengl.c

index 50f46748570931563d2eaf2ee4d8636003553d35..906fa0134a02bca8c6a88ed2836635e416328ab2 100644 (file)
@@ -1629,10 +1629,11 @@ static bool scene_need_update_objects(Main *bmain)
 
 static void scene_update_objects(EvaluationContext *eval_ctx, Main *bmain, Scene *scene, Scene *scene_parent)
 {
-       TaskScheduler *task_scheduler = BLI_task_scheduler_get();
+       TaskScheduler *task_scheduler;
        TaskPool *task_pool;
        ThreadedObjectUpdateState state;
        bool need_singlethread_pass;
+       bool need_free_scheduler;
 
        /* Early check for whether we need to invoke all the task-based
         * things (spawn new ppol, traverse dependency graph and so on).
@@ -1649,6 +1650,15 @@ static void scene_update_objects(EvaluationContext *eval_ctx, Main *bmain, Scene
        state.scene = scene;
        state.scene_parent = scene_parent;
 
+       if (G.debug & G_DEBUG_DEPSGRAPH_NO_THREADS) {
+               task_scheduler = BLI_task_scheduler_create(1);
+               need_free_scheduler = true;
+       }
+       else {
+               task_scheduler = BLI_task_scheduler_get();
+               need_free_scheduler = false;
+       }
+
        /* Those are only needed when blender is run with --debug argument. */
        if (G.debug & G_DEBUG_DEPSGRAPH) {
                const int tot_thread = BLI_task_scheduler_num_threads(task_scheduler);
@@ -1663,9 +1673,6 @@ static void scene_update_objects(EvaluationContext *eval_ctx, Main *bmain, Scene
 #endif
 
        task_pool = BLI_task_pool_create(task_scheduler, &state);
-       if (G.debug & G_DEBUG_DEPSGRAPH_NO_THREADS) {
-               BLI_pool_set_num_threads(task_pool, 1);
-       }
 
        DAG_threaded_update_begin(scene, scene_update_object_add_task, task_pool);
        BLI_task_pool_work_and_wait(task_pool);
@@ -1698,6 +1705,10 @@ static void scene_update_objects(EvaluationContext *eval_ctx, Main *bmain, Scene
        if (need_singlethread_pass) {
                scene_update_all_bases(eval_ctx, scene, scene_parent);
        }
+
+       if (need_free_scheduler) {
+               BLI_task_scheduler_free(task_scheduler);
+       }
 }
 
 static void scene_update_tagged_recursive(EvaluationContext *eval_ctx, Main *bmain, Scene *scene, Scene *scene_parent)
index d27bf4dad20507810e960cb3a99d0d288422afb5..bc695d174fa974fa58debeb837f88cdfd5d212f1 100644 (file)
@@ -96,9 +96,6 @@ void BLI_task_pool_work_and_wait(TaskPool *pool);
 /* cancel all tasks, keep worker threads running */
 void BLI_task_pool_cancel(TaskPool *pool);
 
-/* set number of threads allowed to be used by this pool */
-void BLI_pool_set_num_threads(TaskPool *pool, int num_threads);
-
 /* for worker threads, test if canceled */
 bool BLI_task_pool_canceled(TaskPool *pool);
 
index 5d16fd9229cf3e05c203618e9c02a773ac195438..2bf1ee2650731dbfbdd48e6b39de93619b055306 100644 (file)
@@ -106,8 +106,6 @@ struct TaskPool {
        TaskScheduler *scheduler;
 
        volatile size_t num;
-       size_t num_threads;
-       size_t currently_running_tasks;
        ThreadMutex num_mutex;
        ThreadCondition num_cond;
 
@@ -236,7 +234,6 @@ static void task_pool_num_decrease(TaskPool *pool, size_t done)
        BLI_assert(pool->num >= done);
 
        pool->num -= done;
-       atomic_sub_and_fetch_z(&pool->currently_running_tasks, done);
 
        if (pool->num == 0)
                BLI_condition_notify_all(&pool->num_cond);
@@ -290,17 +287,10 @@ static bool task_scheduler_thread_wait_pop(TaskScheduler *scheduler, Task **task
                                continue;
                        }
 
-                       if (atomic_add_and_fetch_z(&pool->currently_running_tasks, 1) <= pool->num_threads ||
-                           pool->num_threads == 0)
-                       {
-                               *task = current_task;
-                               found_task = true;
-                               BLI_remlink(&scheduler->queue, *task);
-                               break;
-                       }
-                       else {
-                               atomic_sub_and_fetch_z(&pool->currently_running_tasks, 1);
-                       }
+                       *task = current_task;
+                       found_task = true;
+                       BLI_remlink(&scheduler->queue, *task);
+                       break;
                }
                if (!found_task)
                        BLI_condition_wait(&scheduler->queue_cond, &scheduler->queue_mutex);
@@ -502,8 +492,6 @@ static TaskPool *task_pool_create_ex(TaskScheduler *scheduler, void *userdata, c
 
        pool->scheduler = scheduler;
        pool->num = 0;
-       pool->num_threads = 0;
-       pool->currently_running_tasks = 0;
        pool->do_cancel = false;
        pool->run_in_background = is_background;
 
@@ -648,16 +636,12 @@ void BLI_task_pool_work_and_wait(TaskPool *pool)
                /* find task from this pool. if we get a task from another pool,
                 * we can get into deadlock */
 
-               if (pool->num_threads == 0 ||
-                   pool->currently_running_tasks < pool->num_threads)
-               {
-                       for (task = scheduler->queue.first; task; task = task->next) {
-                               if (task->pool == pool) {
-                                       work_task = task;
-                                       found_task = true;
-                                       BLI_remlink(&scheduler->queue, task);
-                                       break;
-                               }
+               for (task = scheduler->queue.first; task; task = task->next) {
+                       if (task->pool == pool) {
+                               work_task = task;
+                               found_task = true;
+                               BLI_remlink(&scheduler->queue, task);
+                               break;
                        }
                }
 
@@ -666,7 +650,6 @@ void BLI_task_pool_work_and_wait(TaskPool *pool)
                /* if found task, do it, otherwise wait until other tasks are done */
                if (found_task) {
                        /* run task */
-                       atomic_add_and_fetch_z(&pool->currently_running_tasks, 1);
                        work_task->run(pool, work_task->taskdata, 0);
 
                        /* delete task */
@@ -687,12 +670,6 @@ void BLI_task_pool_work_and_wait(TaskPool *pool)
        BLI_mutex_unlock(&pool->num_mutex);
 }
 
-void BLI_pool_set_num_threads(TaskPool *pool, int num_threads)
-{
-       /* NOTE: Don't try to modify threads while tasks are running! */
-       pool->num_threads = num_threads;
-}
-
 void BLI_task_pool_cancel(TaskPool *pool)
 {
        pool->do_cancel = true;
index 3a042535d26751d0ec09f91a4ad858ab797d2262..a5f268aac8c1e309c85ebfd21c9f4592654cbba3 100644 (file)
@@ -378,13 +378,20 @@ void deg_evaluate_on_refresh(EvaluationContext *eval_ctx,
        state.graph = graph;
        state.layers = layers;
 
-       TaskScheduler *task_scheduler = BLI_task_scheduler_get();
-       TaskPool *task_pool = BLI_task_pool_create(task_scheduler, &state);
+       TaskScheduler *task_scheduler;
+       bool need_free_scheduler;
 
        if (G.debug & G_DEBUG_DEPSGRAPH_NO_THREADS) {
-               BLI_pool_set_num_threads(task_pool, 1);
+               task_scheduler = BLI_task_scheduler_create(1);
+               need_free_scheduler = true;
+       }
+       else {
+               task_scheduler = BLI_task_scheduler_get();
+               need_free_scheduler = false;
        }
 
+       TaskPool *task_pool = BLI_task_pool_create(task_scheduler, &state);
+
        calculate_pending_parents(graph, layers);
 
        /* Clear tags. */
@@ -410,6 +417,10 @@ void deg_evaluate_on_refresh(EvaluationContext *eval_ctx,
 
        /* Clear any uncleared tags - just in case. */
        deg_graph_clear_tags(graph);
+
+       if (need_free_scheduler) {
+               BLI_task_scheduler_free(task_scheduler);
+       }
 }
 
 }  // namespace DEG
index 90dac7c34ce96b8eb0304fb7d1c2f52c6feba39a..1d0f433ba38b32510baf6967feaa531d050831a3 100644 (file)
@@ -715,7 +715,6 @@ static bool screen_opengl_render_init(bContext *C, wmOperator *op)
                        oglrender->task_scheduler = task_scheduler;
                        oglrender->task_pool = BLI_task_pool_create_background(task_scheduler,
                                                                               oglrender);
-                       BLI_pool_set_num_threads(oglrender->task_pool, 1);
                }
                else {
                        oglrender->task_scheduler = NULL;
@@ -747,6 +746,23 @@ static void screen_opengl_render_end(bContext *C, OGLRender *oglrender)
        int i;
 
        if (oglrender->is_animation) {
+               /* Trickery part for movie output:
+                *
+                * We MUST write frames in an exact order, so we only let background
+                * thread to work on that, and main thread is simply waits for that
+                * thread to do all the dirty work.
+                *
+                * After this loop is done work_and_wait() will have nothing to do,
+                * so we don't run into wrong order of frames written to the stream.
+                */
+               if (BKE_imtype_is_movie(scene->r.im_format.imtype)) {
+                       BLI_mutex_lock(&oglrender->task_mutex);
+                       while (oglrender->num_scheduled_frames > 0) {
+                               BLI_condition_wait(&oglrender->task_condition,
+                                                  &oglrender->task_mutex);
+                       }
+                       BLI_mutex_unlock(&oglrender->task_mutex);
+               }
                BLI_task_pool_work_and_wait(oglrender->task_pool);
                BLI_task_pool_free(oglrender->task_pool);
                /* Depending on various things we might or might not use global scheduler. */