Merge pull request #9359 from RoosterDragon/fix-pathfinder-layer-pooling

Fixed pooling of layers used for pathfinding

OpenRA.Mods.Common/Activities/FindResources.cs

@@ -123,8 +123,10 @@ namespace OpenRA.Mods.Common.Activities
 			var searchRadius = harv.LastOrderLocation.HasValue ? harvInfo.SearchFromOrderRadius : harvInfo.SearchFromProcRadius;
 			var searchRadiusSquared = searchRadius * searchRadius;
 
+			// Find any harvestable resources:
 			var passable = (uint)mobileInfo.GetMovementClass(self.World.TileSet);
-			var search = PathSearch.Search(self.World, mobileInfo, self, true,
+			List<CPos> path;
+			using (var search = PathSearch.Search(self.World, mobileInfo, self, true,
 				loc => domainIndex.IsPassable(self.Location, loc, passable) && self.CanHarvestAt(loc, resLayer, harvInfo, territory))
 				.WithCustomCost(loc =>
 				{
@@ -135,10 +137,8 @@ namespace OpenRA.Mods.Common.Activities
 					return 0;
 				})
 				.FromPoint(self.Location)
-				.FromPoint(searchFromLoc);
+				.FromPoint(searchFromLoc))
-
+				path = pathFinder.FindPath(search);
-			// Find any harvestable resources:
-			var path = pathFinder.FindPath(search);
 
 			if (path.Count > 0)
 				return path[0];

OpenRA.Mods.Common/Activities/Move/Move.cs

@@ -46,9 +46,14 @@ namespace OpenRA.Mods.Common.Activities
 			moveDisablers = self.TraitsImplementing<IDisableMove>().ToArray();
 
 			getPath = () =>
-				self.World.WorldActor.Trait<IPathFinder>().FindPath(
+			{
+				List<CPos> path;
+				using (var search =
 					PathSearch.FromPoint(self.World, mobile.Info, self, mobile.ToCell, destination, false)
-					.WithoutLaneBias());
+					.WithoutLaneBias())
+					path = self.World.WorldActor.Trait<IPathFinder>().FindPath(search);
+				return path;
+			};
 			this.destination = destination;
 			this.nearEnough = WDist.Zero;
 		}
@@ -85,9 +90,14 @@ namespace OpenRA.Mods.Common.Activities
 			moveDisablers = self.TraitsImplementing<IDisableMove>().ToArray();
 
 			getPath = () =>
-				self.World.WorldActor.Trait<IPathFinder>().FindPath(
+			{
+				List<CPos> path;
+				using (var search =
 					PathSearch.FromPoint(self.World, mobile.Info, self, mobile.ToCell, destination, false)
-					.WithIgnoredActor(ignoredActor));
+					.WithIgnoredActor(ignoredActor))
+					path = self.World.WorldActor.Trait<IPathFinder>().FindPath(search);
+				return path;
+			};
 
 			this.destination = destination;
 			this.nearEnough = WDist.Zero;

OpenRA.Mods.Common/Activities/Move/MoveAdjacentTo.cs

@@ -148,10 +148,9 @@ namespace OpenRA.Mods.Common.Activities
 			if (!searchCells.Any())
 				return NoPath;
 
-			var fromSrc = PathSearch.FromPoints(self.World, mobile.Info, self, searchCells, loc, true);
+			using (var fromSrc = PathSearch.FromPoints(self.World, mobile.Info, self, searchCells, loc, true))
-			var fromDest = PathSearch.FromPoint(self.World, mobile.Info, self, loc, targetPosition, true).Reverse();
+			using (var fromDest = PathSearch.FromPoint(self.World, mobile.Info, self, loc, targetPosition, true).Reverse())
-
+				return pathFinder.FindBidiPath(fromSrc, fromDest);
-			return pathFinder.FindBidiPath(fromSrc, fromDest);
 		}
 
 		public override IEnumerable<Target> GetTargets(Actor self)

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

