Jump to content

  • Log In with Google      Sign In   
  • Create Account

Very, very fast event library


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
54 replies to this topic

#1 Sneftel   Senior Moderators   -  Reputation: 1781

Like
0Likes
Like

Posted 21 July 2007 - 09:47 AM

When profiling some code I'd been working on recently, I was surprised and dismayed to see boost::signals functions floating to the top. For those of you who are unaware, boost::signals is a wonderfully useful signal/slot library which can be used alongside boost::bind for delegate-based event handling such as one sees in C#. It is robust, featureful, and flexible. It is also, I have learned, incredibly, terrifyingly slow. For a lot of people who use boost::signals this is fine because they call events very seldom. I was calling several events per frame per object, with predictable results. So I wrote my own. Slightly less flexible and featureful. It's optimized for how everyone tends to actually use events. And event invocation is fifteen to eighty times faster than boost::signals. The library relies upon Don Clugston's excellent FastDelegate library and upon the deliciously evil, horribly useful boost::preprocessor library. The former is included; the latter is not, so you'll need to have Boost installed and its headers available to the compiler. A basic usage guide is as follows:
#include <event.h>
#include <string>
#include <iostream>

class Subject
{
public:
	// events::Event is parameterized by the argument list passed to the event handlers.
	// I like to make events public and mutable. If you want you can hide them behind
	// mutators instead.
	mutable events::Event<std::string const&> NameChangedEvent;

	// I've decided not to make events invocable via operator(). It's too easy to 
	// accidentally invoke an event in this way.
	void SetName(std::string const& name)
	{
		m_name = name;
		NameChangedEvent.Invoke(m_name);
	}

private:
	std::string m_name;
};

class Observer
{
public:
	Observer(Subject const& subject) : m_subject(subject)
	{
		// Registers the OnNameChanged function of this object as a listener, and
		// stores the resultant connection object in the listener's connection set.
		// This is all you need to do for the most common case, where you want the
		// connection to be broken when the Observer is destroyed.
		m_cs.Add(m_subject.NameChangedEvent.Register(*this, &Observer::OnNameChanged));
	}

private:
	// I like to make my event handler functions private, delegating to public functions
	// as necessary.
	void OnNameChanged(std::string const& newName)
	{
		std::cout << "The subject's name is now " << newName << "." << std::endl;
	}

	// The connection set is a utility class which manages connection lifetimes for
	// you. It's similar in functionality to boost::signals::trackable, but provides
	// slightly more flexibility over connection lifetimes. For the normal case,
	// you only need one connection set per listening object (no matter how many
	// things it listens to) and you never need to do anything with it except call Add().
	events::ConnectionSet m_cs;

	Subject const& m_subject;
};

int main()
{
	Subject s;
	Observer o(s);

	s.SetName("Joe");
}













This is beta-quality software, and its interface and implementation are both still in flux. It's intended primarily to get ideas flowing here on what form the ideal event library would take. Is this library too much or too little? Are there widely useful features it's missing? In particular, I'm interested to hear whether the ConnectionSet system is reasonable. download (ver 0.08) [Edited by - Sneftel on July 28, 2007 12:59:58 AM]

Sponsor:

#2 Antheus   Members   -  Reputation: 2397

Like
0Likes
Like

Posted 21 July 2007 - 09:54 AM

Could it work with Impossibly fast delegates

I find the above quoted solution to be far too hackish, relying on compiler and architecture specifics, whereas this one is on-par, but only relies on language features.

It also contains an example of event dispatching, although I didn't test it.

#3 Sneftel   Senior Moderators   -  Reputation: 1781

Like
0Likes
Like

Posted 21 July 2007 - 09:57 AM

