[Bf-blender-cvs] [183ba284f21] master: Cleanup: make guarded memory allocation always thread safe

Brecht Van Lommel noreply at git.blender.org
Wed May 20 01:07:28 CEST 2020


Commit: 183ba284f213903f2208349fe1dd57c07c327ad9
Author: Brecht Van Lommel
Date:   Wed May 20 00:59:41 2020 +0200
Branches: master
https://developer.blender.org/rB183ba284f213903f2208349fe1dd57c07c327ad9

Cleanup: make guarded memory allocation always thread safe

Previously this would be enabled when threads were used, but threads are now
basically always in use so there is no point. Further, this is only needed for
guarded allocation with --debug-memory which is not performance critical.

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

M	intern/guardedalloc/MEM_guardedalloc.h
M	intern/guardedalloc/intern/mallocn.c
M	intern/guardedalloc/intern/mallocn_guarded_impl.c
M	intern/guardedalloc/intern/mallocn_intern.h
M	intern/guardedalloc/intern/mallocn_lockfree_impl.c
M	source/blender/blenkernel/intern/dynamicpaint.c
M	source/blender/blenlib/BLI_threads.h
M	source/blender/blenlib/intern/task_pool.cc
M	source/blender/blenlib/intern/task_range.cc
M	source/blender/blenlib/intern/threads.c
M	source/blender/draw/engines/external/external_engine.c
M	source/blender/editors/render/render_internal.c
M	source/blender/render/extern/include/RE_engine.h
M	source/blender/render/intern/source/external_engine.c
M	source/creator/creator_args.c

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

diff --git a/intern/guardedalloc/MEM_guardedalloc.h b/intern/guardedalloc/MEM_guardedalloc.h
index f4fcebf6811..602297576c8 100644
--- a/intern/guardedalloc/MEM_guardedalloc.h
+++ b/intern/guardedalloc/MEM_guardedalloc.h
@@ -168,10 +168,6 @@ extern void (*MEM_set_error_callback)(void (*func)(const char *));
  * @retval true for correct memory, false for corrupted memory. */
 extern bool (*MEM_consistency_check)(void);
 
-/** Set thread locking functions for safe memory allocation from multiple
- * threads, pass NULL pointers to disable thread locking again. */
-extern void (*MEM_set_lock_callback)(void (*lock)(void), void (*unlock)(void));
-
 /** Attempt to enforce OSX (or other OS's) to have malloc and stack nonzero */
 extern void (*MEM_set_memory_debug)(void);
 
diff --git a/intern/guardedalloc/intern/mallocn.c b/intern/guardedalloc/intern/mallocn.c
index 82a8aa3eb21..e85f8eb03ed 100644
--- a/intern/guardedalloc/intern/mallocn.c
+++ b/intern/guardedalloc/intern/mallocn.c
@@ -54,8 +54,6 @@ void (*MEM_callbackmemlist)(void (*func)(void *)) = MEM_lockfree_callbackmemlist
 void (*MEM_printmemlist_stats)(void) = MEM_lockfree_printmemlist_stats;
 void (*MEM_set_error_callback)(void (*func)(const char *)) = MEM_lockfree_set_error_callback;
 bool (*MEM_consistency_check)(void) = MEM_lockfree_consistency_check;
-void (*MEM_set_lock_callback)(void (*lock)(void),
-                              void (*unlock)(void)) = MEM_lockfree_set_lock_callback;
 void (*MEM_set_memory_debug)(void) = MEM_lockfree_set_memory_debug;
 size_t (*MEM_get_memory_in_use)(void) = MEM_lockfree_get_memory_in_use;
 unsigned int (*MEM_get_memory_blocks_in_use)(void) = MEM_lockfree_get_memory_blocks_in_use;
@@ -115,7 +113,6 @@ void MEM_use_guarded_allocator(void)
   MEM_printmemlist_stats = MEM_guarded_printmemlist_stats;
   MEM_set_error_callback = MEM_guarded_set_error_callback;
   MEM_consistency_check = MEM_guarded_consistency_check;
-  MEM_set_lock_callback = MEM_guarded_set_lock_callback;
   MEM_set_memory_debug = MEM_guarded_set_memory_debug;
   MEM_get_memory_in_use = MEM_guarded_get_memory_in_use;
   MEM_get_memory_blocks_in_use = MEM_guarded_get_memory_blocks_in_use;
diff --git a/intern/guardedalloc/intern/mallocn_guarded_impl.c b/intern/guardedalloc/intern/mallocn_guarded_impl.c
index 8aa9fc767dd..20dcbed7235 100644
--- a/intern/guardedalloc/intern/mallocn_guarded_impl.c
+++ b/intern/guardedalloc/intern/mallocn_guarded_impl.c
@@ -28,6 +28,8 @@
 #include <string.h> /* memcpy */
 #include <sys/types.h>
 