@@ -503,7 +503,7 @@
     <Compile Include="Traits\World\PaletteFromFile.cs" />
     <Compile Include="Traits\World\PaletteFromRGBA.cs" />
     <Compile Include="Traits\World\VoxelNormalsPalette.cs" />
-    <Compile Include="Pathfinder\CellInfoLayerManager.cs" />
+    <Compile Include="Pathfinder\CellInfoLayerPool.cs" />
     <Compile Include="Pathfinder\Constants.cs" />
     <Compile Include="Pathfinder\PathGraph.cs" />
     <Compile Include="Pathfinder\PathFinderUnitPathCacheDecorator.cs" />

OpenRA.Mods.Common/Pathfinder/BasePathSearch.cs

@@ -11,12 +11,11 @@
 using System;
 using System.Collections.Generic;
 using System.Text;
-using OpenRA.Mods.Common.Traits;
 using OpenRA.Primitives;
 
 namespace OpenRA.Mods.Common.Pathfinder
 {
-	public interface IPathSearch
+	public interface IPathSearch : IDisposable
 	{
 		/// <summary>
 		/// The Graph used by the A*
@@ -163,5 +162,17 @@ namespace OpenRA.Mods.Common.Pathfinder
 
 		public bool CanExpand { get { return !OpenQueue.Empty; } }
 		public abstract CPos Expand();
+
+		protected virtual void Dispose(bool disposing)
+		{
+			if (disposing)
+				Graph.Dispose();
+		}
+
+		public void Dispose()
+		{
+			Dispose(true);
+			GC.SuppressFinalize(this);
+		}
 	}
 }

OpenRA.Mods.Common/Pathfinder/CellInfoLayerManager.cs

@@ -1,111 +0,0 @@
-#region Copyright & License Information
-/*
- * Copyright 2007-2015 The OpenRA Developers (see AUTHORS)
- * This file is part of OpenRA, which is free software. It is made
- * available to you under the terms of the GNU General Public License
- * as published by the Free Software Foundation. For more information,
- * see COPYING.
- */
-#endregion
-
-using System.Collections.Generic;
-using System.Drawing;
-using OpenRA.Mods.Common.Traits;
-
-namespace OpenRA.Mods.Common.Pathfinder
-{
-	public interface ICellInfoLayerManager
-	{
-		/// <summary>
-		/// Gets a CellLayer of Nodes from the pool
-		/// </summary>
-		CellLayer<CellInfo> GetFromPool();
-
-		/// <summary>
-		/// Puts a CellLayer into the pool
-		/// </summary>
-		void PutBackIntoPool(CellLayer<CellInfo> ci);
-
-		/// <summary>
-		/// Creates (or obtains from the pool) a CellLayer given a map
-		/// </summary>
-		CellLayer<CellInfo> NewLayer(Map map);
-	}
-
-	public sealed class CellInfoLayerManager : ICellInfoLayerManager
-	{
-		readonly Queue<CellLayer<CellInfo>> cellInfoPool = new Queue<CellLayer<CellInfo>>();
-		readonly object defaultCellInfoLayerSync = new object();
-		CellLayer<CellInfo> defaultCellInfoLayer;
-
-		static ICellInfoLayerManager instance = new CellInfoLayerManager();
-
-		public static ICellInfoLayerManager Instance
-		{
-			get
-			{
-				return instance;
-			}
-		}
-
-		public static void SetInstance(ICellInfoLayerManager cellInfoLayerManager)
-		{
-			instance = cellInfoLayerManager;
-		}
-
-		public CellLayer<CellInfo> GetFromPool()
-		{
-			lock (cellInfoPool)
-				return cellInfoPool.Dequeue();
-		}
-
-		public void PutBackIntoPool(CellLayer<CellInfo> ci)
-		{
-			lock (cellInfoPool)
-				cellInfoPool.Enqueue(ci);
-		}
-
-		public CellLayer<CellInfo> NewLayer(Map map)
-		{
-			CellLayer<CellInfo> result = null;
-			var mapSize = new Size(map.MapSize.X, map.MapSize.Y);
-
-			// HACK: Uses a static cache so that double-ended searches (which have two PathSearch instances)
-			// can implicitly share data.  The PathFinder should allocate the CellInfo array and pass it
-			// explicitly to the things that need to share it.
-			while (cellInfoPool.Count > 0)
-			{
-				var cellInfo = GetFromPool();
-				if (cellInfo.Size != mapSize || cellInfo.Shape != map.TileShape)
-				{
-					Log.Write("debug", "Discarding old pooled CellInfo of wrong size.");
-					continue;
-				}
-
-				result = cellInfo;
-				break;
-			}
-
-			if (result == null)
-				result = new CellLayer<CellInfo>(map);
-
-			lock (defaultCellInfoLayerSync)
-			{
-				if (defaultCellInfoLayer == null ||
-				    defaultCellInfoLayer.Size != mapSize ||
-				    defaultCellInfoLayer.Shape != map.TileShape)
-				{
-					defaultCellInfoLayer =
-						CellLayer<CellInfo>.CreateInstance(
-							mpos => new CellInfo(int.MaxValue, int.MaxValue, mpos.ToCPos(map), CellStatus.Unvisited),
-							new Size(map.MapSize.X, map.MapSize.Y),
-							map.TileShape);
-				}
-
-				result.CopyValuesFrom(defaultCellInfoLayer);
-			}
-
-			return result;
-		}
-	}
-}

