MoveMapGenerator: add proper multithreading support #2827

schell244 · 2024-11-08T12:15:50Z

🍰 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

None

mserajnik · 2024-11-08T12:41:32Z

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 · 2024-11-08T13:25:05Z

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

mserajnik · 2024-11-08T14:03:56Z

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

mserajnik · 2024-11-08T18:54:24Z

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:

I think the output of mmap_extract.py (that I implemented via Use concurrent.futures and improve user feedback in mmap_extract.py. #2559) is a better overview for the average end user than the elaborate MoveMapGenerator output
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 · 2024-11-08T19:44:19Z

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

mserajnik · 2024-11-08T20:39:42Z

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 · 2024-11-09T17:39:43Z

contrib/mmap/mmap_extract.py

@@ -115,7 +115,7 @@ def get_subprocess_config() -> SubprocessConfig:
 def run_move_map_gen(map_id: int) -> None:
    config = get_subprocess_config()
    subprocess.check_call(
-        (config.executable, str(map_id), "--silent", *sys.argv[1:]),
+        (config.executable, str(map_id), "--silent", "--threads", "1", *sys.argv[3:]),


*sys.argv[1:] has to stay as it is; this has nothing to do with the added --threads 1, it just passes along the arguments that the Python script itself was invoked with, e.g., when running it like this to select the configuration and offmesh:

/opt/vmangos/bin/Extractors/mmap_extract.py \ --configInputPath /opt/vmangos/bin/Extractors/config.json \ --offMeshInput /opt/vmangos/bin/Extractors/offmesh.txt

mserajnik · 2024-11-09T17:47:29Z

@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 · 2024-11-09T18:12:25Z

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 · 2024-11-09T18:23:57Z

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 · 2024-11-11T21:25:27Z

@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 · 2024-11-16T14:35:32Z

@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 · 2024-11-16T15:21:02Z

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 · 2024-11-17T17:03:02Z

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 · 2024-11-17T20:35:13Z

@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 · 2024-11-17T20:57:54Z

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

MoveMapGenerator: add proper multithreading support

263f258

0blu added the CPP A issue / PR which references CPP code label Nov 8, 2024

schell244 added 2 commits November 8, 2024 14:12

skip asking user for threads when using mmap_extract.py

6e67316

fix build

69e5ad6

add --threads option

3f1c5da

mserajnik reviewed Nov 9, 2024

View reviewed changes

restore *sys.argv[1:]

d20fec9

fix broken formating

c6b7789

Fix building a single map

97d12a6

ratkosrb merged commit faea064 into vmangos:development Nov 19, 2024
3 checks passed

schell244 deleted the mmap_multi_threading branch November 20, 2024 13:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MoveMapGenerator: add proper multithreading support #2827

MoveMapGenerator: add proper multithreading support #2827

schell244 commented Nov 8, 2024 •

edited

Loading

mserajnik commented Nov 8, 2024

schell244 commented Nov 8, 2024

mserajnik commented Nov 8, 2024

mserajnik commented Nov 8, 2024

0blu commented Nov 8, 2024 •

edited

Loading

mserajnik commented Nov 8, 2024

mserajnik Nov 9, 2024 •

edited

Loading

mserajnik commented Nov 9, 2024

schell244 commented Nov 9, 2024 •

edited

Loading

mserajnik commented Nov 9, 2024

mserajnik commented Nov 11, 2024 •

edited

Loading

mserajnik commented Nov 16, 2024 •

edited

Loading

schell244 commented Nov 16, 2024

schell244 commented Nov 17, 2024 •

edited

Loading

mserajnik commented Nov 17, 2024 •

edited

Loading

schell244 commented Nov 17, 2024

MoveMapGenerator: add proper multithreading support #2827

MoveMapGenerator: add proper multithreading support #2827

Conversation

schell244 commented Nov 8, 2024 • edited Loading

🍰 Pullrequest

How2Test

Todo / Checklist

mserajnik commented Nov 8, 2024

schell244 commented Nov 8, 2024

mserajnik commented Nov 8, 2024

mserajnik commented Nov 8, 2024

0blu commented Nov 8, 2024 • edited Loading

mserajnik commented Nov 8, 2024

mserajnik Nov 9, 2024 • edited Loading

Choose a reason for hiding this comment

mserajnik commented Nov 9, 2024

schell244 commented Nov 9, 2024 • edited Loading

mserajnik commented Nov 9, 2024

mserajnik commented Nov 11, 2024 • edited Loading

mserajnik commented Nov 16, 2024 • edited Loading

schell244 commented Nov 16, 2024

schell244 commented Nov 17, 2024 • edited Loading

mserajnik commented Nov 17, 2024 • edited Loading

schell244 commented Nov 17, 2024

schell244 commented Nov 8, 2024 •

edited

Loading

0blu commented Nov 8, 2024 •

edited

Loading

mserajnik Nov 9, 2024 •

edited

Loading

schell244 commented Nov 9, 2024 •

edited

Loading

mserajnik commented Nov 11, 2024 •

edited

Loading

mserajnik commented Nov 16, 2024 •

edited

Loading

schell244 commented Nov 17, 2024 •

edited

Loading

mserajnik commented Nov 17, 2024 •

edited

Loading