GlobalEventSet::fireEvent performance

[fedyakin]

We've been spending some time optimizing our engine's performance. Using Intel's sampling profiler, we found that GlobalEventSet::fireEvent is the fourth biggest hotspot in our engine. After some investigation, it turns out that the cost of this function is in the string concatenation that builds the event name.

I'm not sure if this effects everyone (we have a lot of windows), but it seems like there might be some merit to refactoring things to remove the per-event string concatenation. Off the top of my head, I see three ways it could be done.

1) Modify EventSet::fireEvent to take the event name and global event name. This would require a second static string constant for all event names. A set of macros could be used to always declare and define both event names. The advantage of this, is that it has a very low runtime cost. The disadvantage is that its either tedious, or uses macros.

2) The implementation of GlobalEventSet could change, to store events in a manner that didn't require a string concatenation to find them. For example a std::map<std::pair<String, String>, Event*>. This has the advantage of not changing the interface to the event system. The downside is that it requires some duplication of code that already exists in EventSet, and is not quite as efficient as the previous method.

3) The implementation of EventSet is modified to support two string keys (as in #2 above), and the existing method implementations pass in the empty namespace string. The advantage is that GlobalEventSet doesn't have to duplicate as much EventSet code, and the existing interface doesn't change. The downside is that it probably adds overhead to non-global events.

What do you guys think? Is there a better solution?

--Thanks

[CrazyEddie]

I have seen this too, when having profiled some years ago - however, it wasn't the issue I was working on, and wasn't that bad given the set of windows and their usage in the test code I was working with - so nothing was done at that time.

I've not re-examined our code as of this writing, so will not comment on the possible solutions given (nor to propose other alternatives) other than to say anything involving macros would be used solely as a last resort, and then only if the world was going to end in 24 hours

However, these issues are important to us, so please add a ticket to mantis with the same information contained in your post (along with anything else you might think useful), otherwise this will be lost, forgotten or "conveniently ignored" until the next guy raises the issue again in a couple of years. Of course, having a mantis ticket does not guarantee a timely resolution, but we do sometimes get guilty feelings when browsing those

For a client project focused solution, if you're not using the global events at all, it might be safe to completely remove the global event set from the CEGUI you're using. Though clearly that's somewhat brutal and depending on various things might be impossible or impractical for your use.

Thanks for raising the issue.

CE.

[fedyakin]

As you suggest, disabling the global event set seems to be safe. We weren't using the global event set, so I just commented out the call to GlobalEventSet::fireEvent in EventSet::fireEvent.

I've entered a report in mantis, and included a link back to this thread. I also entered a report for the only other CEGUI function that showed up in our top hotspots (Window::getTargetChildAtPosition). Overall, cegui consumes very little time though

Thanks for the quick response.

[CrazyEddie]

Thanks for creating the tickets, and for the general feedback - it's very valuable to have that from real-world usage of the lib (as opposed to contrived tests and minimal examples that we generally use internally )

CE.