Merge pull request #9141 from RoosterDragon/pathfinder-perf

Improve pathfinder performance (paths may change)

OpenRA.Mods.Common/OpenRA.Mods.Common.csproj

@@ -504,7 +504,7 @@
     <Compile Include="Pathfinder\CellInfoLayerManager.cs" />
     <Compile Include="Pathfinder\Constants.cs" />
     <Compile Include="Pathfinder\PathGraph.cs" />
-    <Compile Include="Pathfinder\PathFinderCacheDecorator.cs" />
+    <Compile Include="Pathfinder\PathFinderUnitPathCacheDecorator.cs" />
     <Compile Include="Pathfinder\PathCacheStorage.cs" />
     <Compile Include="Traits\World\PathFinder.cs" />
     <Compile Include="Traits\World\PathfinderDebugOverlay.cs" />

OpenRA.Mods.Common/Pathfinder/BasePathSearch.cs

@@ -18,18 +18,11 @@ namespace OpenRA.Mods.Common.Pathfinder
 {
 	public interface IPathSearch
 	{
-		string Id { get; }
-
 		/// <summary>
 		/// The Graph used by the A*
 		/// </summary>
 		IGraph<CellInfo> Graph { get; }
 
-		/// <summary>
-		/// The open queue where nodes that are worth to consider are stored by their estimator
-		/// </summary>
-		IPriorityQueue<CPos> OpenQueue { get; }
-
 		/// <summary>
 		/// Stores the analyzed nodes by the expand function
 		/// </summary>
@@ -64,6 +57,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 		/// <returns>Whether the location is a target</returns>
 		bool IsTarget(CPos location);
 
+		bool CanExpand { get; }
 		CPos Expand();
 	}
 
@@ -71,44 +65,13 @@ namespace OpenRA.Mods.Common.Pathfinder
 	{
 		public IGraph<CellInfo> Graph { get; set; }
 
-		// The Id of a Pathsearch is computed by its properties.
+		protected IPriorityQueue<GraphConnection> OpenQueue { get; private set; }
-		// So two PathSearch instances with the same parameters will
-		// Compute the same Id. This is used for caching purposes.
-		public string Id
-		{
-			get
-			{
-				if (string.IsNullOrEmpty(id))
-				{
-					var builder = new StringBuilder();
-					builder.Append(Graph.Actor.ActorID);
-					while (!startPoints.Empty)
-					{
-						var startpoint = startPoints.Pop();
-						builder.Append(startpoint.X);
-						builder.Append(startpoint.Y);
-						builder.Append(Graph[startpoint].EstimatedTotal);
-					}
-
-					builder.Append(Graph.InReverse);
-					if (Graph.IgnoredActor != null) builder.Append(Graph.IgnoredActor.ActorID);
-					builder.Append(Graph.LaneBias);
-
-					id = builder.ToString();
-				}
-
-				return id;
-			}
-		}
-
-		public IPriorityQueue<CPos> OpenQueue { get; protected set; }
 
 		public abstract IEnumerable<Pair<CPos, int>> Considered { get; }
 
 		public Player Owner { get { return Graph.Actor.Owner; } }
 		public int MaxCost { get; protected set; }
 		public bool Debug { get; set; }
-		string id;
 		protected Func<CPos, int> heuristic;
 		protected Func<CPos, bool> isGoal;
 
@@ -117,13 +80,13 @@ namespace OpenRA.Mods.Common.Pathfinder
 		// points considered and their Heuristics to reach
 		// the target. It pretty match identifies, in conjunction of the Actor,
 		// a deterministic set of calculations
-		protected IPriorityQueue<CPos> startPoints;
+		protected readonly IPriorityQueue<GraphConnection> StartPoints;
 
 		protected BasePathSearch(IGraph<CellInfo> graph)
 		{
 			Graph = graph;
-			OpenQueue = new PriorityQueue<CPos>(new PositionComparer(Graph));
+			OpenQueue = new PriorityQueue<GraphConnection>(GraphConnection.ConnectionCostComparer);
-			startPoints = new PriorityQueue<CPos>(new PositionComparer(Graph));
+			StartPoints = new PriorityQueue<GraphConnection>(GraphConnection.ConnectionCostComparer);
 			Debug = false;
 			MaxCost = 0;
 		}
@@ -198,6 +161,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 			return isGoal(location);
 		}
 
+		public bool CanExpand { get { return !OpenQueue.Empty; } }
 		public abstract CPos Expand();
 	}
 }

OpenRA.Mods.Common/Pathfinder/CellInfo.cs

@@ -57,22 +57,4 @@ namespace OpenRA.Mods.Common.Pathfinder
 			EstimatedTotal = estimatedTotal;
 		}
 	}
-
-	/// <summary>
-	/// Compares two nodes according to their estimations
-	/// </summary>
-	public class PositionComparer : IComparer<CPos>
-	{
-		readonly IGraph<CellInfo> graph;
-
-		public PositionComparer(IGraph<CellInfo> graph)
-		{
-			this.graph = graph;
-		}
-
-		public int Compare(CPos x, CPos y)
-		{
-			return Math.Sign(graph[x].EstimatedTotal - graph[y].EstimatedTotal);
-		}
-	}
 }

OpenRA.Mods.Common/Pathfinder/PathFinderUnitPathCacheDecorator.cs

