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

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 <typename ListenerT,typename ParamT>
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<ListenerT,ParamT>(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]:"<<timer.EllapsedMilliseconds()<<endl;
	cout<<testCppEvent.counter << endl;	

	TestBind testBind;
	boost::function<void (char)> f = 
boost::bind(&TestBind::OnEventReceived,&testBind,_1);
	
	timer.Reset();
	for(int i = 0; i < 10000000; i++)
	{
		f('a');
	}
	timer.CalculateEllapsedTime();
	cout<<"Zeit[TestBind]:"<<timer.EllapsedMilliseconds()<<endl;
	cout<<testBind.counter << endl;	

	TestSignal testSignal;
	boost::signals2::signal<void (char)> 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]:"<<timer.EllapsedMilliseconds()<<endl;

	cout << testSignal.counter << endl;	
}

Johan Råde schrieb:
> 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