StdAfx.h and compiletime

[Auswaschbar]

I'm annoyed by zerver who thinks that he need to include StdAfx.h everywhere, which
[*] increases compiletime a lot
[*] that file needs to be recompiled very often

Could you please stop that?

[Kloot]

Speaking of annoyances, code like...

Code: Select all

 int am2=(ast-2)&CLOUD_MASK, am1=(ast-1)&CLOUD_MASK, aa=(ast)&CLOUD_MASK, ap1=(ast+1)&CLOUD_MASK;
int a3c=ast+a3cstart, a4c=ast+a4cstart;
for(int a=ast; a<aed; ++rc, ++ct){
    ydif[ap1]+=(int)cloudThickness[++a3c]-cloudThickness[++a]*2+cloudThickness[++a4c];
    ydif2[ap1]=(ydif1[ap1]=ydif[ap1]>>1)>>1;
    int dif=(ydif2[am2]+ydif1[am2=am1]+ydif[am1=aa]+ydif1[aa=ap1]+ydif2[++ap1]) / 16;
    if(ap1>=CLOUD_SIZE)
        ap1=0;
    *ct++=128+dif;
    *ct++=thicknessTransform[(*rc)>>7];
    *ct++=255;
}

... is very hard to read, could you use some spaces and comments
in the future? Especially when doing pointer arithmetic such as this:

Code: Select all

int **prcy=prc;
int *pkernel=kernel;
for(int y2=0; y2<cs4a; ++y2, ++prcy, pkernel+=CLOUD_SIZE/4) {
    int *prcx=(*prcy)+x;
    int *pkrn=pkernel;
    for(int x2=0; x2<cs4a; ++x2) {
        (*prcx++)+=blend*(*pkrn++);

It may be slightly faster to juggle array indices manually, but it does
not make things particularly easy to follow or maintain.

[imbaczek]

I'm annoyed by zerver who thinks that he need to include StdAfx.h everywhere, which
[*] increases compiletime a lot
[*] that file needs to be recompiled very often

Could you please stop that?

It actually decreases compile times on VS2008 I believe, that's why he's doing it. Also, I'm not sure if #ifdefing this gives the desired result.

gcc supports precompiled headers and has been for a while, maybe try that?

[Auswaschbar]

But it increases compiletime for aynone else, and because you can't play online with msvc build anyway...

Just to show what we are talking about, I've attached two preprocessed versions of GameVersion.cpp, one with StdAfx.h included and one without. Look at the sizes and guess which is the one without

[imbaczek]

yeah, that's excessive.

i'll try to revive gcc precompiled headers, they seem to work, require some changes to headers, though. (only for scons though, but I think it'll be straightforward to add this to cmake.)

edit: done.
edit2: consistent 10% improvement on a windows box, I'd expect more on Linux.

[zerver]

Speaking of annoyances, code like...

Code: Select all

int **prcy=prc;
int *pkernel=kernel;
for(int y2=0; y2<cs4a; ++y2, ++prcy, pkernel+=CLOUD_SIZE/4) {
    int *prcx=(*prcy)+x;
    int *pkrn=pkernel;
    for(int x2=0; x2<cs4a; ++x2) {
        (*prcx++)+=blend*(*pkrn++);

It may be slightly faster to juggle array indices manually, but it does
not make things particularly easy to follow or maintain.

Sorry, I guess I'm a performance freak. Normally I would not do this pointer juggling because the performance gain is so minimal, but this specific code had 5 nested for loops bloated with repeated array index calculations. Although the optimizer will make things a lot better, it is not as clever as you may think. If you do not like the changes feel free to revert to a previous revision.

[Tobi]

IMO we should remove the StdAfx.h as it is in it's current form because it just promotes spaghetti dependencies between code.

Precompiled header should only be used to include some huge STL stuff and not to define compatibility layer / exception classes etc., making you have to include 100k lines of code just for throwing a content_error.

I tested precompiled headers on GCC a year ago or so and it doesn't make it particularly faster.

All in all, I'd say precompiled headers are a typical example of premature optimization being root of all evil

[imbaczek]

I get ~4:54 mins compile time with gch off and ~4:25 with gch on. It's not stellar, but it is noticeable.

[Tobi]

Oh ok that got slightly better then.

Either way I will perform some cleanup now.

[Auswaschbar]

Oh ok that got slightly better then.

Either way I will perform some cleanup now.

I already started doing some and will commit it as soon as i get it to compile again.

[zerver]

If you remove stdafx I will need some other file that I know will be included before everything else, in every cpp file.

Why? GML needs this to patch boost::detail::atomic_count.

[Auswaschbar]

• You are obviously using it wrong
• buildserv can build the GML profile without this

[zerver]

This requirement is with boost 1.36+

Maybe buildserv is using boost 1.35 ?

Edit: Alright, only the files using GL need will stdafx or an equivalent...

Edit: Never mind, I got it solved but I had to include myGL in some files even though they are not using GL.

[imbaczek]

benchmark after the cleanup:

with precompiled header: 4:34
without: 4:49

basically, traded blows.

edit: if I put top 15 included files into stdafx, compile time with precompiled headers drops to 4:17. unfortunately, debug exe size baloons up from ~86MB to 95MB.

[reivanen]

Wouldn't the thing to focus on here be to make the code as developer-friendly as possible and maintain compatibility with all development environments? Not so much in figuring out which method saves 10-20seconds in compile.

Spaghetti code is a nightmare to read and improve...

[Tobi]

+1

Code for readability, not speed, unless profiling data shows it's the part that takes >90% of CPU time. And even then, try to make it faster while keeping it readable

Either way I suppose the code got more readable from the PCH changes because it drags in less Spring code now I think.