Cleanup allocator_shim.cc from ENABLE_DYNAMIC_ALLOCATOR_SWITCHING

base/allocator/allocator_shim.cc

Index: base/allocator/allocator_shim.cc
diff --git a/base/allocator/allocator_shim.cc b/base/allocator/allocator_shim.cc
index 1d8229117d77a27035c84a33523b230eea7a3351..7eceeb983615286825099b7f8a441a68011a3a68 100644
--- a/base/allocator/allocator_shim.cc
+++ b/base/allocator/allocator_shim.cc
@@ -6,15 +6,14 @@
 
 #include <config.h>
 #include "base/allocator/allocator_extension_thunks.h"
+#include "base/logging.h"
 #include "base/profiler/alternate_timer.h"
 #include "base/sysinfo.h"
 #include "jemalloc.h"
 
-// When defined, different heap allocators can be used via an environment
-// variable set before running the program.  This may reduce the amount
-// of inlining that we get with malloc/free/etc.  Disabling makes it
-// so that only tcmalloc can be used.
-#define ENABLE_DYNAMIC_ALLOCATOR_SWITCHING
+// This shim make it possible to use different allocators via an environment
+// variable set before running the program. This may reduce the
+// amount of inlining that we get with malloc/free/etc.
 
 // TODO(mbelshe): Ensure that all calls to tcmalloc have the proper call depth
 // from the "user code" so that debugging tools (HeapChecker) can work.
