[Bf-blender-cvs] [fcbec6e97e6] master: BLI_task: Add pooled threaded index range iterator, Take II.

Bastien Montagne noreply at git.blender.org
Tue Nov 26 14:37:25 CET 2019


Commit: fcbec6e97e649eee33f06e0202455ee11dcfe46e
Author: Bastien Montagne
Date:   Tue Nov 26 14:26:47 2019 +0100
Branches: master
https://developer.blender.org/rBfcbec6e97e649eee33f06e0202455ee11dcfe46e

BLI_task: Add pooled threaded index range iterator, Take II.

This code allows to push a set of different operations all based on
iterations over a range of indices, and then process them all at once
over multiple threads.

This commit also adds unit tests for both old un-pooled, and new pooled
task_parallel_range family of functions, as well as some basic
performances tests.

This is mainly interesting for relatively low amount of individual
tasks, as expected.

E.g. performance tests on a 32 threads machine, for a set of 10
different tasks, shows following improvements when using pooled version
instead of ten sequential calls to BLI_task_parallel_range():

| Num Items | Sequential | Pooled  | Speed-up |
| --------- | ---------- | ------- | -------- |
|       10K |     365 us |  138 us |   2.5  x |
|      100K |     877 us |  530 us |   1.66 x |
|     1000K |    5521 us | 4625 us |   1.25 x |

Differential Revision: https://developer.blender.org/D6189

Note: Compared to previous commit yesterday, this reworks atomic handling in
parallel iter code, and fixes a dummy double-free bug.

Now we should only use the two critical values for synchronization from
atomic calls results, which is the proper way to do things.

Reading a value after an atomic operation does not guarantee you will
get the latest value in all cases (especially on Windows release builds
it seems).

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

M	source/blender/blenlib/BLI_task.h
M	source/blender/blenlib/intern/task.c
M	tests/gtests/blenlib/BLI_task_performance_test.cc
M	tests/gtests/blenlib/BLI_task_test.cc

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

