From 31b69a64a1e2568a51b0edb133333c0cc2049137 Mon Sep 17 00:00:00 2001
From: asvitkine <alexei.svitkine@gmail.com>
Date: Sun, 9 Jul 2023 22:47:41 -0400
Subject: [PATCH] Rewrite findTargets for better performance. (#11754)

* Rewrite findTargets for better performance.

- Avoids O(n^2) destroyer checks.
- Avoids an expensive removeAll() call.
- Uses a single stream filter to process, removing an unnecessary intermediate collection in the common case when no destroyers are present.

Note: This was showing up in performance profiles.

* Better variable name.
---
 .../steps/fire/general/TargetGroup.java       | 29 +++++++++++--------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/steps/fire/general/TargetGroup.java b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/steps/fire/general/TargetGroup.java
index 16cc2bf706b..ada01ff32b5 100644
--- a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/steps/fire/general/TargetGroup.java
+++ b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/steps/fire/general/TargetGroup.java
@@ -7,7 +7,6 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Comparator;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
@@ -44,13 +43,13 @@ public Collection<Unit> getTargetUnits(final Collection<Unit> units) {
    */
   public static List<TargetGroup> newTargetGroups(
       final Collection<Unit> units, final Collection<Unit> enemyUnits) {
-
     final Set<UnitType> unitTypes = units.stream().map(Unit::getType).collect(Collectors.toSet());
+    final boolean destroyerPresent = unitTypes.stream().anyMatch(Matches.unitTypeIsDestroyer());
     final Set<UnitType> enemyUnitTypes =
         enemyUnits.stream().map(Unit::getType).collect(Collectors.toSet());
     final List<TargetGroup> targetGroups = new ArrayList<>();
     for (final UnitType unitType : unitTypes) {
-      final Set<UnitType> targets = findTargets(unitType, unitTypes, enemyUnitTypes);
+      final Set<UnitType> targets = findTargets(unitType, destroyerPresent, enemyUnitTypes);
       if (targets.isEmpty()) {
         continue;
       }
@@ -65,15 +64,21 @@ public static List<TargetGroup> newTargetGroups(
   }
 
   private static Set<UnitType> findTargets(
-      final UnitType unitType, final Set<UnitType> unitTypes, final Set<UnitType> enemyUnitTypes) {
-    final Set<UnitType> targets = new HashSet<>(enemyUnitTypes);
-    targets.removeAll(unitType.getUnitAttachment().getCanNotTarget());
-    return unitTypes.stream().anyMatch(Matches.unitTypeIsDestroyer())
-        ? targets
-        : targets.stream()
-            .filter(
-                target -> !target.getUnitAttachment().getCanNotBeTargetedBy().contains(unitType))
-            .collect(Collectors.toSet());
+      UnitType unitType, boolean destroyerPresent, Set<UnitType> enemyUnitTypes) {
+    Set<UnitType> cannotTarget = unitType.getUnitAttachment().getCanNotTarget();
+    // Note: uses a single stream instead of a sequence of removeAll() calls for performance.
+    return enemyUnitTypes.stream()
+        .filter(
+            targetUnitType -> {
+              if (cannotTarget.contains(targetUnitType)) {
+                return false;
+              }
+              if (destroyerPresent) {
+                return true;
+              }
+              return !targetUnitType.getUnitAttachment().getCanNotBeTargetedBy().contains(unitType);
+            })
+        .collect(Collectors.toSet());
   }
 
   private static Optional<TargetGroup> findTargetsInTargetGroups(