Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce allocations in AutoTarget.ChooseTarget. #14933

Merged
merged 1 commit into from Mar 21, 2018

Conversation

Projects
None yet
4 participants
@RoosterDragon
Copy link
Member

RoosterDragon commented Mar 17, 2018

  • Cache active target query.
  • Prefer .Count == 0 over Any when working with Lists.
  • Use helper for GetEnabledTargetTypes.
  • Don't declare target variable until actually needed.

Relates to #14905.

targetPriorities = self.TraitsImplementing<AutoTargetPriority>().ToArray();
activeTargetPriorities =
self.TraitsImplementing<AutoTargetPriority>()
.OrderByDescending(ati => ati.Info.Priority).ToArray()

This comment has been minimized.

@chrisforbes

chrisforbes Mar 18, 2018

Member

Why not .ToArray() on the end? The ToList on :269 could be avoided as well.

This comment has been minimized.

@RoosterDragon

RoosterDragon Mar 18, 2018

Author Member

We can ToArray here since the trait and their priorities will not change. But we want the IsTraitEnabled to be re-evaluated every time we re-run this query - as that can change over time.

This comment has been minimized.

@chrisforbes

This comment has been minimized.

@pchote

pchote Mar 20, 2018

Member

Can we get a comment to that effect here?

var validPriorities = activePriorities.Where(ati =>
{
// Already have a higher priority target
if (ati.Priority < chosenTargetPriority)
return false;

// Incompatible target types
if (!targetTypes.Overlaps(ati.ValidTargets) || targetTypes.Overlaps(ati.InvalidTargets))
if (!ati.ValidTargets.Overlaps(targetTypes) || ati.InvalidTargets.Overlaps(targetTypes))

This comment has been minimized.

@chrisforbes

chrisforbes Mar 18, 2018

Member

Good !/$ in bitsetifying targettypes here too.

Reduce allocations in AutoTarget.ChooseTarget.
- Cache active target query.
- Prefer .Count == 0 over Any when working with Lists.
- Use helper for GetEnabledTargetTypes.
- Don't declare target variable until actually needed.

@RoosterDragon RoosterDragon force-pushed the RoosterDragon:auto-target-perf branch from aa5a78d to 70e3fa3 Mar 20, 2018

@pchote

pchote approved these changes Mar 21, 2018

@pchote pchote merged commit 0555ce9 into OpenRA:bleed Mar 21, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@RoosterDragon RoosterDragon deleted the RoosterDragon:auto-target-perf branch Mar 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.