[Bf-blender-cvs] [2291e60] new-filebrowser-preview-temp: Address Brecht's review, mainly:

[Bastien Montagne]

Commit: 2291e6015d346d10772a3773e2bd1c7f573f8510
Author: Bastien Montagne
Date:   Sun Oct 18 18:28:13 2015 +0200
Branches: new-filebrowser-preview-temp
https://developer.blender.org/rB2291e6015d346d10772a3773e2bd1c7f573f8510

Address Brecht's review, mainly:

* Simplifies handling of 'background fallback thread' in scheduler
* Add note (and debug check) that background pools do not get created recursively (from other task pools).

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

M	source/blender/blenlib/intern/task.c

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

diff --git a/source/blender/blenlib/intern/task.c b/source/blender/blenlib/intern/task.c
index abd4435..b5715e0 100644
--- a/source/blender/blenlib/intern/task.c
+++ b/source/blender/blenlib/intern/task.c
@@ -63,14 +63,15 @@ struct TaskPool {
 	volatile bool do_cancel;
 
 	/* If set, this pool may never be work_and_wait'ed, which means TaskScheduler has to use its special
-	 * background fallback thread in case we are in mono-threaded situation. */
-	bool use_force_background;
+	 * background fallback thread in case we are in single-threaded situation. */
+	bool run_in_background;
 };
 
 struct TaskScheduler {
 	pthread_t *threads;
 	struct TaskThread *task_threads;
 	int num_threads;
+	bool background_thread_only;
 
 	ListBase queue;
 	ThreadMutex queue_mutex;
@@ -125,7 +126,7 @@ static void task_pool_num_increase(TaskPool *pool)
 	BLI_mutex_unlock(&pool->num_mutex);
 }
 
-static bool task_scheduler_thread_wait_pop(TaskScheduler *scheduler, Task **task, const bool forced_background)
+static bool task_scheduler_thread_wait_pop(TaskScheduler *scheduler, Task **task)
 {
 	bool found_task = false;
 	BLI_mutex_lock(&scheduler->queue_mutex);
@@ -146,7 +147,7 @@ static bool task_scheduler_thread_wait_pop(TaskScheduler *scheduler, Task **task
 		{
 			TaskPool *pool = current_task->pool;
 
-			if (forced_background && !pool->use_force_background) {
+			if (scheduler->background_thread_only && !pool->run_in_background) {
 				continue;
 			}
 
@@ -169,7 +170,7 @@ static bool task_scheduler_thread_wait_pop(TaskScheduler *scheduler, Task **task
 	return true;
 }
 
-static void *task_scheduler_thread_run_ex(void *thread_p, const bool forced_background)
+static void *task_scheduler_thread_run(void *thread_p)
 {
 	TaskThread *thread = (TaskThread *) thread_p;
 	TaskScheduler *scheduler = thread->scheduler;
@@ -177,7 +178,7 @@ static void *task_scheduler_thread_run_ex(void *thread_p, const bool forced_back
 	Task *task;
 
 	/* keep popping off tasks */
-	while (task_scheduler_thread_wait_pop(scheduler, &task, forced_background)) {
+	while (task_scheduler_thread_wait_pop(scheduler, &task)) {
 		TaskPool *pool = task->pool;
 
 		/* run task */
@@ -194,16 +195,6 @@ static void *task_scheduler_thread_run_ex(void *thread_p, const bool forced_back
 	return NULL;
 }
 
-static void *task_scheduler_thread_run(void *thread_p)
-{
-	return task_scheduler_thread_run_ex(thread_p, false);
-}
-
-static void *task_scheduler_thread_run_forced_background(void *thread_p)
-{
-	return task_scheduler_thread_run_ex(thread_p, true);
-}
-
 TaskScheduler *BLI_task_scheduler_create(int num_threads)
 {
 	TaskScheduler *scheduler = MEM_callocN(sizeof(TaskScheduler), "TaskScheduler");
@@ -224,6 +215,12 @@ TaskScheduler *BLI_task_scheduler_create(int num_threads)
 	/* main thread will also work, so we count it too */
 	num_threads -= 1;
 
+	/* Add background-only thread if needed. */
+	if (num_threads == 0) {
+	    scheduler->background_thread_only = true;
+	    num_threads = 1;
+	}
+
 	/* launch threads that will be waiting for work */
 	if (num_threads > 0) {
 		int i;
@@ -242,21 +239,6 @@ TaskScheduler *BLI_task_scheduler_create(int num_threads)
 			}
 		}
 	}
-	else {
-		/* We create a thread, but only for pools that are 'forced background'. */
-		TaskThread *thread;
-
-		scheduler->num_threads = 1;
-		scheduler->threads = MEM_callocN(sizeof(pthread_t), "TaskScheduler threads");
-		thread = scheduler->task_threads = MEM_callocN(sizeof(TaskThread), "TaskScheduler task threads");
-
-		thread->scheduler = scheduler;
-		thread->id = 1;
-
-		if (pthread_create(&scheduler->threads[0], NULL, task_scheduler_thread_run_forced_background, thread) != 0) {
-			fprintf(stderr, "TaskScheduler failed to launch forced background thread\n");
-		}
-	}
 
 	return scheduler;
 }
@@ -353,12 +335,24 @@ static TaskPool *task_pool_create_ex(TaskScheduler *scheduler, void *userdata, c
 {
 	TaskPool *pool = MEM_callocN(sizeof(TaskPool), "TaskPool");
 
+#ifndef NDEBUG
+	/* Assert we do not try to create a background pool from some parent task - those only work OK from main thread. */
+	if (is_background) {
+		const pthread_t thread_id = pthread_self();
+        int i = scheduler->num_threads;
+
+		while (i--) {
+			BLI_assert(scheduler->threads[i] != thread_id);
+		}
+	}
+#endif
+
 	pool->scheduler = scheduler;
 	pool->num = 0;
 	pool->num_threads = 0;
 	pool->currently_running_tasks = 0;
 	pool->do_cancel = false;
-	pool->use_force_background = is_background;
+	pool->run_in_background = is_background;
 
 	BLI_mutex_init(&pool->num_mutex);
 	BLI_condition_init(&pool->num_cond);
@@ -379,7 +373,7 @@ static TaskPool *task_pool_create_ex(TaskScheduler *scheduler, void *userdata, c
 
 /**
  * Create a normal task pool.
- * This means that in mono-threaded context, it will not be executed at all until you call
+ * This means that in single-threaded context, it will not be executed at all until you call
  * \a BLI_task_pool_work_and_wait() on it.
  */
 TaskPool *BLI_task_pool_create(TaskScheduler *scheduler, void *userdata)
@@ -389,9 +383,13 @@ TaskPool *BLI_task_pool_create(TaskScheduler *scheduler, void *userdata)
 
 /**
  * Create a background task pool.
- * In multi-threaded context, there is no differences with \a BLI_task_pool_create(), but in mono-threaded case
+ * In multi-threaded context, there is no differences with \a BLI_task_pool_create(), but in single-threaded case
  * it is assured to have at least one worker thread to run on (i.e. you do not have to call
  * \a BLI_task_pool_work_and_wait() on it to be sure it will be processed).
+ *
+ * \note Background pools are non-recursive (that is, you should not create other background pools in tasks assigned
+ *       to a brackground pool, they could end never being executed, since the 'fallback' background thread is already
+ *       busy with parent task in single-threaded context).
  */
 TaskPool *BLI_task_pool_create_background(TaskScheduler *scheduler, void *userdata)
 {