[PATCH] Slightly better way to __USE_MALLOC

Brad Spencer spencer@infointeractive.com
Thu Oct 17 11:27:00 GMT 2002


On Thu, Oct 17, 2002 at 11:16:19AM -0500, Loren James Rittle wrote:
> Hi Brad (and members of the list that care),
> 
> Sorry about the long delay in getting back to you.  Your work (BTW,
> consider this a response to the actual patch you sent last Thursday as
> well) is very helpful even with my comments below.  Actually, if you
> are willing, I'd like you to think more radically (to that end, I have
> attached such a patch to this e-mail, please tell me if you would
> dislike my proposed patch verses your approach).

I think this is an excellent patch!  I've been testing a couple of
slight variations and seeing how it performs against the gcc-3.2
release.  I agree with your criticism of my previous patches.

> Instead of that perspective, let's see how to fix this right.  (BTW,
> as you know, the exact problem with the header is an undetectable
> violation of the one-definition-rule and your current patch just adds
> one extra level of checking about the situation but the root violation
> would still be present.)  Then, we test a correctly partitioned
> version against the current one and see if it matters for real
> application code (the code in testsuite/thread/pthread[1-6].cc
> stresses the memory allocators; or may be configured to do so, if you
> don't believe that they do for your machine).

Indeed, I work in an exclusively single-threaded universe, but clearly
not everybody does.  "Optimal" performance for everybody would be
best in my mind, too.

> I would like to see the __USE_MALLOC macro-enabled path completely
> gone not renamed.  Although users could get it, that path was never

This line of reasoning sounds good to me, BTW.

> I hope this late feedback and alternate patch is helpful without
> making you in any way dejected.

Not at all!  You and others have been encouraging and supportive.  I
didn't expect to rewrite stl_alloc.h properly on my first try ;)

> Attached is the patch I would want to apply to mainline to address
> this issue.  My patch shows 4 additions to the ABI: the new statics
> and their guards.  However, I don't believe that they would need to be
> exposed for correct operation.  Benjamin, Phil, little help on that
> detail (it seems we currently unconditionally globalize all such
> statics and their guards)?  Of course, I'm willing to negotiate on any
> part of what I did here, e.g. I could add back in the typedef alias
> for __mem_interface since it was in the header ABI (but not the binary
> library) and perhaps for small embedded targets we really do need to
> merge in something like Brad's configuration-time style as well (but I
> have the reservations listed above).

Presumably this would require the symbol mismatching I included.
Would these extra statics per object themselves be problematic for
embedded targets?

