[Multi-Thread Patch] Movement Thread

[Woweur]

What bug does the patch fix? What features does the patch add?

My patch add a thread to handle a movement opcode. I have write this patch for reduce the session update diff. For example, with 1.2k plr, the session update diff is ~300ms without the patch. With the patch, the session update is ~5 ms. The major problem is to fix some deadlock with lock or other ...

For which repository revision was the patch created?

9113

Who has been writing this patch? Please include either forum user names or email addresses.

Me

Of course, you can post a crashlog ...

If you can, aslo post the uptime you have with the patch or other bug due to the patch ...etc

Crashlog list :

http://paste2.org/p/597363

http://paste2.org/p/597366

http://paste2.org/p/597368

Github :

http://github.com/Izidor/mangos/tree/MovementThread

Patch :

http://paste2.org/p/596791

[XdK]

Nice job.

Maybe provide a git branch too ?

[Woweur]

Yes, i think it's a good idea. I will make that tomorrow

I have fixed a crash in ClientGUID iterator in class player with a LockedSet (like LockedQueue implanted by MaNGOS).

Nobody has tried the patch ?

EDIT : i have added a crashlog list in a first post.

[Forgotten]

the main problem is that these threads will write into grid containers.. i'm not sure that these containers are able to be used by more than one thread

[False.Genesis]

this patch looks interesting, had just a quick look and noticed you are reading from the config file on every update call: sConfig.GetBoolDefault("MovementThread", false) ...

about World::UpdateMovementOpcode(): i am wondering why you re-check for session validity, it should be easier (and faster) to lock the mutex before getting the session first time and avoid a double-call. in 99.9% of all cases a player WILL be logged in when mangos receives a movement opcode.

correct me in case i'm wrong.

EDIT: avoiding this double-check would also possibly fix the deadlock problem - or you use a ACE_Guard<ACE_Thread_Mutex>(mutex) and create a locked scope instead of using explicit acquire()/release() calls, thats a lot safer (would get you rid of the mutex.release() after LogoutPlayer() also)

last thing: why are you starting the thread with lowest priority..?

[Woweur]

Yeah, i think it's better to use a bool in World class for "sConfig.GetBoolDefault("MovementThread", false)".

I recheck the session because if i don't do that, i have a crash with delete pointer, and with this recheck, i haven't crash related to that. For a mutex, yes it's maybe better to use ACE_Guard. Why i have starting the thread with lowest priority ? Because i think the main thread must have the priority, a movement isn't realy important than cast spell, update ..etc.

EDIT : i have added a link to a github in first post

EDIT 2 : i have maybe a idea to fix some deadlock, if in all function where TypeContainerVisitor is used, i use a ACE_Guard , this can be work no ?

[crashuncle]

Looks awesome, I think this will help a lot especially for test areas with 1000+ players.

[zhenya``]

why not implement all opcodehandler in own thread? not only movement

[Woweur]

Because it's cause a lot of deadlock

[xeross155]

What does deadlock mean ?

[leak]

http://lmgtfy.com/?q=deadlock

[xeross155]

So by giving the opcodes there own threads you would cause deadlocks, just use mutexes ?

[False.Genesis]

using many mutexes and a lot of sync/locking will likely make an app slower then using no threading at all. i suggest you read some info about what to take care of when writing multithreaded apps.

it makes only sense putting these parts into an own thread that that use much CPU time and that can run without requiring too many resources from other threads that require locking (e.g. to ensure that pointers are valid, etc).

[nanounico]

I hope be ready soon for port in master, this could be a reall nice improvement, Congrats woweur..

[xeross155]

using many mutexes and a lot of sync/locking will likely make an app slower then using no threading at all. i suggest you read some info about what to take care of when writing multithreaded apps.

it makes only sense putting these parts into an own thread that that use much CPU time and that can run without requiring too many resources from other threads that require locking (e.g. to ensure that pointers are valid, etc).

Your explanation makes enough sense to not have to read more info on it.

[Woweur]

Nobody has tried the patch ?

I think just this code can cause crash :

     UpdatePlayerVisibility(player,new_cell,new_val);
    UpdateObjectsVisibilityFor(player,new_cell,new_val);
    PlayerRelocationNotify(player,new_cell,new_val);

[slackware]

Hi woweur, just thanks for this, you make a good work

I am using this, and i have the same crash

And work nice, the problem is this crashs =(

[Woweur]

You can post the crashlog ? Thanks you.

[henhouse]

Wow looks really good! Will the patch apply fine with 0.12? Because I'll be glad to test it on there, but if it doesn't I might need to manually apply the patch.

[nanounico]

Any news??

[henhouse]

I wrote a backport patch for mangos-0.12: http://pastebin.com/f4a574105

Tested and working fine on 0.12 -- I'm running on LAN testing here and went from like 3-5ms (because of LAN) to 0ms. This thing is pretty amazing!

[XdK]

I think this patch can be considered very stable since it's used with about 3k players at rush hours on Izidor's server and causes no crash.

[leak]

I beg to differ, at least with vmaps that is.

http://paste2.org/p/620332

[slackware]

Here is stable 2k Online. 20h + uptime.

Good Work Woweur

[Shyke]

No, it's fairly unstable. I see crashes like the ones in the first post atleast once every 4 hours.