Question

The following callback class is a generic wrapper to "callable things". I really like its API, which has no templates and is very clean, but under the hood there is some dynamic allocation which I was not able to avoid.

Is there any way to get rid of the new and delete in the code below while maintaining the semantics and API of the callback class? I really wish I could.


Needed stuff:

// base class for something we can "call"
class callable {
  public:
  virtual void operator()() = 0;
  virtual ~callable() {}
};

// wraps pointer-to-members
template<class C>
class callable_from_object : public callable {
  public:
  callable_from_object(C& object, void (C::*method)())
         : o(object), m(method) {}

  void operator()() {
    (&o ->* m) ();
  }
  private:
  C& o;
  void (C::*m)();
};

// wraps pointer-to-functions or pointer-to-static-members
class callable_from_function : public callable {
   public:
   callable_from_function(void (*function)())
         : f(function) {}

   void operator()() {
      f();
   };
   private:
   void (*f)();
};

The callback class:

// generic wrapper for any callable
// this is the only class which is exposed to the user
class callback : public callable {
   public:
   template<class C>
   callback(C& object, void (C::*method)())
         : c(*new callable_from_object<C>(object, method)) {}
   explicit callback(void (*function)())
         : c(*new callable_from_function(function)) {}
   void operator()() {
      c();
   }
   ~callback() {
      std::cout << "dtor\n"; // check for mem leak
      delete &c;
   }
   private:
   callable& c;
};

An API example:

struct X {
  void y() { std::cout << "y\n"; }
  static void z() { std::cout << "z\n"; }
} x;

void w() { std::cout << "w\n"; }

int main(int, char*[]) {
   callback c1(x, &X::y);
   callback c2(X::z);
   callback c3(w);
   c1();
   c2();
   c3();
   return 0;
}

Thanks a lot !! :-)

Was it helpful?

Solution 2

YAY!!!

My best solution, using no templates, no dynamic allocation, no inheritance (just as union):

#include <iostream>
#include <stdexcept>

class callback {

   public:

   callback() :
         type(not_a_callback) {}

   template<class C>
   callback(C& object, void (C::*method)()) :
         type(from_object),
         object_ptr(reinterpret_cast<generic*>(&object)),
         method_ptr(reinterpret_cast<void (generic::*) ()>(method)) {}

   template<typename T>
   explicit callback(T function) :
         type(from_function),
         function_ptr((void (*)()) function) {}

   void operator()() {
      switch(type) {
         case from_object:
            (object_ptr ->* method_ptr) ();
            break;
         case from_function:
            function_ptr();
            break;
         default:
            throw std::runtime_error("invalid callback");
      };
   }

   private:

   enum { not_a_callback, from_object, from_function } type;

   class generic;

   union {
      void (*function_ptr)();
      struct {
         generic* object_ptr;
         void (generic::*method_ptr)();
      };
   };

};

Ok, its a big ugly, but it is FAST. For 20 million iterations, Boost version takes 11.8s, mine using dynamic alloc takes 9.8s and this using union takes 4.2s. And it is 60% smaller than mine using dynamic alloc, and 130% smaller than boost.

Edit: updated default constructor.

OTHER TIPS

You can use placement new. Like, set a maximal limit of size you want to allow callback to have, like 16 bytes. Then, put an unsigned char buffer into your callback class that's exactly that wide, and make sure it's aligned correctly (GCC has an attribute for that, and if you are lucky, microsoft has an attribute for that too).

You may also go fairly safe if you use an union, and beside the char buffer put dummies of the types you want to stuff into it - that will ensure proper alignment too.

Then, instead of using normal new, use placement new, like

if(placement_allocated< callable_from_object<C> >::value) {
  new ((void*)buffer.p) // union member p is the unsigned char buffer
    callable_from_object<C>(object, method);
  c = (callable*)buffer.p;
} else {
  c = new callable_from_object<C>(object, method);
}

Then, make the c member a pointer instead. You also need to set a flag so you remember whether you have to call delete in the destructor, or leave the placement buffer alone with explicitly calling the destructor.

That's basically how boost::function does it. It, however, does a whole lot of other stuff to optimize allocation. It uses its own vtable mechanism to optimize space, and of course is very well tested.

Of course, this is not exactly easy to do. But that seems to be the only thing to do about it.

In your example you could remove the new and delete by removing the callback class. This is just a Decorator on callable_from_object and callable_from_object that provides some syntactic sugar: automatically choosing the correct callable object to delegate to.

However this sugar is nice and you will probably want to keep it. Also you will probably have to place the objects other callable objects on the heap anyway.

To me the bigger question is why are you creating a callback library? If it is just to practice c++ then that is fine, but there are loads of exaples of these already:

Why not use one these?

From looking at your example if you keep on the path you are going your solution will converge towards boost::function without its flexibility. So why not use it? While I don't beleive that the boost developers are Gods they are very tallented engineers with an excellent peer review process that results in very stong libraries. I don't think most individuals or organisations can reinvent better libraries.

If your concern is excessive memeory allocation and deallocation the solution is this case would probably be a custom allocator for the various callable subtypes. But again I prefer to allow other people to research these tecniques and use their libraries.

Using boost::function and boost:bind.

typedef boost::function<void ()> callback;

int main(int, char*[]) {
   callback d1 = boost::bind(&X::y, &x);
   callback d2 = &X::z;
   callback d3 = w;
   d1();
   d2();
   d3();
   return 0;
}

Yeah it works, but it is 20% slower than my implementation (with dynamic allocation) and code is 80% bigger.

PS. I repeated the main code 20 million times. Boost version takes 11.8s, mine takes 9.8s.

Edit:

See this

you wont be able to get rid of new delete as your doing polymorphism and thus it will end up trying to copy the child class into a parent class and loose the child class functionality.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top