Checked LINQ queries and collections for inefficiencies.

- Made Array.IndexOf available via extension method.
- Made ToHashSet extension method.
- Change collections queried often via Contains into sets.
- Avoid Count() extension if Count or Length property exist.
- Made Count() > 0 checks and variations calls to Any() instead.
- Don't call ToList/ToArray if there is no benefit to materializing the sequence.
- If the sequence does benefit from materialization, follow this general pattern:
  - Collection queried often via Contains use ToHashSet to speed up lookups.
  - Short lived variables use ToList. This is because ToArray requires an extra copy to output the final size.
  - Collections persisted into fields or for a long time use ToArray to minimize memory overhead.
This commit is contained in:
RoosterDragon
2014-05-23 19:52:44 +01:00
committed by RoosterDragon
parent f5f3747338
commit 82bea961ba
44 changed files with 151 additions and 126 deletions

View File

@@ -25,8 +25,11 @@ namespace OpenRA.Effects
this.target = target;
player = asPlayer;
remainingTicks = ticks;
foreach (var e in target.World.Effects.OfType<FlashTarget>().Where(a => a.target == target).ToArray())
target.World.Remove(e);
target.World.RemoveAll(effect =>
{
var flashTarget = effect as FlashTarget;
return flashTarget != null && flashTarget.target == target;
});
}
public void Tick(World world)

View File

