Spring is running in SMP

[koshi]

boost is 1.34.1-2ubuntu1.1

from detail/atomic_count_gcc.hpp:

Code: Select all

 void operator++()
    {
        __atomic_add(&value_, 1);
    }

from detail/atomic_count_pthreads.hpp:

Code: Select all

void operator++()
    {
        scoped_lock lock(mutex_);
        ++value_;
    }

edit: btw, if you want some more immediate response find me in #springlobby, #sy or #news

[imbaczek]

zerver: ask bibim for a buildbot.

[zerver]

OK, the win32 versions of the operator++ return a long value, while the pthreads/gcc versions do not. I'm not sure what the reason for the difference is. An atomic count is pretty useless if you cannot retrieve the incremented value.

Koshi, if you want to compile see below.

from detail/atomic_count_gcc.hpp:

Code: Select all

 void operator++()
    {
        __atomic_add(&value_, 1);
    }

Quick fix:

Code: Select all

    long operator++()
    {
        return __exchange_and_add(&value_, 1) + 1;
    }

from detail/atomic_count_pthreads.hpp:

Code: Select all

void operator++()
    {
        scoped_lock lock(mutex_);
        ++value_;
    }

Quick fix again:

Code: Select all

    long operator++()
    {
        scoped_lock lock(mutex_);
        return ++value_;
    }

[koshi]

your commit that was supoosed to fix didn't: http://pastebin.ca/1030325

[zerver]

Okay, but it does not say what the error is ?

Strange...

[koshi]

ups, forgot to redirect stderr to the pastebin script

here's the error: http://pastebin.ca/1030408

[koshi]

got it to compile at last!

had to fix some minor issues: some includes of StdAfx.h had wrong case and a colliding typedef of BOOL (was already defined in a X include). I simply replaced BOOL with BOOL_

AI compile is broken tho. Seems someone removed global namespace pollution, but didn't add all the needed qualifiers in source files. I started fixing AI/Group/EconomyAI but after that i saw AI/Global/KAIK-0.13 errors and stopped

I don't have time to do any real testing, but game starts fine with xta 9.44 and air combat tests script.

[Kloot]

Get zerver to update the AI/ part of his branch to trunk.

[zerver]

Thanks Koshi! Patch has been applied. I'm happy to hear it compiles and even runs.

However, I'm unsure about any possible performance gains on Linux,
because I had to resort to using mutexes instead of volatile optimizations.

I won't update the AI part of my branch because the next step is to put this in trunk

[koshi]

Come friday i'll have more freetime. If you can get me a windows build of your branch i could do some comparisions.

[zerver]

OK... it is in trunk now.

I added a special Release+MT configuration to the visual studio project that enables the multithreading stuff.

[Auswaschbar]

Code: Select all

In file included from rts/lib/gml/gml.h:16,
                 from rts/Rendering/GL/myGL.h:11,
                 from rts/Rendering/VerticalSync.cpp:12:
rts/lib/gml/gmlcls.h: In member function 'void gmlMutex::Unlock()':
rts/lib/gml/gmlcls.h:143: error: expected class-name before '(' token
rts/lib/gml/gmlcls.h: In member function 'bool gmlLock::Lock()':
rts/lib/gml/gmlcls.h:563: error: 'class boost::unique_lock<boost::mutex>' has no member named 'locked'
rts/lib/gml/gmlcls.h:569: error: expected class-name before '(' token
rts/lib/gml/gmlcls.h: In member function 'void gmlLock::Unlock()':
rts/lib/gml/gmlcls.h:577: error: expected class-name before '(' token
scons: *** [build/rts/Rendering/VerticalSync.o] Error 1
scons: building terminated because of errors.

Latest trunk fails to build.

Edit:
Resolved error, an include file was missing.
Now it spams lots of errors about ambigious overloaded function calls (sin, cos, sqrt etc.)

[zerver]

Now it spams lots of errors about ambigious overloaded function calls (sin, cos, sqrt etc.)

Does it compile if you remove this code in lua_include.hpp ?

Code: Select all

#if (BOOST_VERSION >= 103500)
#undef _CMATH_ // workaround to make it compile with MSVC + Boost 1.35
#endif

I had to insert this very ugly "fix" to make Boost 1.35 compile with MSVS, but I forgot to make it conditional for MSVS only.

If it still does not work, tell me about your compiler setup and boost version.

[Auswaschbar]

_CMATH_ has no effekt for g++. I found out that the reason is the following:
We have streflop::sqrtf and also sqrtf in math.h, so this is ambigious because of using namespace streflop; (also, sqrtf is not part of C++)

Also, some files have using namespace std, so there are std::sqrt and streflop sqrt, which is ambigious too.

It seems that because you changed some includes, which is why the math functions get included everywhere, and trigger this.

Now I'm busy with fixing this in half of the code files...

[zerver]

Could be, but I did not include any math.h in my changes so maybe it is included somehow by boost?

Please note that for me it compiles just fine with scons/mingw/gcc4.3.0.

[Auswaschbar]

Could be, but I did not include any math.h in my changes so maybe it is included somehow by boost?

Please note that for me it compiles just fine with scons/mingw/gcc4.3.0.

Or by glew, or gl etc.

PS.: gcc 4.3.1, boost 1.35.0

[zerver]

Please note that my library is based on the assumption that all files that require GL should include mygl.h. Including gl.h, glext.h, glu.h etc. will make the multithreading code fail to run.

[Tobi]

zerver, I noticed your patch duplicated huge chunks of code in e.g. BFGroundDrawer.cpp and UnitDrawer.cpp (and maybe more?).

I'm not particularly fond of code duplication, not in the least because I've been removing/refactoring quite some of it recently.

Some of the problems with this kind of code duplication is that one part might get bugfixed while the other stays broken (it is totally unclear it is duplicated code too), that code is a lot larger then it needs to be (remember, the best code is no code at all), etc.

Could you refactor this ASAP into better maintainable code?

Also use whitespace between operators (1 + 1 and not 1+1) and use only tabs to indent please.

I hoped to spot these things before giving go ahead, but due to lack of time I never got chance to look really good at branch...

[zerver]

The reason for doing this was to make sure the single-threaded performance did not suffer even the tiniest bit. I'll see what I can do.

[imbaczek]

BTW what's the speedup on 2 cores?