@@ -16,14 +16,14 @@ using OpenRA.Traits;
 namespace OpenRA.Mods.Common.Pathfinder
 {
 	/// <summary>
-	/// A decorator used to cache the pathfinder (Decorator design pattern)
+	/// A decorator used to cache FindUnitPath and FindUnitPathToRange (Decorator design pattern)
 	/// </summary>
-	public class PathFinderCacheDecorator : IPathFinder
+	public class PathFinderUnitPathCacheDecorator : IPathFinder
 	{
 		readonly IPathFinder pathFinder;
 		readonly ICacheStorage<List<CPos>> cacheStorage;
 
-		public PathFinderCacheDecorator(IPathFinder pathFinder, ICacheStorage<List<CPos>> cacheStorage)
+		public PathFinderUnitPathCacheDecorator(IPathFinder pathFinder, ICacheStorage<List<CPos>> cacheStorage)
 		{
 			this.pathFinder = pathFinder;
 			this.cacheStorage = cacheStorage;
@@ -68,37 +68,13 @@ namespace OpenRA.Mods.Common.Pathfinder
 		public List<CPos> FindPath(IPathSearch search)
 		{
 			using (new PerfSample("Pathfinder"))
-			{
+				return pathFinder.FindPath(search);
-				var key = "FindPath" + search.Id;
-				var cachedPath = cacheStorage.Retrieve(key);
-
-				if (cachedPath != null)
-					return cachedPath;
-
-				var pb = pathFinder.FindPath(search);
-
-				cacheStorage.Store(key, pb);
-
-				return pb;
-			}
 		}
 
 		public List<CPos> FindBidiPath(IPathSearch fromSrc, IPathSearch fromDest)
 		{
 			using (new PerfSample("Pathfinder"))
-			{
+				return pathFinder.FindBidiPath(fromSrc, fromDest);
-				var key = "FindBidiPath" + fromSrc.Id + fromDest.Id;
-				var cachedPath = cacheStorage.Retrieve(key);
-
-				if (cachedPath != null)
-					return cachedPath;
-
-				var pb = pathFinder.FindBidiPath(fromSrc, fromDest);
-
-				cacheStorage.Store(key, pb);
-
-				return pb;
-			}
 		}
 	}
 }

OpenRA.Mods.Common/Pathfinder/PathGraph.cs

@@ -47,6 +47,18 @@ namespace OpenRA.Mods.Common.Pathfinder
 
 	public struct GraphConnection
 	{
+		public static readonly CostComparer ConnectionCostComparer = CostComparer.Instance;
+
+		public sealed class CostComparer : IComparer<GraphConnection>
+		{
+			public static readonly CostComparer Instance = new CostComparer();
+			CostComparer() { }
+			public int Compare(GraphConnection x, GraphConnection y)
+			{
+				return x.Cost.CompareTo(y.Cost);
+			}
+		}
+
 		public readonly CPos Destination;
 		public readonly int Cost;
 

OpenRA.Mods.Common/Pathfinder/PathSearch.cs

@@ -84,9 +84,11 @@ namespace OpenRA.Mods.Common.Pathfinder
 
 		protected override void AddInitialCell(CPos location)
 		{
-			Graph[location] = new CellInfo(0, heuristic(location), location, CellStatus.Open);
+			var cost = heuristic(location);
-			OpenQueue.Add(location);
+			Graph[location] = new CellInfo(0, cost, location, CellStatus.Open);
-			startPoints.Add(location);
+			var connection = new GraphConnection(location, cost);
+			OpenQueue.Add(connection);
+			StartPoints.Add(connection);
 			considered.AddLast(new Pair<CPos, int>(location, 0));
 		}
 
@@ -99,7 +101,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 		/// <returns>The most promising node of the iteration</returns>
 		public override CPos Expand()
 		{
-			var currentMinNode = OpenQueue.Pop();
+			var currentMinNode = OpenQueue.Pop().Destination;
 
 			var currentCell = Graph[currentMinNode];
 			Graph[currentMinNode] = new CellInfo(currentCell.CostSoFar, currentCell.EstimatedTotal, currentCell.PreviousPos, CellStatus.Closed);
@@ -128,10 +130,11 @@ namespace OpenRA.Mods.Common.Pathfinder
 				else
 					hCost = heuristic(neighborCPos);
 
-				Graph[neighborCPos] = new CellInfo(gCost, gCost + hCost, currentMinNode, CellStatus.Open);
+				var estimatedCost = gCost + hCost;
+				Graph[neighborCPos] = new CellInfo(gCost, estimatedCost, currentMinNode, CellStatus.Open);
 
 				if (neighborCell.Status != CellStatus.Open)
-					OpenQueue.Add(neighborCPos);
+					OpenQueue.Add(new GraphConnection(neighborCPos, estimatedCost));
 
 				if (Debug)
 				{

OpenRA.Mods.Common/Traits/World/PathFinder.cs

@@ -22,7 +22,7 @@ namespace OpenRA.Mods.Common.Traits
 	{
 		public object Create(ActorInitializer init)
 		{
-			return new PathFinderCacheDecorator(new PathFinder(init.World), new PathCacheStorage(init.World));
+			return new PathFinderUnitPathCacheDecorator(new PathFinder(init.World), new PathCacheStorage(init.World));
 		}
 	}
 
@@ -121,7 +121,7 @@ namespace OpenRA.Mods.Common.Traits
 
 			List<CPos> path = null;
 
-			while (!search.OpenQueue.Empty)
+			while (search.CanExpand)
 			{
 				var p = search.Expand();
 				if (search.IsTarget(p))
@@ -156,7 +156,7 @@ namespace OpenRA.Mods.Common.Traits
 				fromDest.Debug = true;
 			}
 
-			while (!fromSrc.OpenQueue.Empty && !fromDest.OpenQueue.Empty)
+			while (fromSrc.CanExpand && fromDest.CanExpand)
 			{
 				// make some progress on the first search
 				var p = fromSrc.Expand();