Performance of GIGI/Font::DetermineLinesImpl

[cyanic]

Hello all. After playing several games of FreeOrion I decided to look into some of the late game lag. I ran FreeOrion through the Very Sleepy profiler and found that it's spending a lot of time in DetermineLinesImpl when selecting a star system. It looks like the six regular expressions in that function are re-compiled each time it is run. I was able to get a very modest performance increase by pulling the TAG_PARAM, WHITESPACE, and TEXT expressions into write-once static variables. Since the known tags are seldom modified while the game is running, I'd expect there are further gains to be had by keeping two copies of OPEN_TAG_NAME and CLOSE_TAG_NAME (one for ignore tags, one for not), and refreshing them whenever the known tags change.

I have not yet mentally parsed the EVERYTHING regex, but I imagine that's where most of the performance pain is.

Since I'm new to the code-base, are there any gotchas about this I should be aware of?

[Geoff the Medio]

Since I'm new to the code-base, are there any gotchas about this I should be aware of?

Not that I'm aware of... but I don't think anyone (who's still around and contributing) sufficiently understands all the GG code base to be sure. If assembling the regexes is doing needless processing each time, caching the results sounds reasonable.

There is probably a larger-scope issue with how the UI is updating when selecting a system though... Perhaps all the controls showing the system names in the sidepanel droplist are being regenerated when selecting a system, which could probably be avoided. (Haven't checked... just a guess...)

[cyanic]

I got better statistics out of Very Sleepy by caching the regexes, but at the cost of making the GIGI code not thread-safe (as in, the instance function assigns its local variables to static variables that are used by the shared regexes). I'm not sure this is actually a problem for the FreeOrion project, but it's icky nonetheless. The limitation was caused by what I believe is a bug in boost's Xpressive assertions (see my post at https://groups.google.com/forum/#!topic ... I-pk2BGea8; no responses yet).

If I don't get a response in the next week I'll post my patch and you can decide if you want it.

I've also found that the filesystem is getting hit on each GetArtDir call, I was able to cache things like the ship icon, research icon, and save some calls to "free" there as well.

When I compare performance of my changes vs the SVN 7108 test build I don't see much perceptual improvement, I'm wondering if the changes I'm making are already done by the compiler when the project is built in release mode (vs. debug).

Your comment about the UI being regenerated may be true but even if it is kept around you will need to change the text of the TextControls, and setText is the function that goes down into the regex compilation effort that I've been working with.

[Geoff the Medio]

I got better statistics out of Very Sleepy by caching the regexes, but at the cost of making the GIGI code not thread-safe (as in, the instance function assigns its local variables to static variables that are used by the shared regexes).

Do the regexes need to be shared? Would it not avoid repeated compilation to have each TextControl store its own regexes, which would be compiled only once per control?

[cyanic]

That is a good idea; I didn't think of it as the regexes are currently in Font, not TextControl, and it doesn't make sense to have one of them per font instance (as that lends itself to the same thread-unsafety issues).

[em3]

Alternatively, there's boost TLS.
http://www.boost.org/doc/libs/1_35_0/do ... orage.html

[Geoff the Medio]

That is a good idea; I didn't think of it as the regexes are currently in Font, not TextControl, and it doesn't make sense to have one of them per font instance (as that lends itself to the same thread-unsafety issues).

Would one regex per TextControl avoid recompiling issues, and how much memory would all those extra regex instances take up per TextControl?

[cyanic]

I've now got a version of SVN 7115 using TLS for the Font.cpp regexes, and performance is better. The SVN head version no longer builds with the SDK (see my other post about trouble with boost vc120).