Re: [Boost-users] [signals2][review] The review of the signals2 library (formerly thread_safe_signals) begins today, Nov 1st

19 Nov 2008

      Hi,

I don't have a review...
I have made a few tests with the library. I think the library is good, 
but I don't know if I make something wrong, but it seems for me that the 
library is slow. I have made a few tests with VC++ 8.0 and _SCL_SECURE=0 
and it seems that it is about 50 times slower than a normal 
boost::function with a bind.... It's a lot...

Best regards
Hansjörg

The test code was:

class PerformanceCounter
{
private:
	LARGE_INTEGER CurrentTickCount_;
	LARGE_INTEGER OldTickCount_;
	LARGE_INTEGER TickFrquency_;
	double EllapsedTime_;
	double TotalTime_;
public:
	PerformanceCounter():
	EllapsedTime_(0.0),
	TotalTime_(0.0)
	{
		::QueryPerformanceFrequency(&TickFrquency_);
		::QueryPerformanceCounter(&CurrentTickCount_);
		OldTickCount_ = CurrentTickCount_;
	}

	void Reset(const double intial_time = 0.0)
	{
		TotalTime_ = intial_time;
		EllapsedTime_ = intial_time;
		::QueryPerformanceCounter(&CurrentTickCount_);
		OldTickCount_ = CurrentTickCount_;
	}
	void CalculateEllapsedTime()
	{
		::QueryPerformanceCounter(&CurrentTickCount_);
		EllapsedTime_ = (double)(CurrentTickCount_.QuadPart - 
OldTickCount_.QuadPart)/TickFrquency_.QuadPart;
		TotalTime_ += EllapsedTime_;
		OldTickCount_ = CurrentTickCount_;
	}
	void CalculateEllapsedTime(const PerformanceCounter& to_intialize)
	{
		CurrentTickCount_ = to_intialize.CurrentTickCount();
		EllapsedTime_ = (double)(CurrentTickCount_.QuadPart - 
OldTickCount_.QuadPart)/TickFrquency_.QuadPart;
		TotalTime_ += EllapsedTime_;
		OldTickCount_ = CurrentTickCount_;
	}
	const LARGE_INTEGER& CurrentTickCount() const {return CurrentTickCount_;}

	double EllapsedMilliseconds() const {return EllapsedTime_ * 1000;}
	double EllapsedSeconds() const {return EllapsedTime_;}
	double TotalTime() const {return TotalTime_;}
};

template <typename ParamT>
class EventHandlerBase1
{
public:
	virtual void notify(ParamT param) = 0;
};

template 
class EventHandler1 : public EventHandlerBase1<ParamT>
{
	typedef void ( ListenerT::*PtrMember)(ParamT);
	ListenerT* m_object;
	PtrMember m_member;

public:

	EventHandler1(ListenerT* object, PtrMember member)
		: m_object(object), m_member(member)
	{}

	void notify(ParamT param)
	{
		(m_object->*m_member)(param);		
	}	
};

template <typename ParamT>
class EventHandlerFunc1 : public EventHandlerBase1<ParamT>
{
	typedef void ( * HandlerFunc)(ParamT);
	HandlerFunc m_handler;

public:

	EventHandlerFunc1(HandlerFunc f)
		: m_handler(f)
	{}

	void notify(ParamT p)
	{
		m_handler(p);		
	}	
};

template <typename ParamT>
class EventHandlerStdCallFunc1 : public EventHandlerBase1<ParamT>
{
	typedef void (_stdcall * HandlerFunc)(ParamT);

	HandlerFunc m_handler;

public:

	EventHandlerStdCallFunc1(HandlerFunc f)
		: m_handler(f)
	{}

	void notify(ParamT p)
	{
		m_handler(p);		
	}	
};

/// <summary>
/// C++ event with one parameter
/// </summary>
template <typename ParamT>
class CppEvent1
{
	typedef std::map< int, EventHandlerBase1<ParamT> *> HandlersMap;
	HandlersMap m_handlers;
	int m_count;

public:

	CppEvent1()
		: m_count(0) {}

	~CppEvent1()
	{
		HandlersMap::iterator it = m_handlers.begin();
		for(; it != m_handlers.end(); it++)
		{
			delete it->second;
		}

	}

	/// <summary>
	/// Attach a member function to the event
	/// </summary>
	template <typename ListenerT>
	CppEventHandler attach(ListenerT* object,void (ListenerT::*member)(ParamT))
	{

		typedef void (ListenerT::*PtrMember)(ParamT);	

		m_handlers[m_count]=new EventHandler1(object,member);
		m_count++;	
		return m_count-1;

	}

