From a232eff7fdeb25350d8cea23a6e541a7f36f412f Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Thu, 31 Dec 2015 01:42:19 +0000 Subject: [PATCH] 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 | 35 ++++-------- OpenRA.Game/Traits/TraitsInterfaces.cs | 5 -- OpenRA.Mods.RA/OpenRA.Mods.RA.csproj | 1 - OpenRA.Mods.RA/Traits/GpsRemoveFrozenActor.cs | 55 ------------------- mods/ra/rules/defaults.yaml | 2 - 5 files changed, 12 insertions(+), 86 deletions(-) delete mode 100644 OpenRA.Mods.RA/Traits/GpsRemoveFrozenActor.cs diff --git a/OpenRA.Game/Traits/Player/FrozenActorLayer.cs b/OpenRA.Game/Traits/Player/FrozenActorLayer.cs index 002c65feb6..5a142312f8 100644 --- a/OpenRA.Game/Traits/Player/FrozenActorLayer.cs +++ b/OpenRA.Game/Traits/Player/FrozenActorLayer.cs @@ -32,7 +32,6 @@ namespace OpenRA.Traits public readonly WPos CenterPosition; public readonly Rectangle Bounds; public readonly HashSet 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().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(); + var frozenActorsToRemove = new List(); VisibilityHash = 0; FrozenHash = 0; @@ -253,22 +248,16 @@ namespace OpenRA.Traits if (dirtyFrozenActorIds.Contains(id)) frozenActor.UpdateVisibility(); - if (frozenActor.ShouldBeRemoved(owner)) - idsToRemove.Add(id); - else if (frozenActor.Visible) + if (frozenActor.Visible) VisibilityHash += hash; else if (frozenActor.Actor == null) - idsToRemove.Add(id); + frozenActorsToRemove.Add(frozenActor); } dirtyFrozenActorIds.Clear(); - foreach (var id in idsToRemove) - { - partitionedFrozenActorIds.Remove(id); - world.ScreenMap.Remove(owner, frozenActorsById[id]); - frozenActorsById.Remove(id); - } + foreach (var fa in frozenActorsToRemove) + Remove(fa); } void UpdateDirtyFrozenActorsFromDirtyBins() diff --git a/OpenRA.Game/Traits/TraitsInterfaces.cs b/OpenRA.Game/Traits/TraitsInterfaces.cs index 5ac5d3feb2..ffa931225a 100644 --- a/OpenRA.Game/Traits/TraitsInterfaces.cs +++ b/OpenRA.Game/Traits/TraitsInterfaces.cs @@ -388,11 +388,6 @@ namespace OpenRA.Traits void DoImpact(Target target, Actor firedBy, IEnumerable damageModifiers); } - public interface IRemoveFrozenActor - { - bool RemoveActor(Actor self, Player owner); - } - public interface IRulesetLoaded { void RulesetLoaded(Ruleset rules, TInfo info); } public interface IRulesetLoaded : IRulesetLoaded, ITraitInfoInterface { } } diff --git a/OpenRA.Mods.RA/OpenRA.Mods.RA.csproj b/OpenRA.Mods.RA/OpenRA.Mods.RA.csproj index b0998d18f0..c35badf4cf 100644 --- a/OpenRA.Mods.RA/OpenRA.Mods.RA.csproj +++ b/OpenRA.Mods.RA/OpenRA.Mods.RA.csproj @@ -90,7 +90,6 @@ - diff --git a/OpenRA.Mods.RA/Traits/GpsRemoveFrozenActor.cs b/OpenRA.Mods.RA/Traits/GpsRemoveFrozenActor.cs deleted file mode 100644 index 3b432096ff..0000000000 --- a/OpenRA.Mods.RA/Traits/GpsRemoveFrozenActor.cs +++ /dev/null @@ -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().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; - } - } -} diff --git a/mods/ra/rules/defaults.yaml b/mods/ra/rules/defaults.yaml index 21a3532603..6e21894c14 100644 --- a/mods/ra/rules/defaults.yaml +++ b/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