Performance: Optimize monster planning

[prharvey]

Summary

Performance "Reduced wait times in high traffic areas by ~15-20%"

Purpose of change

Waiting in TCL is slow, there were a few easy ways to make it less slow.

Describe the solution

Moved the reachability flood fill into map so it could be used in the monster planner to iterate over hostile monsters in O(monsters_in_zone) instead of O(all_monsters).

Describe alternatives you've considered

I originally was also using the zone reachability to optimize sees, but it ended up having no impact. I believe there are some other improvements that could be made to the zone reachability flood fill in general, but it's a pretty small part of the profile now. Worst part by far is the pathfinding.

Testing

Game doesn't crash and the tests pass. PR is still being worked on.

[kevingranade]

I was wondering if using reachable zones to populate the faction map would be a win.
I didn't follow up on it because by the time I landed the change the pertinent parts of monster:: plan() weren't even showing up in profiling any more, so I'm a bit confused what's happening that could yield a 30% speedup.

[prharvey]

I was wondering if using reachable zones to populate the faction map would be a win. I didn't follow up on it because by the time I landed the change the pertinent parts of monster:: plan() weren't even showing up in profiling any more, so I'm a bit confused what's happening that could yield a 30% speedup.

Essentially your change to add the flood fill didn't actually change the algorithmic complexity of the code, it just acted as a fast O(1) check to skip much more expensive O(1) processing. The monster planner as a whole was still O(n^2) where n is number of monsters, this uses the flood fill to turn it into O(n*m) where m is number of monsters in a given zone.

It's not responsible for all of the 30% time reduction (50% speed up), just the biggest part.

[prharvey]

Removed changes unrelated to the flood fill change. I'll PR them separately.

[andrei8l]

I'm not seeing any improvement when waiting in the TCL from this save Yelm.tar.gz

Before

After

But there's too much variance there, unrelated to monster planning.

Furthermore, monster::plan() barley registers when profiling visitable_zone_surface_test so that's not a good benchmark either

Can you update the test or provide a stable benchmark to illustrate the gains here?

[prharvey]

I'm not seeing any improvement when waiting in the TCL from this save Yelm.tar.gz

Before

After

But there's too much variance there, unrelated to monster planning.
Furthermore, monster::plan() barley registers when profiling visitable_zone_surface_test so that's not a good benchmark either

Can you update the test or provide a stable benchmark to illustrate the gains here?

Before

After

TCL Perf.zip

[andrei8l]

TCL Perf.zip

Sorry, I'm still not really seeing it. I had to wear a blindfold and ear plugs from your save because monsters wander into view.

Before

After

If there is an improvement there, it's masked by unrelated variance. A test unit would be best. Even running visitable_zone_surface_test 100 times might work.

This is with gcc 13.2.1 and and -Og build, btw.

[prharvey]

TCL Perf.zip

Sorry, I'm still not really seeing it. I had to wear a blindfold and ear plugs from your save because monsters wander into view.

Before

After

If there is an improvement there, it's masked by unrelated variance. A test unit would be best. Even running visitable_zone_surface_test 100 times might work.

This is with gcc 13.2.1 and and -Og build, btw.

I'm using a release build on windows with MSVC. Could be some compiler shenanigans.

CPU is a i9-13980HX.

[prharvey]

Using your save file:

Before

After

Results are very consistent for me. Are you sure it isn't something silly like you're accidently using the same binary each time?

[akrieger]

This causes a segfault on quit. g is null already so get_map() can't be called.

Creature::~Creature()
{
    get_map().remove_creature_from_reachability( this );
}


map &get_map()
{
    return g->m;
}

[akrieger]

Results wise, at least preliminarily, looks pretty good for me. Total cost across 3 hours was cut by about 25%. monster::plan's self cost was basically eliminated (total cost reduced from 23k to 12k samples, so cut in half basically). I can't tell though if that's just an inlining/outlining change, algorithmic, or something else entirely. For example, map visit reachable creatures was added, as well as a short circuit when the target is null.

This isn't just measurement error, other baseline functions like map::get_vehicles and map::process_fields_in_submap were basically constant.

[prharvey]

This seems to be good as is for now. I have a few improvements I want to make later, but right now it is blocking #69198 which has way more impact.