diff options
Diffstat (limited to 'source')
28 files changed, 175 insertions, 161 deletions
diff --git a/source/blender/blenkernel/intern/bvhutils.c b/source/blender/blenkernel/intern/bvhutils.c index bc63e423c09..116e6279657 100644 --- a/source/blender/blenkernel/intern/bvhutils.c +++ b/source/blender/blenkernel/intern/bvhutils.c @@ -31,6 +31,7 @@ #include "BLI_linklist.h" #include "BLI_math.h" +#include "BLI_task.h" #include "BLI_threads.h" #include "BLI_utildefines.h" @@ -160,6 +161,26 @@ void bvhcache_free(BVHCache *bvh_cache) MEM_freeN(bvh_cache); } +/* BVH tree balancing inside a mutex lock must be run in isolation. Balancing + * is multithreaded, and we do not want the current thread to start another task + * that may involve acquiring the same mutex lock that it is waiting for. */ +static void bvhtree_balance_isolated(void *userdata) +{ + BLI_bvhtree_balance((BVHTree *)userdata); +} + +static void bvhtree_balance(BVHTree *tree, const bool isolate) +{ + if (tree) { + if (isolate) { + BLI_task_isolate(bvhtree_balance_isolated, tree); + } + else { + BLI_bvhtree_balance(tree); + } + } +} + /** \} */ /* -------------------------------------------------------------------- */ /** \name Local Callbacks @@ -566,7 +587,6 @@ static BVHTree *bvhtree_from_editmesh_verts_create_tree(float epsilon, BLI_bvhtree_insert(tree, i, eve->co, 1); } BLI_assert(BLI_bvhtree_get_len(tree) == verts_num_active); - BLI_bvhtree_balance(tree); } return tree; @@ -600,7 +620,6 @@ static BVHTree *bvhtree_from_mesh_verts_create_tree(float epsilon, BLI_bvhtree_insert(tree, i, vert[i].co, 1); } BLI_assert(BLI_bvhtree_get_len(tree) == verts_num_active); - BLI_bvhtree_balance(tree); } } @@ -649,6 +668,7 @@ BVHTree *bvhtree_from_editmesh_verts_ex(BVHTreeFromEditMesh *data, if (data->cached == false) { tree = bvhtree_from_editmesh_verts_create_tree( epsilon, tree_type, axis, em, verts_mask, verts_num_active); + bvhtree_balance(tree, true); /* Save on cache for later use */ /* printf("BVHTree built and saved on cache\n"); */ @@ -660,6 +680,7 @@ BVHTree *bvhtree_from_editmesh_verts_ex(BVHTreeFromEditMesh *data, else { tree = bvhtree_from_editmesh_verts_create_tree( epsilon, tree_type, axis, em, verts_mask, verts_num_active); + bvhtree_balance(tree, false); } if (tree) { @@ -711,6 +732,7 @@ BVHTree *bvhtree_from_mesh_verts_ex(BVHTreeFromMesh *data, if (in_cache == false) { tree = bvhtree_from_mesh_verts_create_tree( epsilon, tree_type, axis, vert, verts_num, verts_mask, verts_num_active); + bvhtree_balance(tree, bvh_cache_p != NULL); if (bvh_cache_p) { /* Save on cache for later use */ @@ -771,7 +793,6 @@ static BVHTree *bvhtree_from_editmesh_edges_create_tree(float epsilon, BLI_bvhtree_insert(tree, i, co[0], 2); } BLI_assert(BLI_bvhtree_get_len(tree) == edges_num_active); - BLI_bvhtree_balance(tree); } return tree; @@ -809,7 +830,6 @@ static BVHTree *bvhtree_from_mesh_edges_create_tree(const MVert *vert, BLI_bvhtree_insert(tree, i, co[0], 2); } - BLI_bvhtree_balance(tree); } } @@ -861,7 +881,7 @@ BVHTree *bvhtree_from_editmesh_edges_ex(BVHTreeFromEditMesh *data, if (data->cached == false) { tree = bvhtree_from_editmesh_edges_create_tree( epsilon, tree_type, axis, em, edges_mask, edges_num_active); - + bvhtree_balance(tree, true); /* Save on cache for later use */ /* printf("BVHTree built and saved on cache\n"); */ bvhcache_insert(bvh_cache, tree, bvh_cache_type); @@ -872,6 +892,7 @@ BVHTree *bvhtree_from_editmesh_edges_ex(BVHTreeFromEditMesh *data, else { tree = bvhtree_from_editmesh_edges_create_tree( epsilon, tree_type, axis, em, edges_mask, edges_num_active); + bvhtree_balance(tree, false); } if (tree) { @@ -928,12 +949,17 @@ BVHTree *bvhtree_from_mesh_edges_ex(BVHTreeFromMesh *data, vert, edge, edges_num, edges_mask, edges_num_active, epsilon, tree_type, axis); if (bvh_cache_p) { + bvhtree_balance(tree, true); + BVHCache *bvh_cache = *bvh_cache_p; /* Save on cache for later use */ /* printf("BVHTree built and saved on cache\n"); */ bvhcache_insert(bvh_cache, tree, bvh_cache_type); in_cache = true; } + else { + bvhtree_balance(tree, false); + } } if (bvh_cache_p) { @@ -994,7 +1020,6 @@ static BVHTree *bvhtree_from_mesh_faces_create_tree(float epsilon, } } BLI_assert(BLI_bvhtree_get_len(tree) == faces_num_active); - BLI_bvhtree_balance(tree); } } @@ -1057,6 +1082,7 @@ BVHTree *bvhtree_from_mesh_faces_ex(BVHTreeFromMesh *data, if (in_cache == false) { tree = bvhtree_from_mesh_faces_create_tree( epsilon, tree_type, axis, vert, face, numFaces, faces_mask, faces_num_active); + bvhtree_balance(tree, bvh_cache_p != NULL); if (bvh_cache_p) { /* Save on cache for later use */ @@ -1127,7 +1153,6 @@ static BVHTree *bvhtree_from_editmesh_looptri_create_tree(float epsilon, } } BLI_assert(BLI_bvhtree_get_len(tree) == looptri_num_active); - BLI_bvhtree_balance(tree); } } @@ -1173,7 +1198,6 @@ static BVHTree *bvhtree_from_mesh_looptri_create_tree(float epsilon, } } BLI_assert(BLI_bvhtree_get_len(tree) == looptri_num_active); - BLI_bvhtree_balance(tree); } } @@ -1229,6 +1253,7 @@ BVHTree *bvhtree_from_editmesh_looptri_ex(BVHTreeFromEditMesh *data, bool in_cache = bvhcache_find( bvh_cache_p, bvh_cache_type, &tree, &lock_started, mesh_eval_mutex); BVHCache *bvh_cache = *bvh_cache_p; + bvhtree_balance(tree, true); if (in_cache == false) { tree = bvhtree_from_editmesh_looptri_create_tree( @@ -1243,6 +1268,7 @@ BVHTree *bvhtree_from_editmesh_looptri_ex(BVHTreeFromEditMesh *data, else { tree = bvhtree_from_editmesh_looptri_create_tree( epsilon, tree_type, axis, em, looptri_mask, looptri_num_active); + bvhtree_balance(tree, false); } if (tree) { @@ -1303,6 +1329,8 @@ BVHTree *bvhtree_from_mesh_looptri_ex(BVHTreeFromMesh *data, looptri_mask, looptri_num_active); + bvhtree_balance(tree, bvh_cache_p != NULL); + if (bvh_cache_p) { BVHCache *bvh_cache = *bvh_cache_p; bvhcache_insert(bvh_cache, tree, bvh_cache_type); @@ -1742,7 +1770,7 @@ BVHTree *BKE_bvhtree_from_pointcloud_get(BVHTreeFromPointCloud *data, BLI_bvhtree_insert(tree, i, pointcloud->co[i], 1); } BLI_assert(BLI_bvhtree_get_len(tree) == pointcloud->totpoint); - BLI_bvhtree_balance(tree); + bvhtree_balance(tree, false); data->coords = pointcloud->co; data->tree = tree; diff --git a/source/blender/blenkernel/intern/editmesh_tangent.c b/source/blender/blenkernel/intern/editmesh_tangent.c index 088a2087a96..d849f4ab37d 100644 --- a/source/blender/blenkernel/intern/editmesh_tangent.c +++ b/source/blender/blenkernel/intern/editmesh_tangent.c @@ -362,7 +362,7 @@ void BKE_editmesh_loop_tangent_calc(BMEditMesh *em, /* Calculation */ if (em->tottri != 0) { TaskPool *task_pool; - task_pool = BLI_task_pool_create(NULL, TASK_PRIORITY_LOW, TASK_ISOLATION_ON); + task_pool = BLI_task_pool_create(NULL, TASK_PRIORITY_LOW); tangent_mask_curr = 0; /* Calculate tangent layers */ diff --git a/source/blender/blenkernel/intern/image.c b/source/blender/blenkernel/intern/image.c index 2f7e2b41a73..dad518ec696 100644 --- a/source/blender/blenkernel/intern/image.c +++ b/source/blender/blenkernel/intern/image.c @@ -68,6 +68,7 @@ #include "BLI_math_vector.h" #include "BLI_mempool.h" #include "BLI_system.h" +#include "BLI_task.h" #include "BLI_threads.h" #include "BLI_timecode.h" /* For stamp time-code format. */ #include "BLI_utildefines.h" @@ -882,6 +883,39 @@ Image *BKE_image_load_exists(Main *bmain, const char *filepath) return BKE_image_load_exists_ex(bmain, filepath, NULL); } +typedef struct ImageFillData { + short gen_type; + uint width; + uint height; + unsigned char *rect; + float *rect_float; + float fill_color[4]; +} ImageFillData; + +static void image_buf_fill_isolated(void *usersata_v) +{ + ImageFillData *usersata = usersata_v; + + const short gen_type = usersata->gen_type; + const uint width = usersata->width; + const uint height = usersata->height; + + unsigned char *rect = usersata->rect; + float *rect_float = usersata->rect_float; + + switch (gen_type) { + case IMA_GENTYPE_GRID: + BKE_image_buf_fill_checker(rect, rect_float, width, height); + break; + case IMA_GENTYPE_GRID_COLOR: + BKE_image_buf_fill_checker_color(rect, rect_float, width, height); + break; + default: + BKE_image_buf_fill_color(rect, rect_float, width, height, usersata->fill_color); + break; + } +} + static ImBuf *add_ibuf_size(unsigned int width, unsigned int height, const char *name, @@ -944,17 +978,16 @@ static ImBuf *add_ibuf_size(unsigned int width, STRNCPY(ibuf->name, name); - switch (gen_type) { - case IMA_GENTYPE_GRID: - BKE_image_buf_fill_checker(rect, rect_float, width, height); - break; - case IMA_GENTYPE_GRID_COLOR: - BKE_image_buf_fill_checker_color(rect, rect_float, width, height); - break; - default: - BKE_image_buf_fill_color(rect, rect_float, width, height, fill_color); - break; - } + ImageFillData data; + + data.gen_type = gen_type; + data.width = width; + data.height = height; + data.rect = rect; + data.rect_float = rect_float; + copy_v4_v4(data.fill_color, fill_color); + + BLI_task_isolate(image_buf_fill_isolated, &data); return ibuf; } diff --git a/source/blender/blenkernel/intern/lib_override.c b/source/blender/blenkernel/intern/lib_override.c index 6b2ffa3b944..c93971e7b11 100644 --- a/source/blender/blenkernel/intern/lib_override.c +++ b/source/blender/blenkernel/intern/lib_override.c @@ -2335,8 +2335,7 @@ bool BKE_lib_override_library_main_operations_create(Main *bmain, const bool for } struct LibOverrideOpCreateData create_pool_data = {.bmain = bmain, .changed = false}; - TaskPool *task_pool = BLI_task_pool_create( - &create_pool_data, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON); + TaskPool *task_pool = BLI_task_pool_create(&create_pool_data, TASK_PRIORITY_HIGH); FOREACH_MAIN_ID_BEGIN (bmain, id) { if (!ID_IS_LINKED(id) && ID_IS_OVERRIDE_LIBRARY_REAL(id) && diff --git a/source/blender/blenkernel/intern/mesh_evaluate.c b/source/blender/blenkernel/intern/mesh_evaluate.c index 826094420a7..bd79ec4a4bd 100644 --- a/source/blender/blenkernel/intern/mesh_evaluate.c +++ b/source/blender/blenkernel/intern/mesh_evaluate.c @@ -1715,8 +1715,7 @@ void BKE_mesh_normals_loop_split(const MVert *mverts, loop_split_generator(NULL, &common_data); } else { - TaskPool *task_pool = BLI_task_pool_create( - &common_data, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON); + TaskPool *task_pool = BLI_task_pool_create(&common_data, TASK_PRIORITY_HIGH); loop_split_generator(task_pool, &common_data); diff --git a/source/blender/blenkernel/intern/mesh_tangent.c b/source/blender/blenkernel/intern/mesh_tangent.c index 6a7ff0851f5..2e22e521a13 100644 --- a/source/blender/blenkernel/intern/mesh_tangent.c +++ b/source/blender/blenkernel/intern/mesh_tangent.c @@ -656,7 +656,7 @@ void BKE_mesh_calc_loop_tangent_ex(const MVert *mvert, /* Calculation */ if (looptri_len != 0) { - TaskPool *task_pool = BLI_task_pool_create(NULL, TASK_PRIORITY_LOW, TASK_ISOLATION_ON); + TaskPool *task_pool = BLI_task_pool_create(NULL, TASK_PRIORITY_LOW); tangent_mask_curr = 0; /* Calculate tangent layers */ diff --git a/source/blender/blenkernel/intern/ocean.c b/source/blender/blenkernel/intern/ocean.c index 9b9ed0adcf4..9d53dad8d03 100644 --- a/source/blender/blenkernel/intern/ocean.c +++ b/source/blender/blenkernel/intern/ocean.c @@ -663,7 +663,7 @@ void BKE_ocean_simulate(struct Ocean *o, float t, float scale, float chop_amount osd.scale = scale; osd.chop_amount = chop_amount; - pool = BLI_task_pool_create(&osd, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON); + pool = BLI_task_pool_create(&osd, TASK_PRIORITY_HIGH); BLI_rw_mutex_lock(&o->oceanmutex, THREAD_LOCK_WRITE); diff --git a/source/blender/blenkernel/intern/particle.c b/source/blender/blenkernel/intern/particle.c index 3ae5d039125..a873ecec6f1 100644 --- a/source/blender/blenkernel/intern/particle.c +++ b/source/blender/blenkernel/intern/particle.c @@ -3179,7 +3179,7 @@ void psys_cache_child_paths(ParticleSimulationData *sim, return; } - task_pool = BLI_task_pool_create(&ctx, TASK_PRIORITY_LOW, TASK_ISOLATION_ON); + task_pool = BLI_task_pool_create(&ctx, TASK_PRIORITY_LOW); totchild = ctx.totchild; totparent = ctx.totparent; diff --git a/source/blender/blenkernel/intern/particle_distribute.c b/source/blender/blenkernel/intern/particle_distribute.c index 6cae6cd6fa2..ad617b4198b 100644 --- a/source/blender/blenkernel/intern/particle_distribute.c +++ b/source/blender/blenkernel/intern/particle_distribute.c @@ -1330,7 +1330,7 @@ static void distribute_particles_on_dm(ParticleSimulationData *sim, int from) return; } - task_pool = BLI_task_pool_create(&ctx, TASK_PRIORITY_LOW, TASK_ISOLATION_ON); + task_pool = BLI_task_pool_create(&ctx, TASK_PRIORITY_LOW); totpart = (from == PART_FROM_CHILD ? sim->psys->totchild : sim->psys->totpart); psys_tasks_create(&ctx, 0, totpart, &tasks, &numtasks); diff --git a/source/blender/blenkernel/intern/volume.cc b/source/blender/blenkernel/intern/volume.cc index c0ce57818d1..41f0b5c6b72 100644 --- a/source/blender/blenkernel/intern/volume.cc +++ b/source/blender/blenkernel/intern/volume.cc @@ -324,15 +324,20 @@ struct VolumeGrid { openvdb::io::File file(filepath); - try { - file.setCopyMaxBytes(0); - file.open(); - openvdb::GridBase::Ptr vdb_grid = file.readGrid(name()); - entry->grid->setTree(vdb_grid->baseTreePtr()); - } - catch (const openvdb::IoError &e) { - entry->error_msg = e.what(); - } + /* Isolate file loading since that's potentially multithreaded and we are + * holding a mutex lock. See BLI_task_isolate. Note OpenVDB already uses + * TBB, so it's fine to use here without a wrapper. */ + tbb::this_task_arena::isolate([&] { + try { + file.setCopyMaxBytes(0); + file.open(); + openvdb::GridBase::Ptr vdb_grid = file.readGrid(name()); + entry->grid->setTree(vdb_grid->baseTreePtr()); + } + catch (const openvdb::IoError &e) { + entry->error_msg = e.what(); + } + }); std::atomic_thread_fence(std::memory_order_release); entry->is_loaded = true; 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); diff --git a/source/blender/compositor/intern/COM_WorkScheduler.cc b/source/blender/compositor/intern/COM_WorkScheduler.cc index cd0139fd18e..157ded943d6 100644 --- a/source/blender/compositor/intern/COM_WorkScheduler.cc +++ b/source/blender/compositor/intern/COM_WorkScheduler.cc @@ -411,8 +411,7 @@ static void threading_model_task_schedule(WorkPackage *package) static void threading_model_task_start() { BLI_thread_local_create(g_thread_device); - g_work_scheduler.task.pool = BLI_task_pool_create( - nullptr, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON); + g_work_scheduler.task.pool = BLI_task_pool_create(nullptr, TASK_PRIORITY_HIGH); } static void threading_model_task_finish() diff --git a/source/blender/depsgraph/intern/eval/deg_eval.cc b/source/blender/depsgraph/intern/eval/deg_eval.cc index 2107e075139..620e86550cc 100644 --- a/source/blender/depsgraph/intern/eval/deg_eval.cc +++ b/source/blender/depsgraph/intern/eval/deg_eval.cc @@ -353,8 +353,7 @@ static TaskPool *deg_evaluate_task_pool_create(DepsgraphEvalState *state) return BLI_task_pool_create_no_threads(state); } - /* TODO: Disable task isolation. */ - return BLI_task_pool_create_suspended(state, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON); + return BLI_task_pool_create_suspended(state, TASK_PRIORITY_HIGH); } /** diff --git a/source/blender/editors/mesh/editmesh_undo.c b/source/blender/editors/mesh/editmesh_undo.c index 274f4cdbb6c..112de68b52c 100644 --- a/source/blender/editors/mesh/editmesh_undo.c +++ b/source/blender/editors/mesh/editmesh_undo.c @@ -626,7 +626,7 @@ static void *undomesh_from_editmesh(UndoMesh *um, BMEditMesh *em, Key *key, Undo # ifdef USE_ARRAY_STORE_THREAD if (um_arraystore.task_pool == NULL) { - um_arraystore.task_pool = BLI_task_pool_create_background(NULL, TASK_PRIORITY_LOW, true); + um_arraystore.task_pool = BLI_task_pool_create_background(NULL, TASK_PRIORITY_LOW); } struct UMArrayData *um_data = MEM_mallocN(sizeof(*um_data), __func__); diff --git a/source/blender/editors/render/render_opengl.c b/source/blender/editors/render/render_opengl.c index cfc07de3f6c..48f937fb4ec 100644 --- a/source/blender/editors/render/render_opengl.c +++ b/source/blender/editors/render/render_opengl.c @@ -859,7 +859,7 @@ static bool screen_opengl_render_init(bContext *C, wmOperator *op) oglrender->task_pool = BLI_task_pool_create_background_serial(oglrender, TASK_PRIORITY_LOW); } else { - oglrender->task_pool = BLI_task_pool_create(oglrender, TASK_PRIORITY_LOW, TASK_ISOLATION_ON); + oglrender->task_pool = BLI_task_pool_create(oglrender, TASK_PRIORITY_LOW); } oglrender->pool_ok = true; BLI_spin_init(&oglrender->reports_lock); diff --git a/source/blender/editors/sculpt_paint/paint_image_proj.c b/source/blender/editors/sculpt_paint/paint_image_proj.c index 41db79bc134..26f86c8eabb 100644 --- a/source/blender/editors/sculpt_paint/paint_image_proj.c +++ b/source/blender/editors/sculpt_paint/paint_image_proj.c @@ -5571,7 +5571,7 @@ static bool project_paint_op(void *state, const float lastpos[2], const float po } if (ps->thread_tot > 1) { - task_pool = BLI_task_pool_create_suspended(NULL, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON); + task_pool = BLI_task_pool_create_suspended(NULL, TASK_PRIORITY_HIGH); } image_pool = BKE_image_pool_new(); diff --git a/source/blender/editors/space_clip/clip_editor.c b/source/blender/editors/space_clip/clip_editor.c index 9aa6a993c13..2da13646a8b 100644 --- a/source/blender/editors/space_clip/clip_editor.c +++ b/source/blender/editors/space_clip/clip_editor.c @@ -904,7 +904,7 @@ static void start_prefetch_threads(MovieClip *clip, queue.do_update = do_update; queue.progress = progress; - TaskPool *task_pool = BLI_task_pool_create(&queue, TASK_PRIORITY_LOW, TASK_ISOLATION_ON); + TaskPool *task_pool = BLI_task_pool_create(&queue, TASK_PRIORITY_LOW); for (int i = 0; i < tot_thread; i++) { BLI_task_pool_push(task_pool, prefetch_task_func, clip, false, NULL); } diff --git a/source/blender/editors/space_clip/clip_ops.c b/source/blender/editors/space_clip/clip_ops.c index 369a6e7c944..f5e4c4d55d9 100644 --- a/source/blender/editors/space_clip/clip_ops.c +++ b/source/blender/editors/space_clip/clip_ops.c @@ -1429,7 +1429,7 @@ static void do_sequence_proxy(void *pjv, queue.do_update = do_update; queue.progress = progress; - TaskPool *task_pool = BLI_task_pool_create(&queue, TASK_PRIORITY_LOW, TASK_ISOLATION_ON); + TaskPool *task_pool = BLI_task_pool_create(&queue, TASK_PRIORITY_LOW); handles = MEM_callocN(sizeof(ProxyThread) * tot_thread, "proxy threaded handles"); for (int i = 0; i < tot_thread; i++) { ProxyThread *handle = &handles[i]; diff --git a/source/blender/editors/space_file/filelist.c b/source/blender/editors/space_file/filelist.c index 8ea44e5c3ee..37a32164cfc 100644 --- a/source/blender/editors/space_file/filelist.c +++ b/source/blender/editors/space_file/filelist.c @@ -1553,8 +1553,7 @@ static void filelist_cache_preview_freef(TaskPool *__restrict UNUSED(pool), void static void filelist_cache_preview_ensure_running(FileListEntryCache *cache) { if (!cache->previews_pool) { - cache->previews_pool = BLI_task_pool_create_background( - cache, TASK_PRIORITY_LOW, TASK_ISOLATION_ON); + cache->previews_pool = BLI_task_pool_create_background(cache, TASK_PRIORITY_LOW); cache->previews_done = BLI_thread_queue_init(); IMB_thumb_locks_acquire(); diff --git a/source/blender/gpencil_modifiers/intern/lineart/lineart_cpu.c b/source/blender/gpencil_modifiers/intern/lineart/lineart_cpu.c index 06d63c3ddab..5a979f01bc3 100644 --- a/source/blender/gpencil_modifiers/intern/lineart/lineart_cpu.c +++ b/source/blender/gpencil_modifiers/intern/lineart/lineart_cpu.c @@ -482,7 +482,7 @@ static void lineart_main_occlusion_begin(LineartRenderBuffer *rb) rb->material.last = rb->material.first; rb->edge_mark.last = rb->edge_mark.first; - TaskPool *tp = BLI_task_pool_create(NULL, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON); + TaskPool *tp = BLI_task_pool_create(NULL, TASK_PRIORITY_HIGH); for (i = 0; i < thread_count; i++) { rti[i].thread_id = i; @@ -1987,7 +1987,7 @@ static void lineart_main_load_geometries( } DEG_OBJECT_ITER_END; - TaskPool *tp = BLI_task_pool_create(NULL, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON); + TaskPool *tp = BLI_task_pool_create(NULL, TASK_PRIORITY_HIGH); for (int i = 0; i < thread_count; i++) { olti[i].rb = rb; diff --git a/source/blender/imbuf/intern/imageprocess.c b/source/blender/imbuf/intern/imageprocess.c index a9b6e2bbb88..e2d469ab5a3 100644 --- a/source/blender/imbuf/intern/imageprocess.c +++ b/source/blender/imbuf/intern/imageprocess.c @@ -523,7 +523,7 @@ void IMB_processor_apply_threaded( int total_tasks = (buffer_lines + lines_per_task - 1) / lines_per_task; int i, start_line; - task_pool = BLI_task_pool_create(do_thread, TASK_PRIORITY_LOW, TASK_ISOLATION_ON); + task_pool = BLI_task_pool_create(do_thread, TASK_PRIORITY_LOW); handles = MEM_callocN(handle_size * total_tasks, "processor apply threaded handles"); diff --git a/source/blender/modifiers/intern/MOD_nodes_evaluator.cc b/source/blender/modifiers/intern/MOD_nodes_evaluator.cc index 170ad0f2c48..bcd10cdbf33 100644 --- a/source/blender/modifiers/intern/MOD_nodes_evaluator.cc +++ b/source/blender/modifiers/intern/MOD_nodes_evaluator.cc @@ -380,7 +380,7 @@ class GeometryNodesEvaluator { void execute() { - task_pool_ = BLI_task_pool_create(this, TASK_PRIORITY_HIGH, TASK_ISOLATION_OFF); + task_pool_ = BLI_task_pool_create(this, TASK_PRIORITY_HIGH); this->create_states_for_reachable_nodes(); this->forward_group_inputs(); |