OpenRA.Mods.Common/Pathfinder/CellInfoLayerPool.cs

@@ -0,0 +1,78 @@
+#region Copyright & License Information
+/*
+ * Copyright 2007-2015 The OpenRA Developers (see AUTHORS)
+ * This file is part of OpenRA, which is free software. It is made
+ * available to you under the terms of the GNU General Public License
+ * as published by the Free Software Foundation. For more information,
+ * see COPYING.
+ */
+#endregion
+
+using System;
+using System.Collections.Generic;
+using System.Drawing;
+
+namespace OpenRA.Mods.Common.Pathfinder
+{
+	sealed class CellInfoLayerPool
+	{
+		const int MaxPoolSize = 4;
+		readonly Stack<CellLayer<CellInfo>> pool = new Stack<CellLayer<CellInfo>>(MaxPoolSize);
+		readonly CellLayer<CellInfo> defaultLayer;
+
+		public CellInfoLayerPool(Map map)
+		{
+			defaultLayer =
+				CellLayer<CellInfo>.CreateInstance(
+					mpos => new CellInfo(int.MaxValue, int.MaxValue, mpos.ToCPos(map), CellStatus.Unvisited),
+					new Size(map.MapSize.X, map.MapSize.Y),
+					map.TileShape);
+		}
+
+		public PooledCellInfoLayer Get()
+		{
+			return new PooledCellInfoLayer(this);
+		}
+
+		CellLayer<CellInfo> GetLayer()
+		{
+			CellLayer<CellInfo> layer = null;
+			lock (pool)
+				if (pool.Count > 0)
+					layer = pool.Pop();
+
+			if (layer == null)
+				layer = new CellLayer<CellInfo>(defaultLayer.Shape, defaultLayer.Size);
+			layer.CopyValuesFrom(defaultLayer);
+			return layer;
+		}
+
+		void ReturnLayer(CellLayer<CellInfo> layer)
+		{
+			lock (pool)
+			   if (pool.Count < MaxPoolSize)
+					pool.Push(layer);
+		}
+
+		public class PooledCellInfoLayer : IDisposable
+		{
+			public CellLayer<CellInfo> Layer { get; private set; }
+			CellInfoLayerPool layerPool;
+
+			public PooledCellInfoLayer(CellInfoLayerPool layerPool)
+			{
+				this.layerPool = layerPool;
+				Layer = layerPool.GetLayer();
+			}
+
+			public void Dispose()
+			{
+				if (Layer == null)
+					return;
+				layerPool.ReturnLayer(Layer);
+				Layer = null;
+				layerPool = null;
+			}
+		}
+	}
+}

