[Bf-blender-cvs] [3f87ac36848] master: Revert "BLI_task: Add pooled threaded index range iterator."

Bastien Montagne noreply at git.blender.org
Mon Nov 25 19:54:54 CET 2019


Commit: 3f87ac368483f0dcf86a1e3057ce8a6fbbe702ac
Author: Bastien Montagne
Date:   Mon Nov 25 19:53:17 2019 +0100
Branches: master
https://developer.blender.org/rB3f87ac368483f0dcf86a1e3057ce8a6fbbe702ac

Revert "BLI_task: Add pooled threaded index range iterator."

This reverts commit f9028a3be1f77c01edca44a68894e2ba9d9cfb14.

This is giving weird heisenbug crash on only Windows release builds...
Reverting until we understand to issue.

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

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 05c3d43a0de..7ef5e518cc8 100644
--- a/source/blender/blenlib/BLI_task.h
+++ b/source/blender/blenlib/BLI_task.h
@@ -196,22 +196,9 @@ void BLI_task_parallel_range(const int start,
                              const int stop,
                              void *userdata,
                              TaskParallelRangeFunc func,
-                             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.
- */
+                             const TaskParallelSettings *settings);
+
+/* 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 e926cbb18bc..0613b558aec 100644
--- a/source/blender/blenlib/intern/task.c
+++ b/source/blender/blenlib/intern/task.c
@@ -1042,56 +1042,15 @@ void BLI_task_pool_delayed_push_end(TaskPool *pool, int thread_id)
   if (((_mem) != NULL) && ((_size) > 8192)) \
   MEM_freeN((_mem))
 
-/* 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. */
+typedef struct ParallelRangeState {
   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;
 
-  /* 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 iter;
   int chunk_size;
-
-  /* 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;
+} ParallelRangeState;
 
 BLI_INLINE void task_parallel_calc_chunk_size(const TaskParallelSettings *settings,
                                               const int tot_items,
@@ -1154,102 +1113,66 @@ BLI_INLINE void task_parallel_calc_chunk_size(const TaskParallelSettings *settin
   }
 }
 
-BLI_INLINE void task_parallel_range_calc_chunk_size(TaskParallelRangePool *range_pool)
+BLI_INLINE void task_parallel_range_calc_chunk_size(const TaskParallelSettings *settings,
+                                                    const int num_tasks,
+                                                    ParallelRangeState *state)
 {
-  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(
-      range_pool->settings, min_num_iters, range_pool->num_tasks, &range_pool->chunk_size);
+      settings, state->stop - state->start, num_tasks, &state->chunk_size);
 }
 
-BLI_INLINE bool parallel_range_next_iter_get(TaskParallelRangePool *__restrict range_pool,
-                                             int *__restrict r_iter,
-                                             int *__restrict r_count,
-                                             TaskParallelRangeState **__restrict r_state)
+BLI_INLINE bool parallel_range_next_iter_get(ParallelRangeState *__restrict state,
+                                             int *__restrict iter,
+                                             int *__restrict count)
 {
-  TaskParallelRangeState *state;
-  int previter = INT32_MAX;
+  int previter = atomic_fetch_and_add_int32(&state->iter, state->chunk_size);
 
-  do {
-    if ((state = range_pool->current_state) == NULL) {
-      break;
-    }
+  *iter = previter;
+  *count = max_ii(0, min_ii(state->chunk_size, state->stop - previter));
 
-    previter = atomic_fetch_and_add_int32(&state->iter_value, range_pool->chunk_size);
-    *r_iter = previter;
-    *r_count = max_ii(0, min_ii(range_pool->chunk_size, state->stop - previter));
-
-    if (previter >= 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. */
-      atomic_cas_ptr((void **)&range_pool->current_state, state, state->next);
-    }
-  } while (state != NULL && previter >= state->stop);
-
-  *r_state = state;
-  return (state != NULL && previter < state->stop);
+  return (previter < state->stop);
 }
 
-static void parallel_range_func(TaskPool *__restrict pool, void *tls_data_idx, int thread_id)
+static void parallel_range_func(TaskPool *__restrict pool, void *userdata_chunk, int thread_id)
 {
-  TaskParallelRangePool *__restrict range_pool = BLI_task_pool_userdata(pool);
+  ParallelRangeState *__restrict state = BLI_task_pool_userdata(pool);
   TaskParallelTLS tls = {
       .thread_id = thread_id,
-      .userdata_chunk = NULL,
+      .userdata_chunk = userdata_chunk,
   };
-  TaskParallelRangeState *state;
   int 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);
+  while (parallel_range_next_iter_get(state, &iter, &count)) {
     for (int i = 0; i < count; i++) {
-      state->func(state->userdata_shared, iter + i, &tls);
+      state->func(state->userdata, iter + i, &tls);
     }
   }
 }
 
-static void parallel_range_single_thread(TaskParallelRangePool *range_pool)
+static void parallel_range_single_thread(const int start,
+                                         int const stop,
+                                         void *userdata,
+                                         TaskParallelRangeFunc func,
+                                         const TaskParallelSettings *settings)
 {
-  for (TaskParallelRangeState *state = range_pool->parallel_range_states; state != NULL;
-       state = state->next) {
-    const int start = state->start;
-    const int stop = state->stop;
-    void *userdata = state->userdata_shared;
-    TaskParallelRangeFunc func = state->func;
-
-    void *initial_tls_memory = state->initial_tls_memory;
-    const size_t tls_data_size = state->tls_data_size;
-    void *flatten_tls_storage = NULL;
-    const bool use_tls_data = (tls_data_size != 0) && (initial_tls_memory != NULL);
-    if (use_tls_data) {
-      flatten_tls_storage = MALLOCA(tls_data_size);
-      memcpy(flatten_tls_storage, initial_tls_memory, tls_data_size);
-    }
-    TaskParallelTLS tls = {
-        .thread_id = 0,
-        .userdata_chunk = flatten_tls_storage,
-    };
-    for (int i = start; i < stop; i++) {
-      func(userdata, i, &tls);
-    }
-    if (state->func_finalize != NULL) {
-      state->func_finalize(userdata, flatten_tls_storage);
-    }
-    MALLOCA_FREE(flatten_tls_storage, tls_data_size);
+  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,

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list