diff --git a/source/blender/blenlib/BLI_task.h b/source/blender/blenlib/BLI_task.h
index 7ef5e518cc8..05c3d43a0de 100644
--- a/source/blender/blenlib/BLI_task.h
+++ b/source/blender/blenlib/BLI_task.h
@@ -196,9 +196,22 @@ void BLI_task_parallel_range(const int start,
                              const int stop,
                              void *userdata,
                              TaskParallelRangeFunc func,
-                             const TaskParallelSettings *settings);
-
-/* This data is shared between all tasks, its access needs thread lock or similar protection. */
+                             TaskParallelSettings *settings);
+
+typedef struct TaskParallelRangePool TaskParallelRangePool;
+struct TaskParallelRangePool *BLI_task_parallel_range_pool_init(
+    const struct TaskParallelSettings *settings);
+void BLI_task_parallel_range_pool_push(struct TaskParallelRangePool *range_pool,
+                                       const int start,
+                                       const int stop,
+                                       void *userdata,
+                                       TaskParallelRangeFunc func,
+                                       const struct TaskParallelSettings *settings);
+void BLI_task_parallel_range_pool_work_and_wait(struct TaskParallelRangePool *range_pool);
+void BLI_task_parallel_range_pool_free(struct TaskParallelRangePool *range_pool);
+
+/* This data is shared between all tasks, its access needs thread lock or similar protection.
+ */
 typedef struct TaskParallelIteratorStateShared {
   /* Maximum amount of items to acquire at once. */
   int chunk_size;
diff --git a/source/blender/blenlib/intern/task.c b/source/blender/blenlib/intern/task.c
index 0613b558aec..293275c402d 100644
--- a/source/blender/blenlib/intern/task.c
+++ b/source/blender/blenlib/intern/task.c
@@ -1042,15 +1042,56 @@ void BLI_task_pool_delayed_push_end(TaskPool *pool, int thread_id)
   if (((_mem) != NULL) && ((_size) > 8192)) \
   MEM_freeN((_mem))
 
-typedef struct ParallelRangeState {
+/* Stores all needed data to perform a parallelized iteration,
+ * with a same operation (callback function).
+ * It can be chained with other tasks in a single-linked list way. */
+typedef struct TaskParallelRangeState {
+  struct TaskParallelRangeState *next;
+
+  /* Start and end point of integer value iteration. */
   int start, stop;
-  void *userdata;
 
+  /* User-defined data, shared between all worker threads. */
+  void *userdata_shared;
+  /* User-defined callback function called for each value in [start, stop[ specified range. */
   TaskParallelRangeFunc func;
 
-  int iter;
+  /* Each instance of looping chunks will get a copy of this data
+   * (similar to OpenMP's firstprivate).
+   */
+  void *initial_tls_memory; /* Pointer to actual user-defined 'tls' data. */
+  size_t tls_data_size;     /* Size of that data.  */
+
+  void *flatten_tls_storage; /* 'tls' copies of initial_tls_memory for each running task. */
+  /* Number of 'tls' copies in the array, i.e. number of worker threads. */
+  size_t num_elements_in_tls_storage;
+
+  /* Function called from calling thread once whole range have been processed. */
+  TaskParallelFinalizeFunc func_finalize;
+
+  /* Current value of the iterator, shared between all threads (atomically updated). */
+  int iter_value;
+  int iter_chunk_num; /* Amount of iterations to process in a single step. */
+} TaskParallelRangeState;
+
+/* Stores all the parallel tasks for a single pool. */
+typedef struct TaskParallelRangePool {
+  /* The workers' task pool. */
+  TaskPool *pool;
+  /* The number of worker tasks we need to create. */
+  int num_tasks;
+  /* The total number of iterations in all the added ranges. */
+  int num_total_iters;
+  /* The size (number of items) processed at once by a worker task. */
   int chunk_size;
-} ParallelRangeState;
+
+  /* Linked list of range tasks to process. */
+  TaskParallelRangeState *parallel_range_states;
+  /* Current range task beeing processed, swapped atomically. */
+  TaskParallelRangeState *current_state;
+  /* Scheduling settings common to all tasks. */
+  TaskParallelSettings *settings;
+} TaskParallelRangePool;
 
 BLI_INLINE void task_parallel_calc_chunk_size(const TaskParallelSettings *settings,
                                               const int tot_items,
@@ -1113,66 +1154,114 @@ BLI_INLINE void task_parallel_calc_chunk_size(const TaskParallelSettings *settin
   }
 }
 
-BLI_INLINE void task_parallel_range_calc_chunk_size(const TaskParallelSettings *settings,
-                                                    const int num_tasks,
-                                                    ParallelRangeState *state)
+BLI_INLINE void task_parallel_range_calc_chunk_size(TaskParallelRangePool *range_pool)
 {
+  int num_iters = 0;
+  int min_num_iters = INT_MAX;
+  for (TaskParallelRangeState *state = range_pool->parallel_range_states; state != NULL;
+       state = state->next) {
+    const int ni = state->stop - state->start;
+    num_iters += ni;
+    if (min_num_iters > ni) {
+      min_num_iters = ni;
+    }
+  }
+  range_pool->num_total_iters = num_iters;
+  /* Note: Passing min_num_iters here instead of num_iters kind of partially breaks the 'static'
+   * scheduling, but pooled range iterator is inherently non-static anyway, so adding a small level
+   * of dynamic scheduling here should be fine. */
   task_parallel_calc_chunk_size(
-      settings, state->stop - state->start, num_tasks, &state->chunk_size);
+      range_pool->settings, min_num_iters, range_pool->num_tasks, &range_pool->chunk_size);
 }
 