+#include <pthread.h>
+
 #include "MEM_guardedalloc.h"
 
 /* to ensure strict conversions */
@@ -50,17 +52,6 @@
 
 //#define DEBUG_MEMCOUNTER
 
-/* Only for debugging:
- * defining DEBUG_THREADS will enable check whether memory manager
- * is locked with a mutex when allocation is called from non-main
- * thread.
- *
- * This helps troubleshooting memory issues caused by the fact
- * guarded allocator is not thread-safe, however this check will
- * fail to check allocations from openmp threads.
- */
-//#define DEBUG_THREADS
-
 /* Only for debugging:
  * Defining DEBUG_BACKTRACE will store a backtrace from where
  * memory block was allocated and print this trace for all
@@ -124,24 +115,6 @@ typedef struct MemHead {
 
 typedef MemHead MemHeadAligned;
 
-/* for openmp threading asserts, saves time troubleshooting
- * we may need to extend this if blender code starts using MEM_
- * functions inside OpenMP correctly with omp_set_lock() */
-
-#if 0 /* disable for now, only use to debug openmp code which doesn lock threads for malloc */
-#  if defined(_OPENMP) && defined(DEBUG)
-#    include <assert.h>
-#    include <omp.h>
-#    define DEBUG_OMP_MALLOC
-#  endif
-#endif
-
-#ifdef DEBUG_THREADS
-#  include <assert.h>
-#  include <pthread.h>
-static pthread_t mainid;
-#endif
-
 #ifdef DEBUG_BACKTRACE
 #  if defined(__linux__) || defined(__APPLE__)
 #    include <execinfo.h>
@@ -192,8 +165,6 @@ static size_t mem_in_use = 0, peak_mem = 0;
 static volatile struct localListBase _membase;
 static volatile struct localListBase *membase = &_membase;
 static void (*error_callback)(const char *) = NULL;
-static void (*thread_lock_callback)(void) = NULL;
-static void (*thread_unlock_callback)(void) = NULL;
 
 static bool malloc_debug_memset = false;
 
@@ -233,40 +204,16 @@ print_error(const char *str, ...)
     fputs(buf, stderr);
 }
 
+static pthread_mutex_t thread_lock = PTHREAD_MUTEX_INITIALIZER;
+
 static void mem_lock_thread(void)
 {
-#ifdef DEBUG_THREADS
-  static int initialized = 0;
-
-  if (initialized == 0) {
-    /* assume first allocation happens from main thread */
-    mainid = pthread_self();
-    initialized = 1;
-  }
-
-  if (!pthread_equal(pthread_self(), mainid) && thread_lock_callback == NULL) {
-    assert(!"Memory function is called from non-main thread without lock");
-  }
-#endif
-
-#ifdef DEBUG_OMP_MALLOC
-  assert(omp_in_parallel() == 0);
-#endif
-
-  if (thread_lock_callback)
-    thread_lock_callback();
+  pthread_mutex_lock(&thread_lock);
 }
 
 static void mem_unlock_thread(void)
 {
-#ifdef DEBUG_THREADS
-  if (!pthread_equal(pthread_self(), mainid) && thread_lock_callback == NULL) {
-    assert(!"Thread lock was removed while allocation from thread is in progress");
-  }
-#endif
-
-  if (thread_unlock_callback)
-    thread_unlock_callback();
+  pthread_mutex_unlock(&thread_lock);
 }
 
 bool MEM_guarded_consistency_check(void)
@@ -287,12 +234,6 @@ void MEM_guarded_set_error_callback(void (*func)(const char *))
   error_callback = func;
 }
 
-void MEM_guarded_set_lock_callback(void (*lock)(void), void (*unlock)(void))
-{
-  thread_lock_callback = lock;
-  thread_unlock_callback = unlock;
-}
-
 void MEM_guarded_set_memory_debug(void)
 {
   malloc_debug_memset = true;
diff --git a/intern/guardedalloc/intern/mallocn_intern.h b/intern/guardedalloc/intern/mallocn_intern.h
index 6e8c580e0ad..ef8845a66b3 100644
--- a/intern/guardedalloc/intern/mallocn_intern.h
+++ b/intern/guardedalloc/intern/mallocn_intern.h
@@ -139,7 +139,6 @@ void MEM_lockfree_callbackmemlist(void (*func)(void *));
 void MEM_lockfree_printmemlist_stats(void);
 void MEM_lockfree_set_error_callback(void (*func)(const char *));
 bool MEM_lockfree_consistency_check(void);
-void MEM_lockfree_set_lock_callback(void (*lock)(void), void (*unlock)(void));
 void MEM_lockfree_set_memory_debug(void);
 size_t MEM_lockfree_get_memory_in_use(void);
 unsigned int MEM_lockfree_get_memory_blocks_in_use(void);
@@ -183,7 +182,6 @@ void MEM_guarded_callbackmemlist(void (*func)(void *));
 void MEM_guarded_printmemlist_stats(void);
 void MEM_guarded_set_error_callback(void (*func)(const char *));
 bool MEM_guarded_consistency_check(void);
-void MEM_guarded_set_lock_callback(void (*lock)(void), void (*unlock)(void));
 void MEM_guarded_set_memory_debug(void);
 size_t MEM_guarded_get_memory_in_use(void);
 unsigned int MEM_guarded_get_memory_blocks_in_use(void);
diff --git a/intern/guardedalloc/intern/mallocn_lockfree_impl.c b/intern/guardedalloc/intern/mallocn_lockfree_impl.c
index 7b8b405b372..205cc688d72 100644
--- a/intern/guardedalloc/intern/mallocn_lockfree_impl.c
+++ b/intern/guardedalloc/intern/mallocn_lockfree_impl.c
@@ -400,12 +400,6 @@ bool MEM_lockfree_consistency_check(void)
   return true;
 }
 
