Ambal

Qsa, you know what it takes before you can claim that your methods are thread-safe, aren't you You have to protect your data with mutexes/atomics or make sure your data is 'read-only' if you don't use locks... and in the end don't forget to CHECK if you haven't missed something Mangos core is designing in such way that multiple Map::Updates() could run in parallel, so more and more functions/managers/data are made thread-safe and put inside this function call... In the end in World::Update() we should end up with just 2 functions: 1) World::UpdateSessions() 2) Map::Update() If you'll have more questions, I'll try to answer them. Cheers

In theory you are right, qsa, since w/o reader_writer locks or atomics it is not possible to guarantee thread safety of methods and consistency of data. But this actually happens ONLY if you read and/or write from ANY thread at the same time. What if you manipulate object's private data in only one thread while other ones can't access your object? For example, MovementInfo is a private data for Unit object and it cannot be manipulated from other threads - why would we need locks in this case when ONLY ONE thread can modify it, e.g. that one which is calling Map::Update() function? Do we need locks if we want to send wispers to players on any map/group<= this is read operations as soon as you can confirm your group is loaded and player is loaded in the world? So to make things simple: Threading model for mangos: thread_1: World::UpdateSessions() ... join(); ... thread_1 thread_2 ............. thread_N map_1->Update() map_2->Update() map_X->Update() ... join() ... World::UpdateSessions() <- single-threaded execution, ALL sorts of read-writes allowed Map::Update() - this is where we process 'thread-safe' packet handlers. writes/reads allowed on data which is either not accessible from other threads OR it is 'read-only' OR protected by locks. So it is your responsibility to check if data accessed by handler stays const OR not accessed from other threads while you are changing it when you want to execute packet handlers in Map::Update() by setting their PacketProcessing type to PROCESS_THREADSAFE!! Is it explains enough for you, qsa?

At current stage you should verify if this patch does not degrade packet processing time because of internal scheduling. You should check server uptime too since if you start getting weird crashes - it means some packets are not threadsafe OR teleport logic started to become broken. So grab all your mtmaps/vmaps//mmaps patches you have and write your findings here Also, I hardly understand what "if overal moving will be improved" means since packet scheduling covers ALL sorts of opcodes and not only movement packets - in future we want to parallelize as many packets as mangos core will allow us. Currently we cannot afford to parallelize ALL opcodes by moving ALL packet processing into Map::Update() - this is very unsafe and it will definitely either make your server crash often OR will lag it to the death Player/object movement is totally different field of logic so it has to be improved in specific patches. P.S. We should also verify possibility to parallelize processing of all "messaging" packets ( WorldSession::HandleMessagechatOpcode() ) - they seems to be heavy too. Cheers to everyone

No specific mtmaps patch is needed - just pick up that which works for you

no, this is not a multithreaded engine - its purpose is to improve core scalability when you have mtmaps patch installed. Mangos core is not officially multithreaded.

this patch is not meant to fix issues with existing packet handlers. So if chat messages are lost - they are still lost...

Hi everyone. Another essential patch for mangos core. This time we are going to implement packet processing schedule algorithm which allows us to handle some packets in multi-threaded mode, e.g. in Map::Update() function (ofc if you have mtmaps patch installed). Patch idea goes to mangos core team and implementation goes to me Why do we need this? If packet is thread-safe to process it in multi-threaded mode - why not do it? For example, movement packets turned to be ~60% of overall packets processed by the server and they are not only HUGE CPU consumers (vmaps, mmaps, relocations, creature AI etc), they are also THREAD-SAFE because their handlers are safe to run in parallel. So to achieve this task, packet scheduling classification was added: enum PacketProcessing { PROCESS_INPLACE = 0, //process packet whenever we receive it - mostly for non-handled or non-implemented packets PROCESS_THREADUNSAFE, //packet is not thread-safe - process it in World::UpdateSessions() PROCESS_THREADSAFE //packet is thread-safe - process it in Map::Update() if player is in world }; In Opcode.cpp file you will find how packets are set to be processed. Currently, there are mostly movement opcodes are set to be processed in Map::Update() function calls. We did not tried to inspect ALL existing packet handlers for their safety since it is nearly impossible to achieve it in small amount of time. So you are free to investigate further and submit your opinions about packet thread-safety It is highly recommended to have a mtmaps installed in order to test current patch, but if you have plenty of players - you can go on w/o installing mtmaps. All we need is to find out if player teleports/logouts do not cause any problems when happen in Map::Update() function calls. Feel free to post you comments, test results and suggestions. UPDATE: please, note, even if packet is set to be PROCESS_THREADSAFE but player is not in world (for example, is teleporting) - this packet will be processed like PROCESS_THREADUNSAFE in World::UpdateSessions()! Have fun Repo: https://github.com/Ambal/mangos

SilverIce does not have an internet access for now, some ISP troubles. I'll try to update this patch if I'll find some free time in nearest days

In [10767]. Thanks

In [10766]. Thank you

In [10759]. Brilliant

Compile w/o TBB memory manager and try to find the leak again - this way there won't be any additional components which could lead to faulty results.

This function is PRIVATE to ACE.dll so changes in MemoryManager.cpp do not interfere with ACE internal memory management. Also, be very careful mixing malloc/free with operators new/delete and any custom functions to allocate/deallocate memory - you can get in BIG trouble doing this.

In [10728]. Thanks you

New map system applied in [10727]. You have to upgrade your patch to make it compatible with new Core. Personal cheers for MMaps

Applied in [10727]. Thanks go to me and everyone involved in commenting and testing

Ok, since noone posted any complains about this patch, I suppose it is proven to be as suppa-cool-fast-bug-free-and-reliable as stated in the first post So get ready to see in git repo... if you don't have any objections

Faramir118, valgrind will do everything for you, so there is no need to write any 'proper' destructors

Ok, another portion of a fix has gone into repo - TerrainInfo class was not exported correctly and SD2 linked with errors. Thanks to zergtmn for spotting the source of this issue. Fixed. Also, here is a small patch to compile SD2 with current one: Change from: float fZ_Ground = m_creature->GetMap()->GetHeight(fX, fY, MAX_HEIGHT); to: float fZ_Ground = m_creature->GetTerrain()->GetHeight(fX, fY, MAX_HEIGHT); This is how the code in hellfire_peninsula.cpp should look like. Have fun Update: since ScriptDev2 team has removed the conflicting code in rev [1873], SD2 should be fully compatible with current patch.

Yep, I'm aware of ScriptDev2 incompatibility with current patch. I'll speak with their devs about writing a patch for this project since I can't simply return code back and wait for someone to update ScriptDev2 because of a simple rule: why bother yourself if your staff works fine? Stay tuned.

Blueboy, thanks alot with VC++ compiler we do use precompiled header so such errors are hard to track under Win. I'll update my patch once again :rolleyes:

Hi, blueboy. Thank you for reporting. Open your MapManager.h file and make following change #include "Platform/Define.h" #include "Policies/Singleton.h" #include "ace/Recursive_Thread_Mutex.h" <= make this change from ace/Thread_Mutex.h #include "Map.h" #include "GridStates.h" It should help.

Patch updated: fixed some compile errors related to const_iterator usage + MapInstanced.h/.cpp are removed from includes, .vcproj and Makefiles. Have fun

Guys, I suggest you to not miss this patch as it is essential to greatly improve mangos performance :rolleyes: Test branch2 first since we need to know how bigger the bonus it gives + it does not have issues with fast moving objects like branch1 does.

Final version of the patch is ready. Added timers to perform global garbage collection of GridMaps on different time for different mapIds - this is to avoid CPU usage peeks.