@@ -119,7 +118,6 @@ extern "C" {
 void* malloc(size_t size) __THROW {
   void* ptr;
   for (;;) {
-#ifdef ENABLE_DYNAMIC_ALLOCATOR_SWITCHING
     switch (allocator) {
       case JEMALLOC:
         ptr = je_malloc(size);
@@ -133,10 +131,6 @@ void* malloc(size_t size) __THROW {
         ptr = do_malloc(size);
         break;
     }
-#else
-    // TCMalloc case.
-    ptr = do_malloc(size);
-#endif
     if (ptr)
       return ptr;
 
@@ -147,7 +141,6 @@ void* malloc(size_t size) __THROW {
 }
 
 void free(void* p) __THROW {
-#ifdef ENABLE_DYNAMIC_ALLOCATOR_SWITCHING
   switch (allocator) {
     case JEMALLOC:
       je_free(p);
@@ -156,10 +149,10 @@ void free(void* p) __THROW {
     case WINLFH:
       win_heap_free(p);
       return;
+    case TCMALLOC:
+      do_free(p);
+      return;
   }
-#endif
-  // TCMalloc case.
-  do_free(p);
 }
 
 void* realloc(void* ptr, size_t size) __THROW {
@@ -171,7 +164,6 @@ void* realloc(void* ptr, size_t size) __THROW {
 
   void* new_ptr;
   for (;;) {
-#ifdef ENABLE_DYNAMIC_ALLOCATOR_SWITCHING
     switch (allocator) {
       case JEMALLOC:
         new_ptr = je_realloc(ptr, size);
@@ -185,10 +177,6 @@ void* realloc(void* ptr, size_t size) __THROW {
         new_ptr = do_realloc(ptr, size);
         break;
     }
-#else
-    // TCMalloc case.
-    new_ptr = do_realloc(ptr, size);
-#endif
 
     // Subtle warning:  NULL return does not alwas indicate out-of-memory.  If
     // the requested new size is zero, realloc should free the ptr and return
@@ -203,7 +191,6 @@ void* realloc(void* ptr, size_t size) __THROW {
 
 // TODO(mbelshe): Implement this for other allocators.
 void malloc_stats(void) __THROW {
-#ifdef ENABLE_DYNAMIC_ALLOCATOR_SWITCHING
   switch (allocator) {
     case JEMALLOC:
       // No stats.
@@ -212,24 +199,27 @@ void malloc_stats(void) __THROW {
     case WINLFH:
       // No stats.
       return;
+    case TCMALLOC:
+      tc_malloc_stats();
+      return;
   }
-#endif
-  tc_malloc_stats();
 }
 
 #ifdef WIN32
 
 extern "C" size_t _msize(void* p) {
-#ifdef ENABLE_DYNAMIC_ALLOCATOR_SWITCHING
   switch (allocator) {
     case JEMALLOC:
       return je_msize(p);
     case WINHEAP:
     case WINLFH:
       return win_heap_msize(p);
+    case TCMALLOC:
+      return MallocExtension::instance()->GetAllocatedSize(p);
   }
-#endif
-  return MallocExtension::instance()->GetAllocatedSize(p);
+
+  NOTREACHED();

[jar (doing other things), 2013/11/07 19:22:37]

Although this is a reasonable debug check.... it is a change in semantics.

Worse yet, in general, the allocator code generally avoids using an CHECK()
style code (which can induce allocations... <oops>), and instead uses extremely
simplistics ASSERT() implementations, which crash with no potential allocation
calls.  

If you are finding this inclusion problematic, I'd suggest either:
a) Don't include logging.h.... don't use NOTREACHED

b) Leave the code as it was, using the default to TCMALLOC, and don't worry
about the error where the case is not specified as you expect.

I'd probably argue for the latter, as it would have no chance to impact on
performace.

+  return 0;
 }
 
 // This is included to resolve references from libcmt.
@@ -238,7 +228,6 @@ extern "C" intptr_t _get_heap_handle() {
 }
 
 static bool get_allocator_waste_size_thunk(size_t* size) {
-#ifdef ENABLE_DYNAMIC_ALLOCATOR_SWITCHING
   switch (allocator) {
     case JEMALLOC:
     case WINHEAP:
@@ -246,7 +235,6 @@ static bool get_allocator_waste_size_thunk(size_t* size) {
       // TODO(alexeif): Implement for allocators other than tcmalloc.
       return false;
   }
-#endif
   size_t heap_size, allocated_bytes, unmapped_bytes;
   MallocExtension* ext = MallocExtension::instance();
   if (ext->GetNumericProperty("generic.heap_size", &heap_size) &&
@@ -270,7 +258,6 @@ static void release_free_memory_thunk() {
 
 // The CRT heap initialization stub.
 extern "C" int _heap_init() {
-#ifdef ENABLE_DYNAMIC_ALLOCATOR_SWITCHING
 // Don't use the environment variable if ADDRESS_SANITIZER is defined on
 // Windows, as the implementation requires Winheap to be the allocator.
 #if !(defined(ADDRESS_SANITIZER) && defined(OS_WIN))
@@ -299,7 +286,7 @@ extern "C" int _heap_init() {
       // fall through
       break;
   }
-#endif
+
   // Initializing tcmalloc.
   // We intentionally leak this object.  It lasts for the process
   // lifetime.  Trying to teardown at _heap_term() is so late that
@@ -346,7 +333,6 @@ void* _aligned_malloc(size_t size, size_t alignment) {
 
   void* ptr;
   for (;;) {
-#ifdef ENABLE_DYNAMIC_ALLOCATOR_SWITCHING
     switch (allocator) {
       case JEMALLOC:
         ptr = je_memalign(alignment, size);
@@ -360,10 +346,7 @@ void* _aligned_malloc(size_t size, size_t alignment) {
         ptr = tc_memalign(alignment, size);
         break;
     }
-#else
-    // TCMalloc case.
-    ptr = tc_memalign(alignment, size);
-#endif
+
     if (ptr) {
       // Sanity check alignment.
       DCHECK_EQ(reinterpret_cast<uintptr_t>(ptr) & (alignment - 1), 0U);
@@ -380,7 +363,6 @@ void _aligned_free(void* p) {
   // Both JEMalloc and TCMalloc return pointers from memalign() that are safe to
   // use with free().  Pointers allocated with win_heap_memalign() MUST be freed
   // via win_heap_memalign_free() since the aligned pointer is not the real one.
-#ifdef ENABLE_DYNAMIC_ALLOCATOR_SWITCHING
   switch (allocator) {
     case JEMALLOC:
       je_free(p);
@@ -389,10 +371,9 @@ void _aligned_free(void* p) {
     case WINLFH:
       win_heap_memalign_free(p);
       return;
+    case TCMALLOC:
+      do_free(p);
   }
-#endif
-  // TCMalloc case.
-  do_free(p);
 }
 
 #endif  // WIN32
@@ -405,7 +386,6 @@ namespace base {
 namespace allocator {
 
 void SetupSubprocessAllocator() {
-#ifdef ENABLE_DYNAMIC_ALLOCATOR_SWITCHING
   size_t primary_length = 0;
   getenv_s(&primary_length, NULL, 0, primary_name);
 
@@ -427,7 +407,6 @@ void SetupSubprocessAllocator() {
     int ret_val = _putenv_s(primary_name, secondary_value);
     DCHECK_EQ(0, ret_val);
   }
-#endif  // ENABLE_DYNAMIC_ALLOCATOR_SWITCHING
 }
 
 void* TCMallocDoMallocForTest(size_t size) {