@@ -106,6 +106,11 @@ namespace OpenRA
return ret;
}
public static int IndexOf<T>(this T[] array, T value)
{
return Array.IndexOf(array, value);
}
public static T Random<T>(this IEnumerable<T> ts, MersenneTwister r)
{
var xs = ts as ICollection<T>;
@@ -299,6 +304,11 @@ namespace OpenRA
return ts.Concat(moreTs);
}
public static HashSet<T> ToHashSet<T>(this IEnumerable<T> source)
{
return new HashSet<T>(source);
}
public static Dictionary<TKey, TSource> ToDictionaryWithConflictLog<TSource, TKey>(this IEnumerable<TSource> source, Func<TSource, TKey> keySelector, string debugName, Func<TKey, string> logKey, Func<TSource, string> logValue)
{
return ToDictionaryWithConflictLog(source, keySelector, x => x, debugName, logKey, logValue);
@@ -341,7 +351,7 @@ namespace OpenRA
// If any duplicates were found, throw a descriptive error
if (dupKeys.Count > 0)
{
var badKeysFormatted = string.Join(", ", dupKeys.Select(p => "{0}: [{1}]".F(logKey(p.Key), string.Join(",", p.Value.ToArray()))).ToArray());
var badKeysFormatted = string.Join(", ", dupKeys.Select(p => "{0}: [{1}]".F(logKey(p.Key), string.Join(",", p.Value))));
var msg = "{0}, duplicate values found for the following keys: {1}".F(debugName, badKeysFormatted);
throw new ArgumentException(msg);
}

View File

@@ -32,14 +32,14 @@ namespace OpenRA.Graphics
var c1 = new HSLColor(c.H, c.S, (byte)Math.Max(rampRange, c.L)).RGB;
var c2 = new HSLColor(c.H, c.S, (byte)Math.Max(0, c.L - rampRange)).RGB;
var baseIndex = ramp[0];
var remapRamp = ramp.Select(r => r - ramp[0]).ToArray();
var remapRamp = ramp.Select(r => r - ramp[0]);
// reversed remapping
if (ramp[0] > ramp[15])
{
baseIndex = ramp[15];
for (var i = 15; i > 0; i--)
remapRamp = ramp.Select(r => r - ramp[15]).ToArray();
remapRamp = ramp.Select(r => r - ramp[15]);
}
remapColors = remapRamp.Select((x, i) => Pair.New(baseIndex + i, Exts.ColorLerp(x / 16f, c1, c2)))

View File

@@ -80,12 +80,11 @@ namespace OpenRA.Graphics
List<IRenderable> GenerateRenderables()
{
var actors = World.ScreenMap.ActorsInBox(Viewport.TopLeft, Viewport.BottomRight)
.Append(World.WorldActor)
.ToList();
.Append(World.WorldActor);
// Include player actor for the rendered player
if (World.RenderPlayer != null)
actors.Add(World.RenderPlayer.PlayerActor);
actors = actors.Append(World.RenderPlayer.PlayerActor);
var worldRenderables = actors.SelectMany(a => a.Render(this));
if (World.OrderGenerator != null)

View File

@@ -15,7 +15,7 @@ namespace OpenRA
{
public class Group
{
List<Actor> actors;
Actor[] actors;
int id;
static int nextGroup;
@@ -24,7 +24,7 @@ namespace OpenRA
public Group(IEnumerable<Actor> actors)
{
this.actors = actors.ToList();
this.actors = actors.ToArray();
foreach (var a in actors)
a.Group = this;

View File

@@ -79,7 +79,7 @@ namespace OpenRA
foreach (var p in maps.Values)
p.UpdateRemoteSearch(MapStatus.Searching, null);
var url = Game.Settings.Game.MapRepository + "hash/" + string.Join(",", maps.Keys.ToArray()) + "/yaml";
var url = Game.Settings.Game.MapRepository + "hash/" + string.Join(",", maps.Keys) + "/yaml";
Action<DownloadDataCompletedEventArgs, bool> onInfoComplete = (i, cancelled) =>
{

View File

@@ -46,7 +46,7 @@ namespace OpenRA
public class MapPreview
{
static readonly List<CPos> NoSpawns = new List<CPos>();
static readonly CPos[] NoSpawns = new CPos[] { };
MapCache cache;
public readonly string Uid;
@@ -54,7 +54,7 @@ namespace OpenRA
public string Type { get; private set; }
public string Author { get; private set; }
public int PlayerCount { get; private set; }
public List<CPos> SpawnPoints { get; private set; }
public CPos[] SpawnPoints { get; private set; }
public Rectangle Bounds { get; private set; }
public Bitmap CustomPreview { get; private set; }
public Map Map { get; private set; }
@@ -112,7 +112,7 @@ namespace OpenRA
Author = m.Author;
PlayerCount = m.Players.Count(x => x.Value.Playable);
Bounds = m.Bounds;
SpawnPoints = m.GetSpawnPoints().ToList();
SpawnPoints = m.GetSpawnPoints();
CustomPreview = m.CustomPreview;
Status = MapStatus.Available;
Class = classification;
@@ -143,9 +143,9 @@ namespace OpenRA
PlayerCount = r.players;
Bounds = r.bounds;
var spawns = new List<CPos>();
var spawns = new CPos[r.spawnpoints.Length / 2];
for (var j = 0; j < r.spawnpoints.Length; j += 2)
spawns.Add(new CPos(r.spawnpoints[j], r.spawnpoints[j + 1]));
spawns[j / 2] = new CPos(r.spawnpoints[j], r.spawnpoints[j + 1]);
SpawnPoints = spawns;
CustomPreview = new Bitmap(new MemoryStream(Convert.FromBase64String(r.minimap)));

View File

@@ -37,7 +37,7 @@ namespace OpenRA.Orders
var orders = world.Selection.Actors
.Select(a => OrderForUnit(a, target, mi))
.Where(o => o != null)
.ToArray();
.ToList();
var actorsInvolved = orders.Select(o => o.Actor).Distinct();
if (actorsInvolved.Any())
@@ -81,8 +81,7 @@ namespace OpenRA.Orders
var orders = world.Selection.Actors
.Select(a => OrderForUnit(a, target, mi))
.Where(o => o != null)
.ToArray();
.Where(o => o != null);
var cursorName = orders.Select(o => o.Cursor).FirstOrDefault();
return cursorName ?? (useSelect ? "select" : "default");

View File

@@ -50,7 +50,7 @@ namespace OpenRA
{
var selectableCountries = world.Map.Rules.Actors["world"].Traits
.WithInterface<CountryInfo>().Where(c => c.Selectable)
.ToArray();
.ToList();
return selectableCountries.FirstOrDefault(c => c.Race == name)
?? selectableCountries.Random(world.SharedRandom);

View File

@@ -80,7 +80,7 @@ namespace OpenRA.Scripting
// The 'this.' resolves the actual (subclass) type
var type = this.GetType();
var names = type.GetCustomAttributes<ScriptGlobalAttribute>(true);
if (names.Count() != 1)
if (names.Length != 1)
throw new InvalidOperationException("[LuaGlobal] attribute not found for global table '{0}'".F(type));
Name = names.First().Name;
@@ -242,13 +242,13 @@ namespace OpenRA.Scripting
runtime.Dispose();
}
static Type[] ExtractRequiredTypes(Type t)
static IEnumerable<Type> ExtractRequiredTypes(Type t)
{
// Returns the inner types of all the Requires<T> interfaces on this type
var outer = t.GetInterfaces()
.Where(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(Requires<>));
return outer.SelectMany(i => i.GetGenericArguments()).ToArray();
return outer.SelectMany(i => i.GetGenericArguments());
}
static readonly object[] NoArguments = new object[0];

View File

@@ -18,7 +18,7 @@ namespace OpenRA
{
public class Selection
{
List<Actor> actors = new List<Actor>();
readonly HashSet<Actor> actors = new HashSet<Actor>();
public void Add(World w, Actor a)
{
actors.Add(a);
@@ -30,20 +30,32 @@ namespace OpenRA
public bool Contains(Actor a)
{
return actors.AsEnumerable().Contains(a);
return actors.Contains(a);
}
public void Combine(World world, IEnumerable<Actor> newSelection, bool isCombine, bool isClick)
{
var oldSelection = actors.AsEnumerable();
if (isClick)
{
var adjNewSelection = newSelection.Take(1); /* TODO: select BEST, not FIRST */
actors = (isCombine ? oldSelection.SymmetricDifference(adjNewSelection) : adjNewSelection).ToList();
if (isCombine)
actors.SymmetricExceptWith(adjNewSelection);
else
{
actors.Clear();
actors.UnionWith(adjNewSelection);
}
}
else
actors = (isCombine ? oldSelection.Union(newSelection) : newSelection).ToList();
{
if (isCombine)
actors.UnionWith(newSelection);
else
{
actors.Clear();
actors.UnionWith(newSelection);
}
}
var voicedUnit = actors.FirstOrDefault(a => a.Owner == world.LocalPlayer && a.IsInWorld && a.HasVoices());
if (voicedUnit != null)
@@ -57,11 +69,11 @@ namespace OpenRA
}
public IEnumerable<Actor> Actors { get { return actors; } }
public void Clear() { actors = new List<Actor>(); }
public void Clear() { actors.Clear(); }
public void Tick(World world)
{
actors.RemoveAll(a => !a.IsInWorld || (!a.Owner.IsAlliedWith(world.RenderPlayer) && world.FogObscures(a)));
actors.RemoveWhere(a => !a.IsInWorld || (!a.Owner.IsAlliedWith(world.RenderPlayer) && world.FogObscures(a)));
foreach (var cg in controlGroups.Values)
{

View File

@@ -406,7 +406,7 @@ namespace OpenRA.Server
public void DispatchOrdersToClients(Connection conn, int frame, byte[] data)
{
var from = conn != null ? conn.PlayerIndex : 0;
foreach (var c in Conns.Except(conn).ToArray())
foreach (var c in Conns.Except(conn).ToList())
DispatchOrdersToClient(c, from, frame, data);
}
@@ -517,9 +517,7 @@ namespace OpenRA.Server
public void DropClient(Connection toDrop, int frame)
{
if (PreConns.Contains(toDrop))
PreConns.Remove(toDrop);
else
if (!PreConns.Remove(toDrop))
{
Conns.Remove(toDrop);

View File

@@ -130,10 +130,7 @@ namespace OpenRA.Traits
public static IEnumerable<CPos> ExpandFootprint(IEnumerable<CPos> cells, bool allowDiagonal)
{
var result = new Dictionary<CPos, bool>();
foreach (var c in cells.SelectMany(c => Neighbours(c, allowDiagonal)))
result[c] = true;
return result.Keys;
return cells.SelectMany(c => Neighbours(c, allowDiagonal)).Distinct();
}
public static IEnumerable<CPos> AdjacentCells(World w, Target target)

View File

@@ -26,7 +26,7 @@ namespace OpenRA.Widgets
Game.Renderer.LineRenderer.DrawLine(origin + new float2(100, 0) * basis, origin + new float2(100, 100) * basis, Color.White, Color.White);
var k = 0;
foreach (var item in PerfHistory.Items.Values.ToArray())
foreach (var item in PerfHistory.Items.Values)
{
var n = 0;
item.Samples().Aggregate((a, b) =>
@@ -55,7 +55,7 @@ namespace OpenRA.Widgets
}
k = 0;
foreach (var item in PerfHistory.Items.Values.ToArray())
foreach (var item in PerfHistory.Items.Values)
{
Game.Renderer.Fonts["Tiny"].DrawText(item.Name, new float2(rect.Left, rect.Top) + new float2(18, 10 * k - 3), Color.White);
++k;

View File

@@ -75,7 +75,7 @@ namespace OpenRA.Widgets
public static int[] GetBorderSizes(string collection)
{
var images = new[] { "border-t", "border-b", "border-l", "border-r" };
var ss = images.Select(i => ChromeProvider.GetImage(collection, i)).ToArray();
var ss = images.Select(i => ChromeProvider.GetImage(collection, i)).ToList();
return new[] { (int)ss[0].Size.Y, (int)ss[1].Size.Y, (int)ss[2].Size.X, (int)ss[3].Size.X };
}

View File

@@ -26,9 +26,16 @@ namespace OpenRA
public class World
{
class ActorIDComparer : IComparer<Actor>
{
public static readonly ActorIDComparer Instance = new ActorIDComparer();
private ActorIDComparer() { }
public int Compare(Actor x, Actor y) { return x.ActorID.CompareTo(y.ActorID); }
}
static readonly Func<MPos, bool> FalsePredicate = _ => false;
internal readonly TraitDictionary TraitDict = new TraitDictionary();
readonly HashSet<Actor> actors = new HashSet<Actor>();
readonly SortedSet<Actor> actors = new SortedSet<Actor>(ActorIDComparer.Instance);
readonly List<IEffect> effects = new List<IEffect>();
readonly Queue<Action<World>> frameEndActions = new Queue<Action<World>>();
@@ -237,6 +244,7 @@ namespace OpenRA
public void Add(IEffect b) { effects.Add(b); }
public void Remove(IEffect b) { effects.Remove(b); }
public void RemoveAll(Predicate<IEffect> predicate) { effects.RemoveAll(predicate); }
public void AddFrameEndTask(Action<World> a) { frameEndActions.Enqueue(a); }