I waffled over that for some time. Impossibly Fast Delegates is incredibly elegant compared to Fastest Possible. It is, however, considerably less supported by actual compilers (IIRC, it doesn't even work on VC++.NET 2003). But yes, it should be trivial to change from one to the other.

#4 SiCrane   Moderators   -  Reputation: 9596

Like
0Likes
Like

Posted 21 July 2007 - 10:11 AM

I'm not convinced Connection::operator=() handles self assignment properly if m_node && m_node->refCount == 1. It seems like it'd decrease the reference count to 0 then assign 0 to m_node thus annihilating its state.

#5 Sneftel   Senior Moderators   -  Reputation: 1781

Like
0Likes
Like

Posted 21 July 2007 - 10:14 AM

*smacks forehead* I'm not exactly sure how that got off my TODO list without actually getting done. Thanks.

#6 SiCrane   Moderators   -  Reputation: 9596

Like
0Likes
Like

Posted 21 July 2007 - 10:16 AM

For that matter, while I suck horribly at reading boost::preprocessor code, I can't seem to find assignment operators or copy constructor for the Event<> class (or a NonCopyable inheritance).

#7 Sneftel   Senior Moderators   -  Reputation: 1781

Like
0Likes
Like

Posted 21 July 2007 - 10:18 AM

Hmm, you're right. I forgot to copy over the boost::noncopyable designation when I switched to file-based iteration.

#8 Sneftel   Senior Moderators   -  Reputation: 1781

Like
0Likes
Like

Posted 21 July 2007 - 12:46 PM

Both of the above issues (and a couple of others) are now fixed in the latest version.

#9 SiCrane   Moderators   -  Reputation: 9596

Like
0Likes
Like

Posted 21 July 2007 - 01:01 PM

I'm trying to convince myself of the utility of the ConnectionSet class, and to be honest, I'm failing. It doesn't seem to supply any additional invariants over a container of Connection objects that an object could hold anyway. Right now it just seems like a good way to create dangling references in an Event<> delegate chain.

#10 Sneftel   Senior Moderators   -  Reputation: 1781

Like
0Likes
Like

Posted 21 July 2007 - 01:03 PM

Quote:
Original post by SiCrane
I'm trying to convince myself of the utility of the ConnectionSet class, and to be honest, I'm failing. It doesn't seem to supply any additional invariants over a container of Connection objects that an object could hold anyway.

Oh, it surely doesn't. It's primarily there as a simplified adapter class, and as a "best practice" container which can have its implementation modified as necessary. In particular, I've been thinking about ways to be able to free the heap-allocated refcount once the Connection is put in a ConnectionSet, to optimize for the case where that's the one and only reference.
Quote:
Right now it just seems like a good way to create dangling references in an Event<> delegate chain.

Hm... how do you mean?

#11 SiCrane   Moderators   -  Reputation: 9596

Like
0Likes
Like

Posted 21 July 2007 - 01:17 PM

Ok, I see dangling references happening in two ways: (1) not so bright library user doesn't realize that the connection set should be associated with an object and creates a global connection set somewhere. I know that a library doesn't necessarily need to protect users from themselves, but having the container in the public interface for a library is just asking for trouble, at least without clear documentation.

(2) ConnectionSet only supports Add() and Clear(). Obvious problem: User wants to change an Event<> callback to a different member function of the object, forgets to call Clear(). End result: ConnectionSet is now keeping alive two delegates into the Event<>. Again, protecting user from themselves problem, but it's still something I can see happening.

Related problem: User wants to only modify one Event<> attached to a delegate. Needs to flush and rebuild all the connections. Not a big issue, but does result in some unnecessary memory juggling and some cache unhappiness, which doesn't exactly play to the "Very, very fast event library" goal.

Basically, without a clear goal for the ConnectionSet, it just seems to add potential problems without adding any value.

Just thinking aloud here: maybe if you modified it so that a ConnectionSet associated Connection objects with Event<> objects?

#12 Sneftel   Senior Moderators   -  Reputation: 1781

Like
0Likes
Like

Posted 21 July 2007 - 01:26 PM

Quote:
Original post by SiCrane
Ok, I see dangling references happening in two ways: (1) not so bright library user doesn't realize that the connection set should be associated with an object and creates a global connection set somewhere. I know that a library doesn't necessarily need to protect users from themselves, but having the container in the public interface for a library is just asking for trouble, at least without clear documentation.

(2) ConnectionSet only supports Add() and Clear(). Obvious problem: User wants to change an Event<> callback to a different member function of the object, forgets to call Clear(). End result: ConnectionSet is now keeping alive two delegates into theEvent<>. Again, protecting user from themselves problem, but it's still something I can see happening.
You're right, of course. One alternative I had thought about was making ConnectionSet into something intended to be inherited from, a little akin to boost::signals::trackable, with Add() renamed to something a little more specific. I can't decide whether that method would be protected or public, though, and I can think of problems with both approaches.
Quote:
Related problem: User wants to only modify one Event<> attached to a delegate. Needs to flush and rebuild all the connections. Not a big issue, but does result in some unnecessary memory juggling and some cache unhappiness, which doesn't exactly play to the "Very, very fast event library" goal.

For this situation, I would envision the object holding a separate Connection, or even a separate ConnectionSet. Of course, this doesn't require the existence of ConnectionSet as opposed to simply a vector<Connection>.
Quote:
Basically, without a clear goal for the ConnectionSet, it just seems to add potential problems without adding any value.
Yeh, I'm coming around to your point of view. If/when I figure out how to put in the no-heap-allocation stuff I mentioned earlier, I'll take another look at it, but for now it doesn't fill a need.
Quote:
Just thinking aloud here: maybe if you modified it so that a ConnectionSet associated Connection objects with Event<> objects?
What do you mean by this? My design was intended to make Connection as opaque a class as possible, for simplicity and because spangling connections with features is one of the primary reasons for boost::signal's slowness. Were you thinking of a ConnectionSet as a means of recovering this hidden information?


#13 SiCrane   Moderators   -  Reputation: 9596

Like
0Likes
Like

Posted 21 July 2007 - 01:53 PM

I was just trying to come up with a point for ConnectionSet. It seems like when you add a Connection to the set, you know the Event, the object being bound and the member function. The object being bound should always be the object owning ConnectionSet (or they should be members of the same object, etc.) so that information doesn't need to be kept around. But if you kept a pointer to the Event, and then the ConnectionSet could use the ref counts that the Connections being stored keep, then you could potentially use the ConnectionSet to determine if a given Event<> is still live.

I suppose even without keeping the Event<> pointers around, you can use the ref count information to purge connections if the associated events have been destroyed. So ConnectionSet could have a member PurgeDeadConnections() or something similar. That wouldn't even need friend access if Connection had a member like bool IsEventAlive(void) const { return m_node->refCount > 0; } Then there would be at least a little value added over a vector<Connection>.

Still just thinking out loud.

That reminds me. You might want to add a swap() function for Connections that just swaps the node pointers.

#14 Sneftel   Senior Moderators   -  Reputation: 1781

Like
0Likes
Like

Posted 21 July 2007 - 02:08 PM

The need for a Purge method is something I've thought a lot about. Event purges only on Invoke, of course, which potentially increases memory use, but I'm hard-pressed to invent a situation where that would be both significant and long-term. As for connections referring to dead events, I don't think it's that much of a problem: in how people use the Observer pattern that I've seen, Observers rarely outlive their Subjects by long.

I see what you mean about ConnectionSet becoming a more featureful event-management system. I have no need for such a thing, at least that I'm yet aware of, so I probably can't design a good one.
Quote:
You might want to add a swap() function for Connections that just swaps the node pointers.

I'll specialize std::swap for that.

#15 SiCrane   Moderators   -  Reputation: 9596

Like
0Likes
Like

Posted 21 July 2007 - 02:44 PM

I can come up with situations where an observer would significantly outlive the subject but not in C++ code you'd actually see in a released product. For example, in C# someone might dynamically create a form and then associate a delegate with long lived object to an OnClick() for a button, and then destroy the form after its processing is done. However, in similar situations in C++, programmers just generally keep a single copy of a dialog around since C++ programmers tend to be more allocation adverse. Even in the situations I can imagine, I don't see the need for a ConnectionSet::Purge(), since none of those situations need a full ConnectionSet, just a single Connection object.
Quote:

I'll specialize std::swap for that.

Also consider adding a version in your events namespace. Koenig lookup is your friend. Except when it isn't.

#16 T1Oracle   Members   -  Reputation: 100

Like
0Likes
Like

Posted 21 July 2007 - 02:59 PM

Interesting work, I will soon be needing a delegate implementation for my GUI although I may go the simple route and use one function object per GUI event instead of maintaining a list. I'm not trying to put too much into the GUI I just want it to share as much code with the rest of my engine as possible.

Either way, keep up the good work.
Programming since 1995.

#17 Promit   Moderators   -  Reputation: 7189

Like
0Likes
Like

Posted 21 July 2007 - 03:05 PM

I'd like to take this time to suggest a promotion for SiCrane from Staff to Total Badass Developer.

#18 Sneftel   Senior Moderators   -  Reputation: 1781

Like
0Likes
Like

Posted 21 July 2007 - 05:01 PM

Quote:
Original post by SiCrane
Koenig lookup is your friend. Except when it isn't.

Wouldn't this be an "isn't" situation? If I put it only in events, std::swap will be transparently inefficient. And if I put it in both, using namespace std or events will cause an ambiguity.

#19 Sneftel   Senior Moderators   -  Reputation: 1781

Like
0Likes
Like

Posted 21 July 2007 - 07:53 PM

Alright, I've had an idea. *cue ominous thunder* I've added an AutoTracked class, very similar to boost::signals::trackable. To use it, you inherit from it, and then instead of calling Register() you call AutoRegister() (not wild about the name, but whatever), a function which puts the connection in a private ConnectionSet (a class which has finally found a use, though it may move to detail) and returns void. So the basic usage mode is to only ever call AutoRegister(); but if you need finer-grained control, you can call Register() instead and deal with the Connection yourself.

I've also made some minor changes to allow the code to compile cleanly under /W4.

#20 SiCrane   Moderators   -  Reputation: 9596

Like
0Likes
Like

Posted 22 July 2007 - 01:04 AM

Quote:
Original post by Sneftel
Quote:
Original post by SiCrane
Koenig lookup is your friend. Except when it isn't.

Wouldn't this be an "isn't" situation? If I put it only in events, std::swap will be transparently inefficient. And if I put it in both, using namespace std or events will cause an ambiguity.


Putting it in both shouldn't cause an ambiguity. ADL should always pickup the version in the events namespace unless std::swap is explicitly used, even if using namespace std, events or both is in scope.

Regarding the AutoTracked class: I'm not sure that private inheritance is a good idea here. Or, at least, if you use private inheritance, put in a using ConnectionSet::Clear. Or, (and this is really the ickiest of the ideas) add functionality for the AutoTracked class to detect that its been registered with the same Event<> multiple times.

Also, I'd probably move AutoRegister to become a member function of AutoTracked rather than AutoRegister. At time of call of AutoRegister, you need to know three things: The identity of the object, the identity of the member function and the identity of the event. With AutoRegister in the Event<> class, it's possible to mistakenly call AutoRegister with a non-AutoTracked object; with AutoRegister as part of the AutoTracked class it's not. Though I admit that you took the time out to make the conversion error understandable.




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS