Improve the performance of Boost.Function's swap. Thanks to Niels Dekker for the original patch. Fixes #1910

[SVN r48615]

doc/history.xml

@@ -13,6 +13,14 @@
 
 <itemizedlist spacing="compact">
 
+  <listitem><para><bold>Version 1.37.0</bold>: </para>
+    <itemizedlist spacing="compact">
+      <listitem><para>Improved the performance of Boost.Function's
+      swap() operation for large function objects. Original patch
+      contributed by Niels Dekker.</para></listitem>
+    </itemizedlist>
+  </listitem>
+
   <listitem><para><bold>Version 1.36.0</bold>: </para>
     <itemizedlist spacing="compact">
       <listitem><para>Boost.Function now implements allocator support

include/boost/function/function_base.hpp

@@ -92,7 +92,7 @@ namespace boost {
       union function_buffer
       {
         // For pointers to function objects
-        void* obj_ptr;
+        mutable void* obj_ptr;
 
         // For pointers to std::type_info objects
         // (get_functor_type_tag, check_functor_type_tag).
@@ -138,6 +138,7 @@ namespace boost {
       // The operation type to perform on the given functor/function pointer
       enum functor_manager_operation_type {
         clone_functor_tag,
+        move_functor_tag,
         destroy_functor_tag,
         check_functor_type_tag,
         get_functor_type_tag
@@ -182,6 +183,11 @@ namespace boost {
             out_buffer.obj_ptr = in_buffer.obj_ptr;
             return;
 
+          case move_functor_tag:
+            out_buffer.obj_ptr = in_buffer.obj_ptr;
+            in_buffer.obj_ptr = 0;
+            return;
+
           case destroy_functor_tag:
             out_buffer.obj_ptr = 0;
             return;
@@ -247,7 +253,10 @@ namespace boost {
         {
           if (op == clone_functor_tag)
             out_buffer.func_ptr = in_buffer.func_ptr;
-          else if (op == destroy_functor_tag)
+          else if (op == move_functor_tag) {
+            out_buffer.func_ptr = in_buffer.func_ptr;
+            in_buffer.func_ptr = 0;
+          } else if (op == destroy_functor_tag)
             out_buffer.func_ptr = 0;
           else /* op == check_functor_type_tag */ {
             const BOOST_FUNCTION_STD_NS::type_info& check_type = 
@@ -264,10 +273,14 @@ namespace boost {
         manage_small(const function_buffer& in_buffer, function_buffer& out_buffer, 
                 functor_manager_operation_type op)
         {
-          if (op == clone_functor_tag) {
+          if (op == clone_functor_tag || op == move_functor_tag) {
             const functor_type* in_functor = 
               reinterpret_cast<const functor_type*>(&in_buffer.data);
             new ((void*)&out_buffer.data) functor_type(*in_functor);
+
+            if (op == move_functor_tag) {
+              reinterpret_cast<functor_type*>(&in_buffer.data)->~Functor();
+            }
           } else if (op == destroy_functor_tag) {
             // Some compilers (Borland, vc6, ...) are unhappy with ~functor_type.
             reinterpret_cast<functor_type*>(&out_buffer.data)->~Functor();
@@ -317,6 +330,9 @@ namespace boost {
               (const functor_type*)(in_buffer.obj_ptr);
             functor_type* new_f = new functor_type(*f);
             out_buffer.obj_ptr = new_f;
+          } else if (op == move_functor_tag) {
+            out_buffer.obj_ptr = in_buffer.obj_ptr;
+            in_buffer.obj_ptr = 0;
           } else if (op == destroy_functor_tag) {
             /* Cast from the void pointer to the functor pointer type */
             functor_type* f =
@@ -409,6 +425,9 @@ namespace boost {
             // Get back to the original pointer type
             functor_wrapper_type* new_f = static_cast<functor_wrapper_type*>(copy);
             out_buffer.obj_ptr = new_f;
+          } else if (op == move_functor_tag) {
+            out_buffer.obj_ptr = in_buffer.obj_ptr;
+            in_buffer.obj_ptr = 0;
           } else if (op == destroy_functor_tag) {
             /* Cast from the void pointer to the functor_wrapper_type */
             functor_wrapper_type* victim =

include/boost/function/function_fwd.hpp

@@ -26,6 +26,8 @@ namespace boost { namespace python { namespace objects {
 #endif
 
 namespace boost {
+  class bad_function_call;
+
 #if !defined(BOOST_FUNCTION_NO_FUNCTION_TYPE_SYNTAX)
   // Preferred syntax
   template<typename Signature> class function;

include/boost/function/function_template.hpp

@@ -729,9 +729,10 @@ namespace boost {
       if (&other == this)
         return;
 
-      BOOST_FUNCTION_FUNCTION tmp = *this;
+      BOOST_FUNCTION_FUNCTION tmp;
-      *this = other;
+      tmp.move_assign(*this);
-      other = tmp;
+      this->move_assign(other);
+      other.move_assign(tmp);
     }
 
     // Clear out a target, if there is one
@@ -786,6 +787,32 @@ namespace boost {
       if (stored_vtable.assign_to_a(f, functor, a)) vtable = &stored_vtable;
       else vtable = 0;
     }
+
+    // Moves the value from the specified argument to *this. If the argument 
+    // has its function object allocated on the heap, move_assign will pass 
+    // its buffer to *this, and set the argument's buffer pointer to NULL. 
+    void move_assign(BOOST_FUNCTION_FUNCTION& f) 
+    { 
+      if (&f == this)
+        return;
+
+#if !defined(BOOST_NO_EXCEPTIONS)      
+      try {
+#endif
+        if (!f.empty()) {
+          this->vtable = f.vtable;
+          f.vtable->manager(f.functor, this->functor,
+                            boost::detail::function::move_functor_tag);
+#if !defined(BOOST_NO_EXCEPTIONS)      
+        } else {
+          clear();
+        }
+      } catch (...) {
+        vtable = 0;
+        throw;
+      }
+#endif
+    }
   };
 
   template<typename R BOOST_FUNCTION_COMMA BOOST_FUNCTION_TEMPLATE_PARMS>

test/Jamfile.v2

@@ -58,6 +58,7 @@ import testing ;
    
   [ run libs/function/test/contains2_test.cpp : : : : ]
    
+  [ run libs/function/test/nothrow_swap.cpp :  :  :  :  ]
  ;
 }
       

test/nothrow_swap.cpp

@@ -0,0 +1,51 @@
+#include <boost/test/minimal.hpp>
+#include <boost/function.hpp>
+
+struct tried_to_copy { };
+
+struct MaybeThrowOnCopy {
+  MaybeThrowOnCopy(int value = 0) : value(value) { }
+
+  MaybeThrowOnCopy(const MaybeThrowOnCopy& other) : value(other.value) {
+    if (throwOnCopy)
+      throw tried_to_copy();
+  }
+
+  MaybeThrowOnCopy& operator=(const MaybeThrowOnCopy& other) {
+    if (throwOnCopy)
+      throw tried_to_copy();
+    value = other.value;
+    return *this;
+  }
+
+  int operator()() { return value; }
+
+  int value;
+
+  // Make sure that this function object doesn't trigger the
+  // small-object optimization in Function.
+  float padding[100];
+
+  static bool throwOnCopy;
+};
+
+bool MaybeThrowOnCopy::throwOnCopy = false;
+
+int test_main(int, char* [])
+{
+  boost::function0<int> f;
+  boost::function0<int> g;
+
+  MaybeThrowOnCopy::throwOnCopy = false;
+  f = MaybeThrowOnCopy(1);
+  g = MaybeThrowOnCopy(2);
+  BOOST_CHECK(f() == 1);
+  BOOST_CHECK(g() == 2);
+
+  MaybeThrowOnCopy::throwOnCopy = true;
+  f.swap(g);
+  BOOST_CHECK(f() == 2);
+  BOOST_CHECK(g() == 1);
+  
+  return 0;
+}