> Rebuilt libstdc++-v3 from scratch on alpha-unknown-freebsd4.7 and
> i386-unknown-freebsd4.7.  No regressions.  Also ran `gmake check' with
> GLIBCPP_FORCE_NEW=1 in environment.

I will run this myself when I can.  I wrote a simple loop that
allocated and freed and timed that just to get a measure of the
performance of the allocation decision logic.

#include <memory>

int
main()
{
  std::allocator<char> alloc;
  
  for(unsigned int i = 0; i < 10000000; ++i) {
    char *memory = alloc.allocate(64);
    alloc.deallocate(memory, 64);
  }

  return 0;
}

Here are some timings when compiling with -O6 and running with and
without GLIBCPP_FORCE_NEW.  Time quoted are user time from "time
./main". 

                       Normal       With GLIBCPP_FORCE_NEW=1
gcc-3.2             :  0m0.250s     n/a
cvs gcc             :  0m0.260s     n/a
cvs gcc w/your patch:  0m0.320s     0m1.470s
cvs gcc w/your patch
  using class static:  0m0.270s     0m1.730s
  [see my first comment below]

> Comments?

By checking the environment variable in both allocate() and
deallocate() (for any given instance of the class template), are we
setting up for someone to change it in between the first calls to
those methods?  Now, granted, that would be evil, but perhaps making
it a class static constant is better?  Given the hint that this is a
performance boost, too, perhaps it's worthwhile.  I've attached a
cumulative patch for stl_alloc.h that includes your changes but makes
this change.  

I don't understand why the class-static version so greatly affects the
GLIBCPP_FORCE_NEW time.  Perhaps the test routine is simply too naive.

For the application to select the allocation scheme explicitly at
runtime, it would have to ensure it called putenv() before any use of
std::allocator.  Is this possible?  I'm not certain it's a desirable
feature, but perhaps someone would want to release a certain
application that performs better one way or the other and they want to
hardwire it that way.  This wouldn't be a property of the target as
much as it is of the application itself?

Regardless, this is going to be a joy to use! :)

-- 
------------------------------------------------------------------
Brad Spencer - spencer@infointeractive.com - "It's quite nice..."
Systems Architect | InfoInterActive Corp. | A Canadian AOL Company
-------------- next part --------------
Index: include/bits/stl_alloc.h
===================================================================
RCS file: /cvsroot/gcc/gcc/libstdc++-v3/include/bits/stl_alloc.h,v
retrieving revision 1.24
diff -u -p -r1.24 stl_alloc.h
--- include/bits/stl_alloc.h	23 Aug 2002 16:52:29 -0000	1.24
+++ include/bits/stl_alloc.h	17 Oct 2002 18:14:00 -0000
@@ -210,15 +210,6 @@ namespace std
     }
 #endif
 
-
-  // Determines the underlying allocator choice for the node allocator.
-#ifdef __USE_MALLOC
-  typedef __malloc_alloc_template<0>  __mem_interface;
-#else
-  typedef __new_alloc                 __mem_interface;
-#endif
-
-
   /**
    *  @if maint
    *  This is used primarily (only?) in _Alloc_traits and other places to
@@ -307,23 +298,17 @@ namespace std
     };
 
 
-#ifdef __USE_MALLOC
-
-  typedef __mem_interface __alloc;
-  typedef __mem_interface __single_client_alloc;
-
-#else
-
-
   /**
    *  @if maint
-   *  Default node allocator.  "SGI" style.  Uses __mem_interface for its
-   *  underlying requests (and makes as few requests as possible).
-   *  **** Currently __mem_interface is always __new_alloc, never __malloc*.
+   *  Default node allocator.  "SGI" style.  Uses various allocators to
+   *  fulfill underlying requests (and makes as few requests as possible
+   *  when in default high-speed pool mode).
    *
    *  Important implementation properties:
+   *  0. If globally mandated (and supportable by the port), then
+   *     allocate objects from __malloc_alloc_template<0>
    *  1. If the clients request an object of size > _MAX_BYTES, the resulting
-   *     object will be obtained directly from the underlying __mem_interface.
+   *     object will be obtained directly from __new_alloc
    *  2. In all other cases, we allocate an object of size exactly
    *     _S_round_up(requested_size).  Thus the client has enough size
    *     information that we can return the object to the proper free list
@@ -394,55 +379,19 @@ namespace std
       } __attribute__ ((__unused__));
       friend struct _Lock;
 
+      // Set at startup based on the setting of the GLIBCPP_FORCE_NEW
+      // environment variable, this determines if all allocation is forced to
+      // use new and delete directly.
+      static const bool _S_force_new;
+      
     public:
       // __n must be > 0
       static void*
-      allocate(size_t __n)
-      {
-        void* __ret = 0;
-
-        if (__n > (size_t) _MAX_BYTES)
-          __ret = __mem_interface::allocate(__n);
-        else
-          {
-            _Obj* volatile* __my_free_list = _S_free_list
-                                             + _S_freelist_index(__n);
-            // Acquire the lock here with a constructor call.  This
-            // ensures that it is released in exit or during stack
-            // unwinding.
-            _Lock __lock_instance;
-            _Obj* __restrict__ __result = *__my_free_list;
-            if (__result == 0)
-              __ret = _S_refill(_S_round_up(__n));
-            else
-              {
-                *__my_free_list = __result -> _M_free_list_link;
-                __ret = __result;
-              }
-          }
-        return __ret;
-      };
+      allocate(size_t __n);
 
       // __p may not be 0
       static void
-      deallocate(void* __p, size_t __n)
-      {
-        if (__n > (size_t) _MAX_BYTES)
-          __mem_interface::deallocate(__p, __n);
-        else
-          {
-            _Obj* volatile*  __my_free_list = _S_free_list
-              + _S_freelist_index(__n);
-            _Obj* __q = (_Obj*)__p;
-
-            // Acquire the lock here with a constructor call.  This
-            // ensures that it is released in exit or during stack
-            // unwinding.
-            _Lock __lock_instance;
-            __q -> _M_free_list_link = *__my_free_list;
-            *__my_free_list = __q;
-          }
-      }
+      deallocate(void* __p, size_t __n);
 
 #ifdef _GLIBCPP_DEPRECATED
       static void*
@@ -450,6 +399,52 @@ namespace std
 #endif
     };
 
+  template<bool __threads, int __inst> void*
+  __default_alloc_template<__threads, __inst>::allocate(size_t __n)
+  {
+    void* __ret = 0;
+
+    if ((__n > (size_t) _MAX_BYTES) || _S_force_new)
+      __ret = __new_alloc::allocate(__n);
+    else
+      {
+	_Obj* volatile* __my_free_list = _S_free_list
+	  + _S_freelist_index(__n);
+	// Acquire the lock here with a constructor call.  This
+	// ensures that it is released in exit or during stack
+	// unwinding.
+	_Lock __lock_instance;
+	_Obj* __restrict__ __result = *__my_free_list;
+	if (__result == 0)
+	  __ret = _S_refill(_S_round_up(__n));
+	else
+	  {
+	    *__my_free_list = __result -> _M_free_list_link;
+	    __ret = __result;
+	  }
+      }
+    return __ret;
+  }
+
+  template<bool __threads, int __inst> void
+  __default_alloc_template<__threads, __inst>::deallocate(void* __p, size_t __n)
+  {
+    if ((__n > (size_t) _MAX_BYTES) || _S_force_new)
+      __new_alloc::deallocate(__p, __n);
+    else
+      {
+	_Obj* volatile*  __my_free_list = _S_free_list
+	  + _S_freelist_index(__n);
+	_Obj* __q = (_Obj*)__p;
+
+	// Acquire the lock here with a constructor call.  This
+	// ensures that it is released in exit or during stack
+	// unwinding.
+	_Lock __lock_instance;
+	__q -> _M_free_list_link = *__my_free_list;
+	*__my_free_list = __q;
+      }
+  }
 
   template<bool __threads, int __inst>
     inline bool
@@ -465,8 +460,8 @@ namespace std
 
 
   // We allocate memory in large chunks in order to avoid fragmenting the
-  // malloc heap (or whatever __mem_interface is using) too much.  We assume
-  // that __size is properly aligned.  We hold the allocation lock.
+  // heap too much.  We assume that __size is properly aligned.  We hold
+  // the allocation lock.
   template<bool __threads, int __inst>
     char*
     __default_alloc_template<__threads, __inst>::
@@ -503,7 +498,7 @@ namespace std
               ((_Obj*)_S_start_free) -> _M_free_list_link = *__my_free_list;
               *__my_free_list = (_Obj*)_S_start_free;
             }
-          _S_start_free = (char*) __mem_interface::allocate(__bytes_to_get);
+          _S_start_free = (char*) __new_alloc::allocate(__bytes_to_get);
           if (0 == _S_start_free)
             {
               size_t __i;
@@ -528,7 +523,7 @@ namespace std
                     }
                 }
               _S_end_free = 0;        // In case of exception.
-              _S_start_free = (char*)__mem_interface::allocate(__bytes_to_get);
+              _S_start_free = (char*)__new_alloc::allocate(__bytes_to_get);
               // This should either throw an exception or remedy the situation.
               // Thus we assume it succeeded.
             }
@@ -616,9 +611,12 @@ namespace std
     typename __default_alloc_template<__threads,__inst>::_Obj* volatile
     __default_alloc_template<__threads,__inst>::_S_free_list[_NFREELISTS];
 
+  template<bool __threads, int __inst>
+    const bool __default_alloc_template<__threads,__inst>::_S_force_new =
+    getenv("GLIBCPP_FORCE_NEW");
+
   typedef __default_alloc_template<true,0>    __alloc;
   typedef __default_alloc_template<false,0>   __single_client_alloc;
-#endif /* ! __USE_MALLOC */
 
 
   /**
@@ -628,10 +626,6 @@ namespace std
    *  of stl_alloc.h.)
    *
    *  The underlying allocator behaves as follows.
-   *  - if __USE_MALLOC then
-   *    - thread safety depends on malloc and is entirely out of our hands
-   *    - __malloc_alloc_template is used for memory requests
-   *  - else (the default)
    *    - __default_alloc_template is used via two typedefs
    *    - "__single_client_alloc" typedef does no locking for threads
    *    - "__alloc" typedef is threadsafe via the locks
@@ -908,7 +902,6 @@ namespace std
       typedef __allocator<_Tp, __malloc_alloc_template<__inst> > allocator_type;
     };
 
-#ifndef __USE_MALLOC
   template<typename _Tp, bool __threads, int __inst>
     struct _Alloc_traits<_Tp, __default_alloc_template<__threads, __inst> >
     {
@@ -918,7 +911,6 @@ namespace std
       typedef __allocator<_Tp, __default_alloc_template<__threads, __inst> >
       allocator_type;
     };
-#endif
 
   template<typename _Tp, typename _Alloc>
     struct _Alloc_traits<_Tp, __debug_alloc<_Alloc> >
@@ -941,7 +933,6 @@ namespace std
       typedef __allocator<_Tp, __malloc_alloc_template<__inst> > allocator_type;
     };
 
-#ifndef __USE_MALLOC
   template<typename _Tp, typename _Tp1, bool __thr, int __inst>
     struct _Alloc_traits<_Tp, __allocator<_Tp1, __default_alloc_template<__thr, __inst> > >
     {
@@ -951,7 +942,6 @@ namespace std
       typedef __allocator<_Tp, __default_alloc_template<__thr,__inst> >
       allocator_type;
     };
-#endif
 
   template<typename _Tp, typename _Tp1, typename _Alloc>
     struct _Alloc_traits<_Tp, __allocator<_Tp1, __debug_alloc<_Alloc> > >
@@ -967,11 +957,7 @@ namespace std
   // NB: This syntax is a GNU extension.
   extern template class allocator<char>;
   extern template class allocator<wchar_t>;
-#ifdef __USE_MALLOC
-  extern template class __malloc_alloc_template<0>;
-#else
   extern template class __default_alloc_template<true,0>;
-#endif
 } // namespace std
 
 #endif


More information about the Libstdc++ mailing list