MoveMapGenerator: add proper multithreading support

[schell244]

🍰 Pullrequest

Allow mmap extraction to run properly multi threaded. No changes have been made to the extraction logic itself, some funtions where just moved into TileWorker to run asynchronous. Depending on the cpu, mmaps can now be build in <1 hour.

How2Test

• start mmap extraction as usual
• select number of threads to be used
• resulting mmaps are the same as without those changes

Todo / Checklist

• [X] None

[mserajnik]

I guess when it’s run from mmap_extract.py it shouldn’t prompt the user but instead just select 1 thread automatically (since mmap_extract.py already spawns multiple workers).

[schell244]

@mserajnik can you maybe give it try and confirm that mmap_extract.py now works as expected?

[mserajnik]

@schell244 Currently not at home, but I’ll give it a try later today or tomorrow at the latest!

[mserajnik]

I'm building Docker images off of this PR right now and will test it tomorrow morning.

In the meantime, because it came to my mind: one could make the argument to remove the mmap_extract.py script altogether if MoveMapGenerator now supports multi-threading itself. However, I would keep it for two reasons:

1. I think the output of mmap_extract.py (that I implemented via #2559) is a better overview for the average end user than the elaborate MoveMapGenerator output
2. Removing the script now would potentially break VMaNGOS-related projects that rely on it (e.g., I use it for my Docker setup; of course, I monitor VMaNGOS development closely and could easily adjust it, but not everyone might do that)

[0blu]

Yeah, maybe instead of --skipPromtCores make it --threads <num>. So the python script can just forward the count.

[mserajnik]

Yeah, maybe instead of --skipPromtCores make it --threads <num>. So the python script can just forward the count.

The Python script would need to use --threads 1 still then, because it determines the maps beforehand and then spawns a MoveMapGenerator process for each map, so it already implements multithreading on its own. Changing this would be possible, but that would make it impossible/hard to produce the custom output as it does now, so personally I wouldn't do that.

However, other than that I quite like the idea; so if you pass --threads <num> it just uses that number and skips the prompt. That would be a lot more flexible (especially for scripting) than --skipPromtCores (should probably be Prompt, by the way, there's a typo) because then you can also do --threads $(nproc) and such.

[mserajnik]

@schell244

First of all, I added a comment (see above) because you cut off the Python script arguments with your change (that should be fixed), but that's probably not the root cause of the following:

I just tried it, but unfortunately it's not working when running mmap_extract.py now, MoveMapGenerator doesn't seem to actually run. I don't have time to properly debug this today, but my guess is that even with the passed --threads 1 it might be waiting for user input before continuing, because the last output I see from the Python script is this print.

Can you confirm that MoveMapGenerator works if you pass --threads 1 directly (without invoking it through the Python script)?

[schell244]

Thanks a lots for testing it! I'll adapt the Python script arguments accordingly. I just re-checked starting the process from windows cmd with MoveMapGenerator.exe --threads 1 and it works for me:

D:\Downloads>MoveMapGenerator.exe --silent --threads 12 --configInputPath "D:\Downloads\config.json" --offMeshInput "D:\Downloads\offmesh.txt"
MMap Generator
====================================
offMeshInputPath = D:\Downloads\offmesh.txt
configInputPath = D:\Downloads\config.json
Discovering maps... found 43.
Discovering tiles... found 2429.

Building map 000:
[Map 000] Creating navMesh [maxTiles=687]
[Map 000] We have 687 tiles.
...

[mserajnik]

I just re-checked starting the process from windows cmd with MoveMapGenerator.exe --threads 1 and it works for me

Thanks for confirming, I'll retest this again then (tomorrow probably) with the *sys.argv[1:] fix and see if I can't figure it out.

[mserajnik]

@schell244

I've tried again, with the same result. I can see the mmap_extract.py script spawning the MoveMapGenerator "workers" (8, in my case): But then nothing happens (and you can see, they basically don't use any CPU or memory); as if they were waiting for user input, which of course shouldn't happen with --threads 1.

---

The script effectively spawns each MoveMapGenerator worker like this (34 being an example map ID here):

MoveMapGenerator 34 --silent --threads 1 --configInputPath /path/to/config.json --offMeshInput /path/to/offmesh.txt

Can you see anything wrong with that in regards to your changes?

---

I'm a bit short on time at the moment, but tomorrow I'll hopefully get around to try not swallowing the output from MoveMapGenerator in the Python script (it's getting swallowed for cosmetic reasons, the script generates its own "friendlier" output, as mentioned before) so I can maybe see better what's going on here.

[mserajnik]

@schell244

I'll hopefully get around to try not swallowing the output from MoveMapGenerator in the Python script

I was finally able to try that now and can see the following output:

MMap Generator
MMap Generator
MMap Generator
====================================
====================================
====================================
MMap Generator
====================================
MMap Generator
====================================
MMap Generator
====================================
MMap Generator
====================================
MMap Generator
====================================
offMeshInputPath = /opt/vmangos/bin/Extractors/offmesh.txt
configInputPath = /opt/vmangos/bin/Extractors/config.json
offMeshInputPath = /opt/vmangos/bin/Extractors/offmesh.txt
configInputPath = /opt/vmangos/bin/Extractors/config.json
offMeshInputPath = /opt/vmangos/bin/Extractors/offmesh.txt
configInputPath = /opt/vmangos/bin/Extractors/config.json
offMeshInputPath = /opt/vmangos/bin/Extractors/offmesh.txt
offMeshInputPath = /opt/vmangos/bin/Extractors/offmesh.txt
configInputPath = /opt/vmangos/bin/Extractors/config.json
configInputPath = /opt/vmangos/bin/Extractors/config.json
offMeshInputPath = /opt/vmangos/bin/Extractors/offmesh.txt
offMeshInputPath = /opt/vmangos/bin/Extractors/offmesh.txt
configInputPath = /opt/vmangos/bin/Extractors/config.json
configInputPath = /opt/vmangos/bin/Extractors/config.json
offMeshInputPath = /opt/vmangos/bin/Extractors/offmesh.txt
configInputPath = /opt/vmangos/bin/Extractors/config.json

So each of the 8 MoveMapGenerator "workers" the Python script starts simulatenously logs:

MMap Generator
====================================
offMeshInputPath = /opt/vmangos/bin/Extractors/offmesh.txt
configInputPath = /opt/vmangos/bin/Extractors/config.json

(On a sidenote, it probably shouldn't log anything with the --silent flag provided)

If I remove the --silent I can actually see that the maps are being processed, e.g.,:

MMap Generator
====================================
offMeshInputPath = /opt/vmangos/bin/Extractors/offmesh.txt
configInputPath = /opt/vmangos/bin/Extractors/config.json
Discovering maps... found 43.
Discovering tiles... found 2429.

Building map 034:                                    
[Map 034] Creating navMesh [maxTiles=4]
[Map 034] We have 4 tiles.                          
[Map 034] Complete!    

But MoveMapGenerator doesn't exit afterwards, which is likely what's causing the issue here as the Python script is waiting for MoveMapGenerator to terminate. I confirmed it also doesn't exit when running it manually (so not spawned by the Python script).

Edit 1: Nevermind, I think I forgot it should still be processing at this point (so that's why it doesn't exit); but it seems to hang with no resource usage after these log lines (also when running it manually).

Edit 2: Actually, looking at the code, it seems like it should be done processing here after all and should exit; so maybe my first statement was right after all. I'm just confusing myself at this point, so I'll leave it at that. 😆 I'm also not sure if it actually processes the map for me because if I try it with map 1 and --threads 1 it prints [Map 001] Complete! basically immediately, when before this used to take like an hour or so for map 1 on the machine I tested this.

So I'm not sure why this is working for you but not for me; it has always worked fine before these changes. Let me know if you want me to try something specific to debug this.

[schell244]

Thanks a lot for the testing efforts! I am currently also a bit time-constrained, but I will take care of it in the next days and will try to figure out whats causing problems.

[schell244]

Hi @mserajnik, I've indeed missed some changes in order to build single maps. Also I've turned off some logs, when --silent option is used. (It's also possible now to build a single map with more than 1 thread, like when you build a big map like 0 or 1)

[mserajnik]

@schell244

Thanks for the fixes, it seems to work fine now:

---

It's also possible now to build a single map with more than 1 thread, like when you build a big map like 0 or 1

I think that's great for rebuilding specific maps! 👍 Let's leave it at --threads 1 for the mmap_extract.py script though since that already spawns one MoveMapGenerator process per logical CPU (which process one map each) and since we're already at 100% CPU utilization it's probably not beneficial for performance to use even more threads here.

[schell244]

Cool, thanks a lot for your help getting this right!