-void MEM_lockfree_set_lock_callback(void (*lock)(void), void (*unlock)(void))
-{
-  (void)lock;
-  (void)unlock;
-}
-
 void MEM_lockfree_set_memory_debug(void)
 {
   malloc_debug_memset = true;
diff --git a/source/blender/blenkernel/intern/dynamicpaint.c b/source/blender/blenkernel/intern/dynamicpaint.c
index 79d9a40f06b..f3cc17f46f6 100644
--- a/source/blender/blenkernel/intern/dynamicpaint.c
+++ b/source/blender/blenkernel/intern/dynamicpaint.c
@@ -4646,9 +4646,6 @@ static int dynamicPaint_paintParticles(DynamicPaintSurface *surface,
     return 1;
   }
 
-  /* begin thread safe malloc */
-  BLI_threaded_malloc_begin();
-
   /* only continue if particle bb is close enough to canvas bb */
   if (boundsIntersectDist(&grid->grid_bounds, &part_bb, range)) {
     int c_index;
@@ -4684,7 +4681,6 @@ static int dynamicPaint_paintParticles(DynamicPaintSurface *surface,
                               &settings);
     }
   }
-  BLI_threaded_malloc_end();
   BLI_kdtree_3d_free(tree);
 
   return 1;
diff --git a/source/blender/blenlib/BLI_threads.h b/source/blender/blenlib/BLI_threads.h
index c199417017b..03fe27c10ed 100644
--- a/source/blender/blenlib/BLI_threads.h
+++ b/source/blender/blenlib/BLI_threads.h
@@ -58,9 +58,6 @@ void BLI_threadpool_clear(struct ListBase *threadbase);
 void BLI_threadpool_end(struct ListBase *threadbase);
 int BLI_thread_is_main(void);
 
-void BLI_threaded_malloc_begin(void);
-void BLI_threaded_malloc_end(void);
-
 /* System Information */
 
 int BLI_system_thread_count(void); /* gets the number of threads the system can make use of */
diff --git a/source/blender/blenlib/intern/task_pool.cc b/source/blender/blenlib/intern/task_pool.cc
index 670787697a3..cf328ec407c 100644
--- a/source/blender/blenlib/intern/task_pool.cc
+++ b/source/blender/blenlib/intern/task_pool.cc
@@ -364,14 +364,6 @@ static void background_task_pool_free(TaskPool *pool)
 
 static TaskPool *task_pool_create_ex(void *userdata, TaskPoolType type, TaskPriority priority)
 {
-  /* Ensure malloc will go fine from threads,
-   *
-   * This is needed because we could be in main thread here
-   * and malloc could be non-thread safe at this point because
-   * no other jobs are running.
-   */
-  BLI_threaded_malloc_begin();
-
   const bool use_threads = BLI_task_scheduler_num_threads() > 1 && type != TASK_POOL_NO_THREADS;
 
   /* Background task pool uses regular TBB scheduling if available. Only when
@@ -475,8 +467,6 @@ void BLI_task_pool_free(TaskPool *pool)
   BLI_mutex_end(&pool->user_mutex);
 
   MEM_freeN(pool);
-
-  BLI_threaded_malloc_end();
 }
 
 void BLI_task_pool_push(TaskPool *pool,
diff --git a/source/blender/blenlib/intern/task_range.cc b/source/blender/blenlib/intern/task_range.cc
index da38c8fd352..67d8960434e 100644
--- a/source/blender/blenlib/intern/task_range.cc
+++ b/source/blender/blenlib/intern/task_range.cc
@@ -115,8 +115,6 @@ void BLI_task_parallel_range(const int start,
 #ifdef WITH_TBB
   /* Multithreading. */
  

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list