Add domain checks to HierarchicalPathFinder

[RoosterDragon]

The domains in HierarchicalPathFinder can be compared to find disjoint areas. For example islands on a water map will belong to different domains. Use these domains in path searches to allow us to bail out early if a path is impossible, e.g. trying to path between different islands. Keeping the domains updated via the RebuildDomains method adds some cost to the average path search, but that savings from path searches that can bail early pays for this many times over.

---

In the replay from #21132, the changes the time spent as follows:

Before

• HPF.FindPath = 6.71%
  • PathSearch.ExpandToTarget = 5.27%
  • HPF.RebuildDirtyGrids = 0.19%

After

• HPF.FindPath = 1.28%
  • PathSearch.FindBidiPath = 0.41%
  • HPF.RebuildDirtyGrids = 0.19%
  • HPF.RebuildDomains = 0.21%

The extra RebuildDomains call isn't free, but the path searches it allows short-circuiting more than pays for itself.

[pchote]

Code changes LGTM, and works as advertised on RA map Pool Party.

[anvilvapre]

Can I ask why you hadn't added this functionality before? Because you were afraid of the negative performance impact?

[RoosterDragon]

Can I ask why you hadn't added this functionality before? Because you were afraid of the negative performance impact?

Correct. Calling RebuildDomains is definitely an added cost, and if won't do get to bail early then it's added overhead for the times we still need to try and find a path without any other benefit. I previously hadn't tested it to see if the tradeoff would be worth it.

[pchote]

Shouldn't RebuildDomains end up being a no-op most of the time (when domains aren't dirty)?

[RoosterDragon]

Yes, most of the time it's a no-op. But when it isn't a no-op, it felt like it would be expensive because it clears and rebuilds domains for the whole map. Compare this to RebuildDirtyGrids which is smarter and does an incremental rebuild, so usually only affects a small portion of the map. On the plus side, the domains work at the abstract level so there's 100x less work to do compared to the grids which operate at cell level.

I was probably conditioned into this thinking. Rebuilding dirty grids for the whole map was horrendously expensive so I had to invest time writing the convoluted method to do incremental updates so it was fast enough. Having spent time writing that I wasn't keen to be doing any more refreshing-the-whole-map operations.

But the tests above show that the domain refresh happens to be about the same as the grid refresh in practise, so it turns out this is acceptable.

[pchote]

If performance did turn out to be a problem, would it be feasible to incrementally update the domains before we need to access them, using any spare time that might be left over at the end of several ticks?

[RoosterDragon]

Changing to an incremental update should reduce the cost by 10-100x and make the impact negligible. We don't even need to get fancy about using extra times between ticks. It'd just be complicated to write (the incremental update, that is).

[pchote]

(note: this discussion has now diverged from being relevant to this PR, but I think its useful to carry it through here as it may prompt future optimisation PRs).

Thinking out loud...

Assumption 1: A (the most?) common cause for dirtying the HPF is going to be resource patches growing/being mined, which changes the terrain type / pathing cost.

Assumption 2: Most (in the default mods, all) units have the same reachability for clear/resource cells, just different movement speeds.

We could therefore win a lot by checking the abstract edges before/after updating a dirty cell, and only clear the domain cache if connections are added or removed.

[RoosterDragon]

We could therefore win a lot by checking the abstract edges before/after updating a dirty cell, and only clear the domain cache if connections are added or removed.

Enterprising readers will find the relevant code in this method. We could compare old grid to new grid and only perform the RebuildCostTable and abstractDomains.Clear steps if the grid has changed if the abstract edges for the grid (GetAbstractEdgesForGrid) have changed.

OpenRA/OpenRA.Mods.Common/Pathfinder/HierarchicalPathFinder.cs

Lines 981 to 999 in 216758d

	void RebuildDirtyGrids()
	{
	if (dirtyGridIndexes.Count == 0)
	return;
	// An empty domain indicates it is out of date and will require rebuilding when next accessed.
	abstractDomains.Clear();
	var customMovementLayers = world.GetCustomMovementLayers();
	foreach (var gridIndex in dirtyGridIndexes)
	{
	var oldGrid = gridInfos[gridIndex];
	var gridTopLeft = GetGridTopLeft(gridIndex, 0);
	gridInfos[gridIndex] = BuildGrid(gridTopLeft.X, gridTopLeft.Y, customMovementLayers);
	RebuildCostTable(gridTopLeft.X, gridTopLeft.Y, oldGrid, customMovementLayers);
	}
	dirtyGridIndexes.Clear();
	}

[PunkPun]

LGTM

This removes the lag completely when units are ordered to attack inaccessible terrain, making #20954 pretty redundant and makes #20945 much less of a priority

[PunkPun]

Changelog