FreeOrion

OK, that's probably a cheesy title for this thread, but users of MSVC++ have the advantage hundreds of millions of dollars worth of programming hours spent in refining cl.exe resulting in the availability of link-time optimizations. This is nice in several ways, you get really good optimizations of stuff across object files and even allows you to use some practices that aren't good elsewhere, like putting trivial function definitions in your .cpp files instead of inlining them your headers (like getters & setters). However, for those using compilers that don't support link-time optimizations (or who's support is not yet mature, in the case of gcc) we end up with code that is both slower (due to making a function call) and larger (because the function call is more code than the actual function definition), which again results it in being even slower because we have a lower cache-hit likelihood. This doesn't even include missing out on other various optimizations as the result of the trivial code not being inlined.

As such, can we start putting trivial function definitions inline, either in the class declaration or at the bottom of header files? I've noticed that this is the case in some places, but not all (e.g., FleetUIManager::ActiveFleetWnd()).

Can you provide some evidence, ideally using specific changes to FreeOrion code, that demonstrates that this sort of change will actually make a difference to run time performance?

Also, would using the inline keyword not also produce similar results in some cases?

Yes, adding inline keyword will make the functions inline, as long as the calling code sees the definition. So for public methods one must place their definitions in headers (although not necessarily in class body).

_________________
[...] for Man has earned his right to hold this planet against all comers, by virtue of occasionally producing someone totally batshit insane. - xkcd

Well, rather that build test cases, I think it's better to just link some documentation. It's long been a "best practice" to simply always inline trivial getters and setters. I started using C++ back in 1996 and a whole lot has changed both in the language and in the compilers since then. MSVC++ has had "link time optimizations" (a.k.a. "whole program optimizations) since around 2003 (see http://msdn.microsoft.com/en-us/library/0zza0de8%28v=vs.71%29.aspx), so it's really a pretty new feature and gcc hasn't even caught up quite yet. (It's in gcc 4.6, but it is still considered experimental and not planned to be finalized until the release of 4.7)

Intel's compiler has support for it now, not sure when they introduced theirs. (http://software.intel.com/en-us/articles/intel-c-compiler-compatibility-with-microsoft-visual-c/#a_UIIDEIPO).

Also, you have to explicitly enable it, as my understanding is that the optimization is disabled by default. Also, I believe it's incompatible with the debug change and continue feature, but that doesn't matter much because you aren't going to use it when debugging. (here's the docs on MSVC 2010's http://msdn.microsoft.com/en-us/library/0zza0de8.aspx).

So yes, the inline keyword will do the trick, but only if it's the same translation unit that calls the inlined function. It can either be in the class declaration body or just stand-alone anywhere after the class declaration. Otherwise, it has to make a function call.

EDIT: An an afterthought, I have to comment how awesome compilers are now about inlined functions. Before, many compilers wouldn't remove duplicate function definitions at link time (due to functions declared as inline, but outlined anyway) unless the inlined function definition was in the class body, so it *only* used to work in the class bodies. It's really amazing how far C++ has come.

Well, no; I want specific cases where inlining a getter and setter in FreeOrion actually makes an appreciable difference to runtime performance, and linking documentation cannot demonstrate this.

I'm well aware that inlining might improve program performance in some cases, but I don't want to deal with making a bunch of changes all around the code and making headers more complicated and messy if there's no noticable difference in the runtime performance from doing so.

Well "appreciable difference" is a subjective specifier, but I get your point. The practice of inlining trivial member functions is almost as old as the C++ language. I don't know the code well enough to know where to pick out a few items that will show a marked difference or even if inlining all such member functions will make a marked difference at all, especially when "appreciable difference" can mean 1% improvement or 20%. Frankly, and with all due respect, I'm not interested in "making the case" that a C++ best practice will make a difference in FreeOrion. Maybe a student or newbie to programming can play with that, but if I'm going to contribute to a project (which I happen to enjoy) I’d rather do something meaningful.

If you're interested in a more in-depth treatment on the subject, I recommend Scott Meyer's "Effective C++" (preferably his 3rd edition) Item 30: "Understand the ins and outs of inlining". Or, if you want to get a general idea, make two different builds on MSVC++ (2003 or later), one with link-time optimizations and another without. While the difference between these two will be more drastic than the difference between inlining and not inlining trivial functions (when using another compiler), it should at least demonstrate the principle.

In sympathy to your concerns, it's sometimes favorable to not inline a trivial function when it increases header file dependencies, thereby causing more object files to be rebuilt when a change is made. However, if the trivial member function returns a pointer or a reference, a forward declaration for the class is sufficient, e.g.,



However, if a trivial member function attempts to access a member of that class, the declaration is required, which can expand header file dependencies (and none of us like that). I say "can expand" because it doesn't have to; the inlined trivial member function can be defined in the other header file -- not ideal, but it is a work-around.

Overall, the decision as to rather to inline or not can be complicated and depend upon several factors. But, simple getters and setters are just no-brainers; they are pretty much guaranteed to both decrease code size, increase speed. And on the bright side, if something seems reasonable to inline and the compiler thinks otherwise, it just ignores us and outlines it anyway! The exception to that being, of course, compiler-specific "force inline" modifiers, which I have rarely ever used.

There are a few other things that can aid a program somewhat under gcc, like the thoughtful use of "likely" and "unlikely" branch modifiers (or their "cold" and "hot" function modifier counterparts), etc., but if used poorly, they can end up harming performance rather than helping. MSVC++ doesn't implement these. Instead, they have a profiler which performs such optimizations automatically (after running an analysis).