Page MenuHome

Sculpt: use TBB instead of BLI_task for multithreading

Authored by Brecht Van Lommel (brecht) on Wed, Oct 9, 5:12 PM.



This solves performance issues on some computers where there is significant
threading overhead. Rather than doing the complicated work of optimizing our
own task scheduler, use TBB which appears to work well. The downside is that
we have another thread pool, but it is already there when using OpenVDB voxel

For future releases we can switch to using TBB to replace our task scheduler
implementation entirely, and use the same thread pool for BLI_task, Cycles,
Mantaflow, etc.

See D5996: Sculpt: Change default min_iter_per_thread for more details.

Diff Detail

rB Blender

Event Timeline

Brecht Van Lommel (brecht) planned changes to this revision.Wed, Oct 9, 5:14 PM

This is still missing the thread id, which is used by brush textures (not sure it's strictly needed even).

@Sergey Sharybin (sergey), do you perhaps have some code for this from an earlier test with TBB? I can probably figure it out, but it's not immediately obvious to me.

Unfortunately, don't have anything finished. Ended up passing thread_id of 0 for the tests i needed.
There is tbb::this_tbb_thread::get_id(), but it's not in a range we would expect it to be.

Thing is, in an ideal world the application code should not use thread_id for decision-making.
The only thing which really needs it is the texture nodes. The rest is our attempt to optimize pushing tasks from worker thread (which we don't need anymore if we go TBB). But even then it might easier to have something like:

static int global_thread_id_counter = 0;
__local int thread_id = -1;
if (thread_id == -1) {
  thread_id = atomic_fetch_and_add_uint64(&global_thread_id_counter, 1);

Here is the patch with my experiments from back in the days. Might be useful for something.

Add thread_id support and some other fixes. I looked at getting texture nodes
to not require a thread ID but it's complicated and we plan to rewrite them
anyway. In practice this mechanism should work fine.

This revision is now accepted and ready to land.Thu, Oct 10, 5:10 PM