Replace IRemoveFrozenActor with FrozenActorLayer.Remove.

The IRemoveFrozenActor interface is replaced with a Remove method on FrozenActorLayer. IRemoveFrozenActor is a performance problem for FrozenActorLayer.Tick as it incurs a large cache miss penalty in order to load and enumerate the array of these interfaces for every frozen actor. Instead, we invert control and allow traits to remove frozen actors directly which eliminates the performance penalty.

OpenRA.Game/Traits/Player/FrozenActorLayer.cs

@@ -32,7 +32,6 @@ namespace OpenRA.Traits
 		public readonly WPos CenterPosition;
 		public readonly Rectangle Bounds;
 		public readonly HashSet<string> TargetTypes;
-		readonly IRemoveFrozenActor[] removeFrozenActors;
 		readonly Actor actor;
 		readonly Shroud shroud;
 
@@ -59,7 +58,6 @@ namespace OpenRA.Traits
 			actor = self;
 			this.shroud = shroud;
 			NeedRenderables = startsRevealed;
-			removeFrozenActors = self.TraitsImplementing<IRemoveFrozenActor>().ToArray();
 
 			// Consider all cells inside the map area (ignoring the current map bounds)
 			Footprint = footprint
@@ -149,16 +147,6 @@ namespace OpenRA.Traits
 
 		public bool HasRenderables { get { return !Shrouded && Renderables.Any(); } }
 
-		public bool ShouldBeRemoved(Player owner)
-		{
-			// PERF: Avoid LINQ.
-			foreach (var rfa in removeFrozenActors)
-				if (rfa.RemoveActor(actor, owner))
-					return true;
-
-			return false;
-		}
-
 		public override string ToString()
 		{
 			return "{0} {1}{2}".F(Info.Name, ID, IsValid ? "" : " (invalid)");
@@ -211,6 +199,13 @@ namespace OpenRA.Traits
 			partitionedFrozenActorIds.Add(fa.ID, FootprintBounds(fa));
 		}
 
+		public void Remove(FrozenActor fa)
+		{
+			partitionedFrozenActorIds.Remove(fa.ID);
+			world.ScreenMap.Remove(owner, fa);
+			frozenActorsById.Remove(fa.ID);
+		}
+
 		Rectangle FootprintBounds(FrozenActor fa)
 		{
 			var p1 = fa.Footprint[0];
@@ -238,7 +233,7 @@ namespace OpenRA.Traits
 		{
 			UpdateDirtyFrozenActorsFromDirtyBins();
 
-			var idsToRemove = new List<uint>();
+			var frozenActorsToRemove = new List<FrozenActor>();
 			VisibilityHash = 0;
 			FrozenHash = 0;
 
@@ -253,22 +248,16 @@ namespace OpenRA.Traits
 				if (dirtyFrozenActorIds.Contains(id))
 					frozenActor.UpdateVisibility();
 
-				if (frozenActor.ShouldBeRemoved(owner))
+				if (frozenActor.Visible)
-					idsToRemove.Add(id);
-				else if (frozenActor.Visible)
 					VisibilityHash += hash;
 				else if (frozenActor.Actor == null)
-					idsToRemove.Add(id);
+					frozenActorsToRemove.Add(frozenActor);
 			}
 
 			dirtyFrozenActorIds.Clear();
 
-			foreach (var id in idsToRemove)
+			foreach (var fa in frozenActorsToRemove)
-			{
+				Remove(fa);
-				partitionedFrozenActorIds.Remove(id);
-				world.ScreenMap.Remove(owner, frozenActorsById[id]);
-				frozenActorsById.Remove(id);
-			}
 		}
 
 		void UpdateDirtyFrozenActorsFromDirtyBins()

OpenRA.Game/Traits/TraitsInterfaces.cs

@@ -388,11 +388,6 @@ namespace OpenRA.Traits
 		void DoImpact(Target target, Actor firedBy, IEnumerable<int> damageModifiers);
 	}
 
-	public interface IRemoveFrozenActor
-	{
-		bool RemoveActor(Actor self, Player owner);
-	}
-
 	public interface IRulesetLoaded<TInfo> { void RulesetLoaded(Ruleset rules, TInfo info); }
 	public interface IRulesetLoaded : IRulesetLoaded<ActorInfo>, ITraitInfoInterface { }
 }

OpenRA.Mods.RA/OpenRA.Mods.RA.csproj

@@ -90,7 +90,6 @@
     <Compile Include="Traits\Chronoshiftable.cs" />
     <Compile Include="Traits\Cloneable.cs" />
     <Compile Include="Traits\Disguise.cs" />
-    <Compile Include="Traits\GpsRemoveFrozenActor.cs" />
     <Compile Include="Traits\HarvesterHuskModifier.cs" />
     <Compile Include="Traits\Infiltration\InfiltrateForCash.cs" />
     <Compile Include="Traits\Infiltration\InfiltrateForExploration.cs" />

OpenRA.Mods.RA/Traits/GpsRemoveFrozenActor.cs

@@ -1,55 +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.Linq;
-using OpenRA.Traits;
-
-namespace OpenRA.Mods.RA.Traits
-{
-	[Desc("Removes frozen actors of actors that are dead or sold," +
-		" when having an active GPS power.")]
-	public class GpsRemoveFrozenActorInfo : ITraitInfo
-	{
-		[Desc("Should this trait also affect allied players?")]
-		public bool GrantAllies = true;
-
-		public object Create(ActorInitializer init) { return new GpsRemoveFrozenActor(init.Self, this); }
-	}
-
-	public class GpsRemoveFrozenActor : IRemoveFrozenActor
-	{
-		readonly GpsWatcher[] watchers;
-		readonly GpsRemoveFrozenActorInfo info;
-
-		public GpsRemoveFrozenActor(Actor self, GpsRemoveFrozenActorInfo info)
-		{
-			this.info = info;
-			watchers = self.World.ActorsWithTrait<GpsWatcher>().Select(w => w.Trait).ToArray();
-		}
-
-		public bool RemoveActor(Actor self, Player owner)
-		{
-			if (!self.IsDead)
-				return false;
-
-			foreach (var w in watchers)
-			{
-				if (w.Owner != owner && !(info.GrantAllies && w.Owner.IsAlliedWith(owner)))
-					continue;
-
-				if (w.Launched)
-					return true;
-			}
-
-			return false;
-		}
-	}
-}

mods/ra/rules/defaults.yaml

@@ -430,7 +430,6 @@
 	Guardable:
 		Range: 3c0
 	FrozenUnderFog:
-	GpsRemoveFrozenActor:
 	Tooltip:
 		GenericName: Structure
 	Demolishable:
@@ -500,7 +499,6 @@
 		SellSounds: cashturn.aud
 	Guardable:
 	FrozenUnderFog:
-	GpsRemoveFrozenActor:
 	Health:
 		HP: 100
 		Shape: Rectangle