OpenRA.Mods.Common/Pathfinder/PathGraph.cs

@@ -69,7 +69,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 		}
 	}
 
-	public class PathGraph : IGraph<CellInfo>
+	sealed class PathGraph : IGraph<CellInfo>
 	{
 		public Actor Actor { get; private set; }
 		public World World { get; private set; }
@@ -82,11 +82,13 @@ namespace OpenRA.Mods.Common.Pathfinder
 		readonly CellConditions checkConditions;
 		readonly MobileInfo mobileInfo;
 		readonly MobileInfo.WorldMovementInfo worldMovementInfo;
+		readonly CellInfoLayerPool.PooledCellInfoLayer pooledLayer;
 		CellLayer<CellInfo> cellInfo;
 
-		public PathGraph(CellLayer<CellInfo> cellInfo, MobileInfo mobileInfo, Actor actor, World world, bool checkForBlocked)
+		public PathGraph(CellInfoLayerPool layerPool, MobileInfo mobileInfo, Actor actor, World world, bool checkForBlocked)
 		{
-			this.cellInfo = cellInfo;
+			pooledLayer = layerPool.Get();
+			cellInfo = pooledLayer.Layer;
 			World = world;
 			this.mobileInfo = mobileInfo;
 			worldMovementInfo = mobileInfo.GetWorldMovementInfo(world);
@@ -174,26 +176,16 @@ namespace OpenRA.Mods.Common.Pathfinder
 			return cellCost;
 		}
 
-		bool disposed;
-		public void Dispose()
-		{
-			if (disposed)
-				return;
-
-			disposed = true;
-
-			CellInfoLayerManager.Instance.PutBackIntoPool(cellInfo);
-			cellInfo = null;
-
-			GC.SuppressFinalize(this);
-		}
-
-		~PathGraph() { Dispose(); }
-
 		public CellInfo this[CPos pos]
 		{
 			get { return cellInfo[pos]; }
 			set { cellInfo[pos] = value; }
 		}
+
+		public void Dispose()
+		{
+			pooledLayer.Dispose();
+			cellInfo = null;
+		}
 	}
 }

OpenRA.Mods.Common/Pathfinder/PathSearch.cs

@@ -11,6 +11,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using OpenRA.Mods.Common.Traits;
 using OpenRA.Primitives;
 
