diff options
Diffstat (limited to 'source/blender/blenlib')
| -rw-r--r-- | source/blender/blenlib/BLI_task.h | 76 | ||||
| -rw-r--r-- | source/blender/blenlib/intern/task_graph.cc | 2 | ||||
| -rw-r--r-- | source/blender/blenlib/intern/task_iterator.c | 4 | ||||
| -rw-r--r-- | source/blender/blenlib/intern/task_pool.cc | 63 | ||||
| -rw-r--r-- | source/blender/blenlib/intern/task_range.cc | 12 | ||||
| -rw-r--r-- | source/blender/blenlib/intern/task_scheduler.cc | 10 | ||||
| -rw-r--r-- | source/blender/blenlib/tests/BLI_linklist_lockfree_test.cc | 2 |
7 files changed, 61 insertions, 108 deletions
diff --git a/source/blender/blenlib/BLI_task.h b/source/blender/blenlib/BLI_task.h index 83ccfda7e38..d6b068c3889 100644 --- a/source/blender/blenlib/BLI_task.h +++ b/source/blender/blenlib/BLI_task.h @@ -67,55 +67,17 @@ typedef enum TaskPriority { TASK_PRIORITY_HIGH, } TaskPriority; -/** - * Task isolation helps avoid unexpected task scheduling decisions that can lead to bugs if wrong - * assumptions were made. Typically that happens when doing "nested threading", i.e. one thread - * schedules a bunch of main-tasks and those spawn new sub-tasks. - * - * What can happen is that when a main-task waits for its sub-tasks to complete on other threads, - * another main-task is scheduled within the already running main-task. Generally, this is good, - * because it leads to better performance. However, sometimes code (often unintentionally) makes - * the assumption that at most one main-task runs on a thread at a time. - * - * The bugs often show themselves in two ways: - * - Deadlock, when a main-task holds a mutex while waiting for its sub-tasks to complete. - * - Data corruption, when a main-task makes wrong assumptions about a thread-local variable. - * - * Task isolation can avoid these bugs by making sure that a main-task does not start executing - * another main-task while waiting for its sub-tasks. More precisely, a function that runs in an - * isolated region is only allowed to run sub-tasks that were spawned in the same isolated region. - * - * Unfortunately, incorrect use of task isolation can lead to deadlocks itself. This can happen - * when threading primitives are used that separate spawning tasks from executing them. The problem - * occurs when a task is spawned in one isolated region while the tasks are waited for in another - * isolated region. In this setup, the thread that is waiting for the spawned tasks to complete - * cannot run the tasks itself. On a single thread, that causes a deadlock already. When there are - * multiple threads, another thread will typically run the task and avoid the deadlock. However, if - * this situation happens on all threads at the same time, all threads will deadlock. This happened - * in T88598. - */ -typedef enum TaskIsolation { - /* Do not use task isolation. Always use this when tasks are pushed recursively. */ - TASK_ISOLATION_OFF, - /* Run each task in its own isolated region. */ - TASK_ISOLATION_ON, -} TaskIsolation; - typedef struct TaskPool TaskPool; typedef void (*TaskRunFunction)(TaskPool *__restrict pool, void *taskdata); typedef void (*TaskFreeFunction)(TaskPool *__restrict pool, void *taskdata); /* Regular task pool that immediately starts executing tasks as soon as they * are pushed, either on the current or another thread. */ -TaskPool *BLI_task_pool_create(void *userdata, - TaskPriority priority, - TaskIsolation task_isolation); +TaskPool *BLI_task_pool_create(void *userdata, TaskPriority priority); /* Background: always run tasks in a background thread, never immediately * execute them. For running background jobs. */ -TaskPool *BLI_task_pool_create_background(void *userdata, - TaskPriority priority, - TaskIsolation task_isolation); +TaskPool *BLI_task_pool_create_background(void *userdata, TaskPriority priority); /* Background Serial: run tasks one after the other in the background, * without parallelization between the tasks. */ @@ -125,9 +87,7 @@ TaskPool *BLI_task_pool_create_background_serial(void *userdata, TaskPriority pr * as threads can't immediately start working. But it can be used if the data * structures the threads operate on are not fully initialized until all tasks * are created. */ -TaskPool *BLI_task_pool_create_suspended(void *userdata, - TaskPriority priority, - TaskIsolation task_isolation); +TaskPool *BLI_task_pool_create_suspended(void *userdata, TaskPriority priority); /* No threads: immediately executes tasks on the same thread. For debugging. */ TaskPool *BLI_task_pool_create_no_threads(void *userdata); @@ -365,6 +325,36 @@ struct TaskNode *BLI_task_graph_node_create(struct TaskGraph *task_graph, bool BLI_task_graph_node_push_work(struct TaskNode *task_node); void BLI_task_graph_edge_create(struct TaskNode *from_node, struct TaskNode *to_node); +/* Task Isolation + * + * Task isolation helps avoid unexpected task scheduling decisions that can lead to bugs if wrong + * assumptions were made. Typically that happens when doing "nested threading", i.e. one thread + * schedules a bunch of main-tasks and those spawn new sub-tasks. + * + * What can happen is that when a main-task waits for its sub-tasks to complete on other threads, + * another main-task is scheduled within the already running main-task. Generally, this is good, + * because it leads to better performance. However, sometimes code (often unintentionally) makes + * the assumption that at most one main-task runs on a thread at a time. + * + * The bugs often show themselves in two ways: + * - Deadlock, when a main-task holds a mutex while waiting for its sub-tasks to complete. + * - Data corruption, when a main-task makes wrong assumptions about a thread-local variable. + * + * Task isolation can avoid these bugs by making sure that a main-task does not start executing + * another main-task while waiting for its sub-tasks. More precisely, a function that runs in an + * isolated region is only allowed to run sub-tasks that were spawned in the same isolated region. + * + * Unfortunately, incorrect use of task isolation can lead to deadlocks itself. This can happen + * when threading primitives are used that separate spawning tasks from executing them. The problem + * occurs when a task is spawned in one isolated region while the tasks are waited for in another + * isolated region. In this setup, the thread that is waiting for the spawned tasks to complete + * cannot run the tasks itself. On a single thread, that causes a deadlock already. When there are + * multiple threads, another thread will typically run the task and avoid the deadlock. However, if + * this situation happens on all threads at the same time, all threads will deadlock. This happened + * in T88598. + */ +void BLI_task_isolate(void (*func)(void *userdata), void *userdata); + #ifdef __cplusplus } #endif diff --git a/source/blender/blenlib/intern/task_graph.cc b/source/blender/blenlib/intern/task_graph.cc index 32450c16630..ff7d0ecb4c4 100644 --- a/source/blender/blenlib/intern/task_graph.cc +++ b/source/blender/blenlib/intern/task_graph.cc @@ -91,7 +91,7 @@ struct TaskNode { #ifdef WITH_TBB tbb::flow::continue_msg run(const tbb::flow::continue_msg UNUSED(input)) { - tbb::this_task_arena::isolate([this] { run_func(task_data); }); + run_func(task_data); return tbb::flow::continue_msg(); } #endif diff --git a/source/blender/blenlib/intern/task_iterator.c b/source/blender/blenlib/intern/task_iterator.c index ee41c277b34..f67671c65e0 100644 --- a/source/blender/blenlib/intern/task_iterator.c +++ b/source/blender/blenlib/intern/task_iterator.c @@ -237,7 +237,7 @@ static void task_parallel_iterator_do(const TaskParallelSettings *settings, void *userdata_chunk_array = NULL; const bool use_userdata_chunk = (userdata_chunk_size != 0) && (userdata_chunk != NULL); - TaskPool *task_pool = BLI_task_pool_create(state, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON); + TaskPool *task_pool = BLI_task_pool_create(state, TASK_PRIORITY_HIGH); if (use_userdata_chunk) { userdata_chunk_array = MALLOCA(userdata_chunk_size * num_tasks); @@ -442,7 +442,7 @@ void BLI_task_parallel_mempool(BLI_mempool *mempool, return; } - task_pool = BLI_task_pool_create(&state, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON); + task_pool = BLI_task_pool_create(&state, TASK_PRIORITY_HIGH); num_threads = BLI_task_scheduler_num_threads(); /* The idea here is to prevent creating task for each of the loop iterations diff --git a/source/blender/blenlib/intern/task_pool.cc b/source/blender/blenlib/intern/task_pool.cc index d72674c1c00..6250c1b9986 100644 --- a/source/blender/blenlib/intern/task_pool.cc +++ b/source/blender/blenlib/intern/task_pool.cc @@ -22,7 +22,6 @@ #include <cstdlib> #include <memory> -#include <thread> #include <utility> #include "MEM_guardedalloc.h" @@ -156,7 +155,6 @@ enum TaskPoolType { struct TaskPool { TaskPoolType type; bool use_threads; - TaskIsolation task_isolation; ThreadMutex user_mutex; void *userdata; @@ -164,8 +162,6 @@ struct TaskPool { #ifdef WITH_TBB /* TBB task pool. */ TBBTaskGroup tbb_group; - /* This is used to detect a common way to accidentally create a deadlock with task isolation. */ - std::thread::id task_pool_create_thread_id; #endif volatile bool is_suspended; BLI_mempool *suspended_mempool; @@ -179,33 +175,9 @@ struct TaskPool { /* Execute task. */ void Task::operator()() const { -#ifdef WITH_TBB - if (pool->task_isolation == TASK_ISOLATION_ON) { - tbb::this_task_arena::isolate([this] { run(pool, taskdata); }); - return; - } -#endif run(pool, taskdata); } -static void assert_on_valid_thread(TaskPool *pool) -{ - /* TODO: Remove this `return` to enable the check. */ - return; -#ifdef DEBUG -# ifdef WITH_TBB - if (pool->task_isolation == TASK_ISOLATION_ON) { - const std::thread::id current_id = std::this_thread::get_id(); - /* This task pool is modified from different threads. To avoid deadlocks, `TASK_ISOLATION_OFF` - * has to be used. Task isolation can still be used in a more fine-grained way within the - * tasks, but should not be enabled for the entire task pool. */ - BLI_assert(pool->task_pool_create_thread_id == current_id); - } -# endif -#endif - UNUSED_VARS_NDEBUG(pool); -} - /* TBB Task Pool. * * Task pool using the TBB scheduler for tasks. When building without TBB @@ -391,10 +363,7 @@ static void background_task_pool_free(TaskPool *pool) /* Task Pool */ -static TaskPool *task_pool_create_ex(void *userdata, - TaskPoolType type, - TaskPriority priority, - TaskIsolation task_isolation) +static TaskPool *task_pool_create_ex(void *userdata, TaskPoolType type, TaskPriority priority) { const bool use_threads = BLI_task_scheduler_num_threads() > 1 && type != TASK_POOL_NO_THREADS; @@ -410,11 +379,6 @@ static TaskPool *task_pool_create_ex(void *userdata, pool->type = type; pool->use_threads = use_threads; - pool->task_isolation = task_isolation; - -#ifdef WITH_TBB - pool->task_pool_create_thread_id = std::this_thread::get_id(); -#endif pool->userdata = userdata; BLI_mutex_init(&pool->user_mutex); @@ -437,9 +401,9 @@ static TaskPool *task_pool_create_ex(void *userdata, /** * Create a normal task pool. Tasks will be executed as soon as they are added. */ -TaskPool *BLI_task_pool_create(void *userdata, TaskPriority priority, TaskIsolation task_isolation) +TaskPool *BLI_task_pool_create(void *userdata, TaskPriority priority) { - return task_pool_create_ex(userdata, TASK_POOL_TBB, priority, task_isolation); + return task_pool_create_ex(userdata, TASK_POOL_TBB, priority); } /** @@ -454,11 +418,9 @@ TaskPool *BLI_task_pool_create(void *userdata, TaskPriority priority, TaskIsolat * 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(void *userdata, - TaskPriority priority, - TaskIsolation task_isolation) +TaskPool *BLI_task_pool_create_background(void *userdata, TaskPriority priority) { - return task_pool_create_ex(userdata, TASK_POOL_BACKGROUND, priority, task_isolation); + return task_pool_create_ex(userdata, TASK_POOL_BACKGROUND, priority); } /** @@ -466,11 +428,9 @@ TaskPool *BLI_task_pool_create_background(void *userdata, * for until BLI_task_pool_work_and_wait() is called. This helps reducing threading * overhead when pushing huge amount of small initial tasks from the main thread. */ -TaskPool *BLI_task_pool_create_suspended(void *userdata, - TaskPriority priority, - TaskIsolation task_isolation) +TaskPool *BLI_task_pool_create_suspended(void *userdata, TaskPriority priority) { - return task_pool_create_ex(userdata, TASK_POOL_TBB_SUSPENDED, priority, task_isolation); + return task_pool_create_ex(userdata, TASK_POOL_TBB_SUSPENDED, priority); } /** @@ -479,8 +439,7 @@ TaskPool *BLI_task_pool_create_suspended(void *userdata, */ TaskPool *BLI_task_pool_create_no_threads(void *userdata) { - return task_pool_create_ex( - userdata, TASK_POOL_NO_THREADS, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON); + return task_pool_create_ex(userdata, TASK_POOL_NO_THREADS, TASK_PRIORITY_HIGH); } /** @@ -489,7 +448,7 @@ TaskPool *BLI_task_pool_create_no_threads(void *userdata) */ TaskPool *BLI_task_pool_create_background_serial(void *userdata, TaskPriority priority) { - return task_pool_create_ex(userdata, TASK_POOL_BACKGROUND_SERIAL, priority, TASK_ISOLATION_ON); + return task_pool_create_ex(userdata, TASK_POOL_BACKGROUND_SERIAL, priority); } void BLI_task_pool_free(TaskPool *pool) @@ -517,8 +476,6 @@ void BLI_task_pool_push(TaskPool *pool, bool free_taskdata, TaskFreeFunction freedata) { - assert_on_valid_thread(pool); - Task task(pool, run, taskdata, free_taskdata, freedata); switch (pool->type) { @@ -536,8 +493,6 @@ void BLI_task_pool_push(TaskPool *pool, void BLI_task_pool_work_and_wait(TaskPool *pool) { - assert_on_valid_thread(pool); - switch (pool->type) { case TASK_POOL_TBB: case TASK_POOL_TBB_SUSPENDED: diff --git a/source/blender/blenlib/intern/task_range.cc b/source/blender/blenlib/intern/task_range.cc index a27241e91dd..871d04c1f35 100644 --- a/source/blender/blenlib/intern/task_range.cc +++ b/source/blender/blenlib/intern/task_range.cc @@ -90,13 +90,11 @@ struct RangeTask { void operator()(const tbb::blocked_range<int> &r) const { - tbb::this_task_arena::isolate([this, r] { - TaskParallelTLS tls; - tls.userdata_chunk = userdata_chunk; - for (int i = r.begin(); i != r.end(); ++i) { - func(userdata, i, &tls); - } - }); + TaskParallelTLS tls; + tls.userdata_chunk = userdata_chunk; + for (int i = r.begin(); i != r.end(); ++i) { + func(userdata, i, &tls); + } } void join(const RangeTask &other) diff --git a/source/blender/blenlib/intern/task_scheduler.cc b/source/blender/blenlib/intern/task_scheduler.cc index b22334a5676..69117e9dc7e 100644 --- a/source/blender/blenlib/intern/task_scheduler.cc +++ b/source/blender/blenlib/intern/task_scheduler.cc @@ -28,6 +28,7 @@ #ifdef WITH_TBB /* Need to include at least one header to get the version define. */ # include <tbb/blocked_range.h> +# include <tbb/task_arena.h> # if TBB_INTERFACE_VERSION_MAJOR >= 10 # include <tbb/global_control.h> # define WITH_TBB_GLOBAL_CONTROL @@ -76,3 +77,12 @@ int BLI_task_scheduler_num_threads() { return task_scheduler_num_threads; } + +void BLI_task_isolate(void (*func)(void *userdata), void *userdata) +{ +#ifdef WITH_TBB + tbb::this_task_arena::isolate([&] { func(userdata); }); +#else + func(userdata); +#endif +} diff --git a/source/blender/blenlib/tests/BLI_linklist_lockfree_test.cc b/source/blender/blenlib/tests/BLI_linklist_lockfree_test.cc index 8be89d66062..e9810aed179 100644 --- a/source/blender/blenlib/tests/BLI_linklist_lockfree_test.cc +++ b/source/blender/blenlib/tests/BLI_linklist_lockfree_test.cc @@ -81,7 +81,7 @@ TEST(LockfreeLinkList, InsertMultipleConcurrent) LockfreeLinkList list; BLI_linklist_lockfree_init(&list); /* Initialize task scheduler and pool. */ - TaskPool *pool = BLI_task_pool_create_suspended(&list, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON); + TaskPool *pool = BLI_task_pool_create_suspended(&list, TASK_PRIORITY_HIGH); /* Push tasks to the pool. */ for (int i = 0; i < num_nodes; ++i) { BLI_task_pool_push(pool, concurrent_insert, POINTER_FROM_INT(i), false, nullptr); |