From 137d384304a0bf05b92bfc2ef7f784d4b79dcdb9 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sun, 28 Nov 2021 18:24:27 +0000 Subject: [PATCH] Remove path caching. The path cache was originally a moderate benefit, but over time a couple of things have conspired against it: - Only paths with BlockedByActor.None are cached. Originally all paths regardless of blocking were cached but this was deemed unacceptable due to potentially returning outdated paths as actors move about. Paths with BlockedByActor.None are only invalidated if terrain conditions change, which are rarer. - Move will try and find a path 4 times, trying with a different BlockedByActor check each time. BlockedByActor.None is the last check and only reached if the other searches fail. This is a rare scenario. Overall, this means the hit rate for the cache is almost non-existent. Given the constraints on path validity it seems unlikely that the hit rate could be improved significantly, therefore it seems reasonable to remove the cache entirely to remove the overhead of cache management. --- OpenRA.Game/CacheStorage.cs | 20 ---- .../Pathfinder/PathCacheStorage.cs | 74 --------------- .../PathFinderUnitPathCacheDecorator.cs | 91 ------------------- OpenRA.Mods.Common/Traits/World/PathFinder.cs | 2 +- 4 files changed, 1 insertion(+), 186 deletions(-) delete mode 100644 OpenRA.Game/CacheStorage.cs delete mode 100644 OpenRA.Mods.Common/Pathfinder/PathCacheStorage.cs delete mode 100644 OpenRA.Mods.Common/Pathfinder/PathFinderUnitPathCacheDecorator.cs diff --git a/OpenRA.Game/CacheStorage.cs b/OpenRA.Game/CacheStorage.cs deleted file mode 100644 index 9f1c4a618e..0000000000 --- a/OpenRA.Game/CacheStorage.cs +++ /dev/null @@ -1,20 +0,0 @@ -#region Copyright & License Information -/* - * Copyright 2007-2021 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, either version 3 of - * the License, or (at your option) any later version. For more - * information, see COPYING. - */ -#endregion - -namespace OpenRA -{ - public interface ICacheStorage - { - void Remove(string key); - void Store(string key, T data); - T Retrieve(string key); - } -} diff --git a/OpenRA.Mods.Common/Pathfinder/PathCacheStorage.cs b/OpenRA.Mods.Common/Pathfinder/PathCacheStorage.cs deleted file mode 100644 index 961e5593aa..0000000000 --- a/OpenRA.Mods.Common/Pathfinder/PathCacheStorage.cs +++ /dev/null @@ -1,74 +0,0 @@ -#region Copyright & License Information -/* - * Copyright 2007-2021 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, either version 3 of - * the License, or (at your option) any later version. For more - * information, see COPYING. - */ -#endregion - -using System.Collections.Generic; -using System.Linq; - -namespace OpenRA.Mods.Common.Pathfinder -{ - public class PathCacheStorage : ICacheStorage> - { - class CachedPath - { - public List Result; - public int Tick; - } - - const int MaxPathAge = 50; - readonly World world; - Dictionary cachedPaths = new Dictionary(100); - - public PathCacheStorage(World world) - { - this.world = world; - } - - public void Remove(string key) - { - cachedPaths.Remove(key); - } - - public void Store(string key, List data) - { - // Eventually clean up the cachedPaths dictionary - if (cachedPaths.Count >= 100) - foreach (var cachedPath in cachedPaths.Where(p => IsExpired(p.Value)).ToList()) - cachedPaths.Remove(cachedPath.Key); - - cachedPaths.Add(key, new CachedPath - { - Tick = world.WorldTick, - Result = data - }); - } - - public List Retrieve(string key) - { - if (cachedPaths.TryGetValue(key, out var cached)) - { - if (IsExpired(cached)) - { - cachedPaths.Remove(key); - return null; - } - - return cached.Result; - } - - return null; - } - - bool IsExpired(CachedPath path) - { - return world.WorldTick - path.Tick > MaxPathAge; - } - } -} diff --git a/OpenRA.Mods.Common/Pathfinder/PathFinderUnitPathCacheDecorator.cs b/OpenRA.Mods.Common/Pathfinder/PathFinderUnitPathCacheDecorator.cs deleted file mode 100644 index 9823e576f7..0000000000 --- a/OpenRA.Mods.Common/Pathfinder/PathFinderUnitPathCacheDecorator.cs +++ /dev/null @@ -1,91 +0,0 @@ -#region Copyright & License Information -/* - * Copyright 2007-2021 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, either version 3 of - * the License, or (at your option) any later version. For more - * information, see COPYING. - */ -#endregion - -using System.Collections.Generic; -using OpenRA.Mods.Common.Traits; -using OpenRA.Support; -using OpenRA.Traits; - -namespace OpenRA.Mods.Common.Pathfinder -{ - /// - /// A decorator used to cache FindUnitPath and FindUnitPathToRange (Decorator design pattern) - /// - public class PathFinderUnitPathCacheDecorator : IPathFinder - { - readonly IPathFinder pathFinder; - readonly ICacheStorage> cacheStorage; - - public PathFinderUnitPathCacheDecorator(IPathFinder pathFinder, ICacheStorage> cacheStorage) - { - this.pathFinder = pathFinder; - this.cacheStorage = cacheStorage; - } - - public List FindUnitPath(CPos source, CPos target, Actor self, Actor ignoreActor, BlockedByActor check) - { - using (new PerfSample("Pathfinder")) - { - var key = "FindUnitPath" + self.ActorID + source.X + source.Y + target.X + target.Y; - - // Only cache path when transient actors are ignored, otherwise there is no guarantee that the path - // is still valid at the next check. - if (check == BlockedByActor.None) - { - var cachedPath = cacheStorage.Retrieve(key); - if (cachedPath != null) - return cachedPath; - } - - var pb = pathFinder.FindUnitPath(source, target, self, ignoreActor, check); - - if (check == BlockedByActor.None) - cacheStorage.Store(key, pb); - - return pb; - } - } - - public List FindUnitPathToRange(CPos source, SubCell srcSub, WPos target, WDist range, Actor self, BlockedByActor check) - { - using (new PerfSample("Pathfinder")) - { - var key = "FindUnitPathToRange" + self.ActorID + source.X + source.Y + target.X + target.Y; - - if (check == BlockedByActor.None) - { - var cachedPath = cacheStorage.Retrieve(key); - if (cachedPath != null) - return cachedPath; - } - - var pb = pathFinder.FindUnitPathToRange(source, srcSub, target, range, self, check); - - if (check == BlockedByActor.None) - cacheStorage.Store(key, pb); - - return pb; - } - } - - public List FindPath(IPathSearch search) - { - using (new PerfSample("Pathfinder")) - return pathFinder.FindPath(search); - } - - public List FindBidiPath(IPathSearch fromSrc, IPathSearch fromDest) - { - using (new PerfSample("Pathfinder")) - return pathFinder.FindBidiPath(fromSrc, fromDest); - } - } -} diff --git a/OpenRA.Mods.Common/Traits/World/PathFinder.cs b/OpenRA.Mods.Common/Traits/World/PathFinder.cs index 7b1d804f81..79876e1eed 100644 --- a/OpenRA.Mods.Common/Traits/World/PathFinder.cs +++ b/OpenRA.Mods.Common/Traits/World/PathFinder.cs @@ -22,7 +22,7 @@ namespace OpenRA.Mods.Common.Traits { public override object Create(ActorInitializer init) { - return new PathFinderUnitPathCacheDecorator(new PathFinder(init.World), new PathCacheStorage(init.World)); + return new PathFinder(init.World); } }