-BLI_INLINE bool parallel_range_next_iter_get(ParallelRangeState *__restrict state,
-                                             int *__restrict iter,
-                                             int *__restrict count)
+BLI_INLINE bool parallel_range_next_iter_get(TaskParallelRangePool *__restrict range_pool,
+                                             int *__restrict r_iter,
+                                             int *__restrict r_count,
+                                             TaskParallelRangeState **__restrict r_state)
 {
-  int previter = atomic_fetch_and_add_int32(&state->iter, state->chunk_size);
-
-  *iter = previter;
-  *count = max_ii(0, min_ii(state->chunk_size, state->stop - previter));
+  /* We need an atomic op here as well to fetch the initial state, since some other thread might
+   * have already updated it. */
+  TaskParallelRangeState *current_state = atomic_cas_ptr(
+      (void **)&range_pool->current_state, NULL, NULL);
+
+  int previter = INT32_MAX;
+
+  while (current_state != NULL && previter >= current_state->stop) {
+    previter = atomic_fetch_and_add_int32(&current_state->iter_value, range_pool->chunk_size);
+    *r_iter = previter;
+    *r_count = max_ii(0, min_ii(range_pool->chunk_size, current_state->stop - previter));
+
+    if (previter >= current_state->stop) {
+      /* At this point the state we got is done, we need to go to the next one. In case some other
+       * thread already did it, then this does nothing, and we'll just get current valid state
+       * at start of the next loop. */
+      TaskParallelRangeState *current_state_from_atomic_cas = atomic_cas_ptr(
+          (void **)&range_pool->current_state, current_state, current_state->next);
+
+      if (current_state == current_state_from_atomic_cas) {
+        /* The atomic CAS operation was successful, we did update range_pool->current_state, so we
+         * can safely switch to next state. */
+        current_state = current_state->next;
+      }
+      else {
+        /* The atomic CAS operation failed, but we still got range_pool->current_state value out of
+         * it, just use it as our new current state. */
+        current_state = current_state_from_atomic_cas;
+      }
+    }
+  }
 
-  return (previter < state->stop);
+  *r_state = current_state;
+  return (current_state != NULL && previter < current_state->stop);
 }
 
-static void parallel_range_func(TaskPool *__restrict pool, void *userdata_chunk, int thread_id)
+static void parallel_range_func(TaskPool *__restrict pool, void *tls_data_idx, int thread_id)
 {
-  ParallelRangeState *__restrict state = BLI_task_pool_userdata(pool);
+  TaskParallelRangePool *__restrict range_pool = BLI_task_pool_userdata(pool);
   TaskParallelTLS tls = {
       .thread_id = thread_id,
-      .userdata_chunk = userdata_chunk,
+      .userdata_chunk = NULL,
   };
+  TaskParallelRangeState *state;
   int iter, count;
-  while (parallel_range_next_iter_get(state, &iter, &count)) {
+  while (parallel_range_next_iter_get(range_pool, &iter, &count, &state)) {
+    tls.userdata_chunk = (char *)state->flatten_tls_storage +
+                         (((size_t)POINTER_AS_INT(tls_data_idx)) * state->tls_data_size);
     for (int i = 0; i < count; i++) {
-      state->func(state->userdata, iter + i, &tls);
+      state->func(state->userdata_shared, iter + i, &tls);
     }
   }
 }
 
-static void parallel_range_single_thread(const int start,
-                                         int const stop,
-                                         void *userdata,
-                                         TaskParallelRangeFunc func,
-                                         const TaskParallelSettings *settings)
+static void parallel_range_single_thread(TaskParallelRangePool *range_pool)
 {
-  void *userdata_chunk = settings->userdata_chunk;
-  const size_t userdata_chunk_size = settings->userdata_chunk_size;
-  void *userdata_chunk_local = NULL;
-  const bool use_userdata_chunk = (userdata_chunk_size != 0) && (userdata_chunk != NULL);
-  if (use_userdata_chunk) {
-    userdata_chunk_local = MALLOCA(userdata_chunk_size);
-    memcpy(userdata_chunk_local, userdata_chunk, userdata_chunk_size);
-  }
-  TaskParallelTLS tls = {
-      .thread_id = 0,
-      .userdata_chunk = userdata_chunk_local,
-  };
-  for (int i = start; i < stop; i++) {
-    func(userdata, i, &tls);
-  }
-  if (settings->func_finalize != NULL) {
-    settings->func_finalize(userdata, userdata_chunk_local);
+  for (TaskParallelRangeState *state = range_pool->parallel_range_states; state != NULL;
+       state = state->next) {
+    const in

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list