	typedef void ( * Callback)(ParamT);

	/// <summary>
	/// Attach a static function to the event
	/// </summary>
	CppEventHandler attach(Callback f)
	{
		m_handlers[m_count]=new EventHandlerFunc1<ParamT>(f);
		m_count++;	
		return m_count-1;

	}
	typedef void (_stdcall * MngCallback)(ParamT);

	/// <summary>
	/// Attach a managed function to the event
	/// </summary>
	CppEventHandler attach(MngCallback f)
	{
		m_handlers[m_count]=new EventHandlerStdCallFunc1<ParamT>(f);
		m_count++;	
		return m_count-1;

	}

	/// <summary>
	/// Detach an event handler from the event
	/// </summary>
	bool detach(CppEventHandler id)
	{
		HandlersMap::iterator it = m_handlers.find(id);

		if(it == m_handlers.end())
			return false;

		delete it->second;
		m_handlers.erase(it);
		return true;
	}

	/// <summary>
	/// Fire the event
	/// </summary>
	void notify(ParamT param)
	{

		HandlersMap::iterator it = m_handlers.begin();
		for(; it != m_handlers.end(); it++)
		{
			it->second->notify(param);
		}
		return;
	}
};

CppEvent1< char > cppEvent;

class TestCppEvent
{
public:
	CppEventHandler eventHandler;
	int counter;

	void Open()
	{
		eventHandler = cppEvent.attach(this,&TestCppEvent::OnEventReceived);
		counter=0;
	}

	void Close()
	{
		cppEvent.detach(eventHandler);
	}

	void OnEventReceived(char b)
	{
		counter++;		
	}
};

class TestBind
{
public:

	int counter;

	TestBind():counter(0){}

	void OnEventReceived(char b)
	{
		counter++;
	}
};

class TestSignal
{
public:

	int counter;
	TestSignal():counter(0){}

	void OnEventReceived(char b)
	{
		counter++;
	}
};

int _tmain(int argc, _TCHAR* argv[])
{
	PerformanceCounter timer;

	TestCppEvent testCppEvent;
	testCppEvent.Open();

	timer.Reset();
	for(int i = 0; i < 10000000; i++)
	{
		cppEvent.notify('a');
	}
	timer.CalculateEllapsedTime();
	testCppEvent.Close();

	cout<<"Zeit[TestCppEvent]:"< f = 
boost::bind(&TestBind::OnEventReceived,&testBind,_1);

	timer.Reset();
	for(int i = 0; i < 10000000; i++)
	{
		f('a');
	}
	timer.CalculateEllapsedTime();
	cout<<"Zeit[TestBind]:"< sig;

	sig.connect(boost::bind(&TestSignal::OnEventReceived,&testSignal,_1));;

	timer.Reset();
	for(int i = 0; i < 10000000; i++)
	{
		sig('a');
	}
	timer.CalculateEllapsedTime();
	cout<<"Zeit[TestSignal]:"<
...
Here is a late review.
...
* What is your evaluation of the design?
Excellent. The design of Boost.signals is excellent,
and Boost.signals2 follows the same design.
Boost.signals2 should provide an optional (non-thread safe)
compatibility mode with Boost.signals.
This mode should not be available by default,
but only if the user defines some macro.
This will make porting from Boost.signals
to Boost.signals2 much smoother.
Frank also mentioned the possibility of adding
a thread safe version of boost::trackable
based on boost::enable_shared_from_this.
That would be a very valuable addition to the library.
...
* What is your evaluation of the implementation?
Have not looked at the implementation.
...
* What is your evaluation of the documentation?
I agree with the other reviewers that it could be improved.
...
* What is your evaluation of the potential usefulness of the library?
Very useful. The lack of thread safety is the main weakness of 
Boost.signals,
and is the only point where Boost.signals is inferior to the Qt signals.
...
* Did you try to use the library? With what compiler? Did you have
any problems?
Tried with MSVS 9.0. No problems.
...
* How much effort did you put into your evaluation? A glance? A
quick reading? In-depth study?
Read the docs. Wrote, compiled and ran a few simple examples.
...
* Are you knowledgeable about the problem domain?
Yes. I do all my work in an event-driven framework.
The three Boost libraries I use most are smart pointers, bind and signals.
I also have a lot of experience with the Qt signals.
...
* Do you think the library should be accepted as a Boost library?
Yes. We need thread safe signals.
Many thanks to Frank for submitting this excellent library,
Johan Råde

Re: [Boost-users] [signals2][review] The review of the signals2 library (formerly thread_safe_signals) begins today, Nov 1st

Hansi