@@ -18,6 +19,14 @@ namespace OpenRA.Mods.Common.Pathfinder
 {
 	public sealed class PathSearch : BasePathSearch
 	{
+		static readonly ConditionalWeakTable<World, CellInfoLayerPool> LayerPoolTable = new ConditionalWeakTable<World, CellInfoLayerPool>();
+		static readonly ConditionalWeakTable<World, CellInfoLayerPool>.CreateValueCallback CreateLayerPool = world => new CellInfoLayerPool(world.Map);
+
+		static CellInfoLayerPool LayerPoolForWorld(World world)
+		{
+			return LayerPoolTable.GetValue(world, CreateLayerPool);
+		}
+
 		public override IEnumerable<Pair<CPos, int>> Considered
 		{
 			get { return considered; }
@@ -35,7 +44,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 
 		public static IPathSearch Search(World world, MobileInfo mi, Actor self, bool checkForBlocked, Func<CPos, bool> goalCondition)
 		{
-			var graph = new PathGraph(CellInfoLayerManager.Instance.NewLayer(world.Map), mi, self, world, checkForBlocked);
+			var graph = new PathGraph(LayerPoolForWorld(world), mi, self, world, checkForBlocked);
 			var search = new PathSearch(graph);
 			search.isGoal = goalCondition;
 			search.heuristic = loc => 0;
@@ -44,7 +53,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 
 		public static IPathSearch FromPoint(World world, MobileInfo mi, Actor self, CPos from, CPos target, bool checkForBlocked)
 		{
-			var graph = new PathGraph(CellInfoLayerManager.Instance.NewLayer(world.Map), mi, self, world, checkForBlocked);
+			var graph = new PathGraph(LayerPoolForWorld(world), mi, self, world, checkForBlocked);
 			var search = new PathSearch(graph)
 			{
 				heuristic = DefaultEstimator(target)
@@ -64,7 +73,7 @@ namespace OpenRA.Mods.Common.Pathfinder
 
 		public static IPathSearch FromPoints(World world, MobileInfo mi, Actor self, IEnumerable<CPos> froms, CPos target, bool checkForBlocked)
 		{
-			var graph = new PathGraph(CellInfoLayerManager.Instance.NewLayer(world.Map), mi, self, world, checkForBlocked);
+			var graph = new PathGraph(LayerPoolForWorld(world), mi, self, world, checkForBlocked);
 			var search = new PathSearch(graph)
 			{
 				heuristic = DefaultEstimator(target)

OpenRA.Mods.Common/Traits/Harvester.cs

@@ -167,23 +167,24 @@ namespace OpenRA.Mods.Common.Traits
 				.ToDictionary(r => r.Location);
 
 			// Start a search from each refinery's delivery location:
+			List<CPos> path;
 			var mi = self.Info.Traits.Get<MobileInfo>();
-			var path = self.World.WorldActor.Trait<IPathFinder>().FindPath(
+			using (var search = PathSearch.FromPoints(self.World, mi, self, refs.Values.Select(r => r.Location), self.Location, false)
-				PathSearch.FromPoints(self.World, mi, self, refs.Values.Select(r => r.Location), self.Location, false)
+				.WithCustomCost(loc =>
-					.WithCustomCost(loc =>
+				{
-					{
+					if (!refs.ContainsKey(loc))
-						if (!refs.ContainsKey(loc))
+						return 0;
-							return 0;
 
-						var occupancy = refs[loc].Occupancy;
+					var occupancy = refs[loc].Occupancy;
 
-						// 4 harvesters clogs up the refinery's delivery location:
+					// 4 harvesters clogs up the refinery's delivery location:
-						if (occupancy >= 3)
+					if (occupancy >= 3)
-							return Constants.InvalidNode;
+						return Constants.InvalidNode;
 
-						// Prefer refineries with less occupancy (multiplier is to offset distance cost):
+					// Prefer refineries with less occupancy (multiplier is to offset distance cost):
-						return occupancy * 12;
+					return occupancy * 12;
-					}));
+				}))
+				path = self.World.WorldActor.Trait<IPathFinder>().FindPath(search);
 
 			if (path.Count != 0)
 				return refs[path.Last()].Actor;

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

@@ -72,9 +72,10 @@ namespace OpenRA.Mods.Common.Traits
 					return EmptyPath;
 			}
 
-			var pb = FindBidiPath(
+			List<CPos> pb;
-				PathSearch.FromPoint(world, mi, self, target, source, true),
+			using (var fromSrc = PathSearch.FromPoint(world, mi, self, target, source, true))
-				PathSearch.FromPoint(world, mi, self, source, target, true).Reverse());
+			using (var fromDest = PathSearch.FromPoint(world, mi, self, source, target, true).Reverse())
+				pb = FindBidiPath(fromSrc, fromDest);
 
 			CheckSanePath2(pb, source, target);
 
@@ -106,11 +107,9 @@ namespace OpenRA.Mods.Common.Traits
 					return EmptyPath;
 			}
 
-			var path = FindBidiPath(
+			using (var fromSrc = PathSearch.FromPoints(world, mi, self, tilesInRange, source, true))
-				PathSearch.FromPoints(world, mi, self, tilesInRange, source, true),
+			using (var fromDest = PathSearch.FromPoint(world, mi, self, source, targetCell, true).Reverse())
-				PathSearch.FromPoint(world, mi, self, source, targetCell, true).Reverse());
+				return FindBidiPath(fromSrc, fromDest);
-
-			return path;
 		}
 
 		public List<CPos> FindPath(IPathSearch search)