add streamReplacementEffects - #12427

Its fine I guess though I want to see @MostCromulent findings

Looks like a significant performance regression in this form, and marginally worse than master even with modifications to address the issues identified by Claude as causing the slowdown.

Run through the same profiling as #12297:

Profiling summary

Method. Three builds run fresh side by side: the base commit (1679040), this PR (84a2725), and this PR with the modifications described below. Desktop, JDK 25. Whole AI-vs-AI games run headless from fixed seeds, with the AI decision timeout raised so no build abandons a decision another finishes. Four repetitions per build, with the build order rotated and reversed between repetitions. Four workloads: a mirror match of Goblin token decks (20 games), a mirror match of a simple red-green creature and burn deck (20 games), a mirror match of Goblin lord decks (20 games), and a four-player Commander table of precons (3 games). Per-game figures are the median of the four repetitions. Android was not measured.

Totals, mean of four repetitions (range), change against the base:

Workload	Base	This PR	Modified
Goblin tokens, 20 games	47.9 s (47.8–48.1)	62.5 s (62.3–62.8), +31%	48.8 s (48.1–49.1), +1.9%
Red-green creatures, 20 games	35.0 s (34.8–35.3)	45.3 s (45.1–45.5), +29%	35.6 s (35.3–35.9), +1.7%
Goblin lords, 20 games	27.2 s (27.1–27.5)	32.2 s (31.9–32.4), +18%	27.5 s (27.4–27.6), +1.0%
Four-player Commander, 3 games	74.0 s (73.1–75.1)	84.1 s (83.6–84.6), +14%	75.1 s (74.6–75.9), +1.5%

This PR's ranges do not overlap the base's on any workload, and it is slower on 62 of the 63 games. The modified version is 1–2% slower than the base on every workload. That is within run-to-run noise for these workloads (about 3.5% for tokens, 4–7% for Commander), but its runs sit at or above the base's on all four, so a small cost is likely. Per game it ranges from 9% faster to 11% slower, and it is slower on 43 of the 63. All 63 games had the same turn count and log-entry count in every build and every repetition.

Goblin tokens, all 20 games

Game	Turns	Base	This PR	Change	Modified	Change
1	11	0.60 s	0.76 s	+26%	0.61 s	+2%
2	29	2.86 s	3.74 s	+31%	2.93 s	+2%
3	14	0.42 s	0.52 s	+22%	0.42 s	+0%
4	18	3.40 s	4.62 s	+36%	3.53 s	+4%
5	18	1.73 s	2.25 s	+30%	1.76 s	+2%
6	28	1.46 s	1.90 s	+30%	1.47 s	+0%
7	24	0.88 s	1.30 s	+48%	0.91 s	+3%
8	13	0.39 s	0.43 s	+12%	0.35 s	−9%
9	13	0.53 s	0.83 s	+56%	0.56 s	+4%
10	17	0.71 s	0.98 s	+37%	0.74 s	+4%
11	18	1.45 s	1.90 s	+31%	1.46 s	+1%
12	14	0.51 s	0.62 s	+23%	0.48 s	−5%
13	37	2.72 s	3.62 s	+33%	2.81 s	+3%
14	19	2.98 s	3.96 s	+33%	3.04 s	+2%
15	17	1.59 s	2.06 s	+30%	1.65 s	+4%
16	12	0.50 s	0.67 s	+33%	0.48 s	−5%
17	20	0.75 s	1.01 s	+35%	0.73 s	−3%
18	37	21.43 s	27.42 s	+28%	21.97 s	+3%
19	11	0.36 s	0.43 s	+21%	0.36 s	+1%
20	20	2.55 s	3.46 s	+36%	2.59 s	+2%

Red-green creatures, all 20 games

Game	Turns	Base	This PR	Change	Modified	Change
1	90	25.77 s	33.01 s	+28%	26.13 s	+1%
2	31	0.71 s	0.96 s	+35%	0.72 s	+1%
3	30	0.93 s	1.22 s	+32%	0.93 s	+0%
4	18	0.43 s	0.47 s	+10%	0.43 s	+0%
5	14	0.32 s	0.38 s	+20%	0.32 s	+2%
6	17	0.33 s	0.43 s	+31%	0.31 s	−5%
7	22	0.41 s	0.52 s	+27%	0.43 s	+5%
8	15	0.30 s	0.39 s	+29%	0.31 s	+4%
9	25	0.53 s	0.78 s	+47%	0.53 s	+0%
10	26	0.76 s	0.87 s	+14%	0.77 s	+1%
11	16	0.38 s	0.42 s	+10%	0.39 s	+2%
12	24	0.57 s	0.88 s	+54%	0.59 s	+3%
13	21	0.61 s	0.98 s	+61%	0.68 s	+11%
14	22	0.36 s	0.49 s	+35%	0.39 s	+7%
15	18	0.40 s	0.57 s	+42%	0.41 s	+2%
16	19	0.37 s	0.48 s	+29%	0.41 s	+10%
17	18	0.30 s	0.43 s	+45%	0.31 s	+3%
18	18	0.45 s	0.59 s	+31%	0.42 s	−6%
19	21	0.82 s	1.13 s	+38%	0.88 s	+7%
20	12	0.24 s	0.26 s	+9%	0.24 s	+0%

Goblin lords, all 20 games

Game	Turns	Base	This PR	Change	Modified	Change
1	18	3.65 s	3.94 s	+8%	3.63 s	+0%
2	12	0.52 s	0.60 s	+14%	0.51 s	−3%
3	12	0.68 s	0.78 s	+14%	0.68 s	+0%
4	9	0.34 s	0.37 s	+7%	0.34 s	+1%
5	18	1.30 s	1.65 s	+27%	1.28 s	−1%
6	12	0.37 s	0.43 s	+16%	0.37 s	+0%
7	10	0.44 s	0.48 s	+11%	0.47 s	+8%
8	9	0.42 s	0.45 s	+6%	0.43 s	+1%
9	10	0.34 s	0.42 s	+24%	0.34 s	−1%
10	26	7.03 s	9.00 s	+28%	7.25 s	+3%
11	9	0.34 s	0.37 s	+6%	0.33 s	−3%
12	18	1.63 s	2.02 s	+23%	1.64 s	+1%
13	9	0.36 s	0.38 s	+5%	0.34 s	−3%
14	20	6.84 s	7.78 s	+14%	6.88 s	+1%
15	9	0.28 s	0.32 s	+17%	0.27 s	−2%
16	10	0.34 s	0.38 s	+11%	0.33 s	−2%
17	12	0.59 s	0.68 s	+15%	0.59 s	+0%
18	9	0.30 s	0.29 s	−3%	0.27 s	−9%
19	10	0.44 s	0.49 s	+11%	0.44 s	+0%
20	16	1.07 s	1.35 s	+27%	1.05 s	−2%

Four-player Commander, all 3 games

Game	Turns	Base	This PR	Change	Modified	Change
1	60	26.46 s	31.30 s	+18%	27.08 s	+2%
2	54	32.35 s	35.65 s	+10%	32.62 s	+1%
3	50	14.97 s	16.99 s	+13%	15.16 s	+1%

Cause in this PR. JFR on the token batch, compared by sample count across the same 20 games (base 4,236 samples, this PR 5,669):

• ReplacementHandler.getReplacementList rises from 668 samples to 1,723, and GameEntity.staticDamagePrevention from 218 to 528. Together they account for 95% of the increase.
• Card.streamReplacementEffects is on the stack in 1,347 samples, 24% of the run. About 480 samples of self time are inside the stream library itself (Stream.concat spliterators, pipeline setup and findFirst), against about 10 on the base.
• The base skipped cards with no replacement effects before the zone lookup and the last-known-information checks. With that skip removed, every card in the game takes the full path, and the scan's own time rises from 8 samples to 140.
• staticDamagePrevention no longer returns as soon as it finds a full prevention effect, so it keeps building a stream for each remaining card.

The modified version. It keeps this PR's split: the cached list holds only what the card's traits produce, and the Adventure, Omen, Vengeance and counter effects are added when the list is read. It changes how the hot paths read that split:

1. Card and CardState get a visitReplacementEffects(Visitor) that walks the cached list by index and then the rule effects, without allocating. getReplacementList, getTotalPreventionShieldAmount and isPreventCombatDamageThisTurn use it. streamReplacementEffects stays, built from the visitor.
2. The empty-card skip in getReplacementList is restored through a new Card.hasReplacementEffects(). It also checks the three subtypes and three counters, because the cached list can be empty while the rule effects are not.
3. staticDamagePrevention returns to a plain loop with the early return 0, and skips cards with no effects.

The desktop test suite passes on it (1,279 tests). JFR on the same token batch puts it at 4,229 samples against the base's 4,236. The stream frames are gone. The cached list itself is cheaper than the base (CardState.getReplacementEffects is about 110 samples lower), which looks like this PR's simpler cache key paying off. That gain is spent on the new empty check, though. hasReplacementEffects is in 820 samples, 250 of them in three CardType.hasSubtype lookups per card, which the base folded into its cache key. To come out ahead of the base, the subtype answer would probably need to be cached alongside the list. That has not been measured.

Diff of the modified version against this PR

--- a/forge-game/src/main/java/forge/game/GameEntity.java
+++ b/forge-game/src/main/java/forge/game/GameEntity.java
@@ -106,32 +105,31 @@
         for (final Card ca : getGame().getCardsIn(ZoneType.STATIC_ABILITIES_SOURCE_ZONES)) {
-            Stream<ReplacementEffect> filteredStream = ca.streamReplacementEffects().filter(re -> {
+            if (!ca.hasReplacementEffects()) {
+                continue;
+            }
+            for (final ReplacementEffect re : ca.streamReplacementEffects().toList()) {
                 if (!re.getMode().equals(ReplacementType.DamageDone) ||
                         (!re.hasParam("PreventionEffect") && !re.hasParam("Prevent"))) {
-                    return false;
+                    continue;
                 }
 (the remaining `return false` checks become `continue` the same way)
-                return true;
-            });
-
-            damage = filteredStream.reduce(damage, (d, re) -> {
                 if (re.hasParam("Prevent")) {
                     return 0;
                 } else if (re.getOverridingAbility() != null) {
                     SpellAbility repSA = re.getOverridingAbility();
                     if (repSA.getApi() == ApiType.ReplaceDamage) {
-                        return Math.max(0, d - AbilityUtils.calculateAmount(ca, repSA.getParam("Amount"), repSA));
+                        damage = Math.max(0, damage - AbilityUtils.calculateAmount(ca, repSA.getParam("Amount"), repSA));
                     }
-                    return d;
                 } else {
                     return 0;
                 }
-            }, Integer::sum);
+            }
         }
--- a/forge-game/src/main/java/forge/game/card/Card.java
+++ b/forge-game/src/main/java/forge/game/card/Card.java
     public Stream<ReplacementEffect> streamReplacementEffects() {
-        Stream<ReplacementEffect> result = currentState.streamReplacementEffects();
-        Stream.Builder<ReplacementEffect> sb = Stream.builder();
+        final List<ReplacementEffect> all = new ArrayList<>();
+        visitReplacementEffects(all::add);
+        return all.stream();
+    }
+
+    /** Whether the card has any replacement effect, counting those its subtypes and counters add. */
+    public boolean hasReplacementEffects() {
+        return !currentState.getReplacementEffects().isEmpty() || currentState.hasRuleReplacementEffects()
+                || getCounters(CounterEnumType.SHIELD) > 0 || getCounters(CounterEnumType.STUN) > 0
+                || getCounters(CounterEnumType.FINALITY) > 0;
+    }
+
+    /** Visits the state's effects, then those the counters add. Returns false if the visitor stopped. */
+    public boolean visitReplacementEffects(final Visitor<ReplacementEffect> visitor) {
+        if (!currentState.visitReplacementEffects(visitor)) {
+            return false;
+        }
         // Shield Counter aren't affected by Changed Card Traits
 (counter effect creation unchanged)
-            sb.add(shieldCounterReplaceDamage);
-            sb.add(shieldCounterReplaceDestroy);
+            if (!visitor.visit(shieldCounterReplaceDamage) || !visitor.visit(shieldCounterReplaceDestroy)) {
+                return false;
+            }
-            sb.add(stunCounterReplaceUntap);
+            if (!visitor.visit(stunCounterReplaceUntap)) {
+                return false;
+            }
-            sb.add(finalityCounterReplaceDying);
+            return visitor.visit(finalityCounterReplaceDying);
         }
-        return Stream.concat(result, sb.build());
+        return true;
     }
--- a/forge-game/src/main/java/forge/game/card/CardState.java
+++ b/forge-game/src/main/java/forge/game/card/CardState.java
     public Stream<ReplacementEffect> streamReplacementEffects() {
-        Stream<ReplacementEffect> result = getReplacementEffects().stream();
-        Stream.Builder<ReplacementEffect> sb = Stream.builder();
+        final List<ReplacementEffect> all = new java.util.ArrayList<>();
+        visitReplacementEffects(all::add);
+        return all.stream();
+    }
+
+    public boolean hasRuleReplacementEffects() {
+        return type.hasSubtype("Adventure") || type.hasSubtype("Omen") || type.hasSubtype("Vengeance");
+    }
+
+    /** Visits the cached effects, then those the rules add. Returns false if the visitor stopped. */
+    public boolean visitReplacementEffects(final Visitor<ReplacementEffect> visitor) {
+        if (!visitor.visitAll(getReplacementEffects())) {
+            return false;
+        }
         // below are global rules
 (rule effect creation unchanged)
-            sb.add(adventureRep);
+            if (!visitor.visit(adventureRep)) {
+                return false;
+            }
-            sb.add(omenRep);
+            if (!visitor.visit(omenRep)) {
+                return false;
+            }
-            sb.add(vengeanceRep);
+            return visitor.visit(vengeanceRep);
         }
-        return Stream.concat(result, sb.build());
+        return true;
     }
--- a/forge-game/src/main/java/forge/game/replacement/ReplacementHandler.java
+++ b/forge-game/src/main/java/forge/game/replacement/ReplacementHandler.java
             boolean noLKIstate = c != crd || event != ReplacementType.Moved || c.isImmutable() || runParams.get(AbilityKey.LastStateBattlefield) == null;

+            // most cards have no replacement effects: skip them before the zone lookup
+            if (noLKIstate && !c.hasReplacementEffects()) {
+                return true;
+            }
+
             Zone cardZone = game.getZoneOf(c);
 ...
             final Zone finalCardZone = cardZone;
-            Stream<ReplacementEffect> filteredStream = c.streamReplacementEffects().filter(replacementEffect -> {
-                return !replacementEffect.hasRun()
+            // a CantHappen effect found stops the whole scan
+            return c.visitReplacementEffects(replacementEffect -> {
+                if (!replacementEffect.hasRun()
                         && (layer == null || replacementEffect.getLayer() == layer)
                         && replacementEffect.modeCheck(event, runParams)
                         && (hasRun.isEmpty() || !hasRun.contains(replacementEffect))
-                        //&& !possibleReplacers.contains(replacementEffect)
                         && replacementEffect.zonesCheck(finalCardZone)
                         && replacementEffect.requirementsCheck(game)
-                        && replacementEffect.canReplace(runParams);
-            });
-
-            if (layer == ReplacementLayer.CantHappen) {
-                Optional<ReplacementEffect> find = filteredStream.findFirst();
-                if (find.isPresent()) {
-                    possibleReplacers.add(find.get());
-                    return false;
-                }
-            } else {
-                filteredStream.forEach(possibleReplacers::add);
-            }
-
-            return true;
+                        && replacementEffect.canReplace(runParams)) {
+                    possibleReplacers.add(replacementEffect);
+                    return layer != ReplacementLayer.CantHappen;
+                }
+                return true;
+            });
 (getTotalPreventionShieldAmount and isPreventCombatDamageThisTurn switch from
  streamReplacementEffects().filter/anyMatch to visitReplacementEffects the same way)

@MostCromulent see main issue #11058
Vistor interface is exactly what i don't want, and i would prefer if I could use Stream instead

@MostCromulent re-added the hasReplacementEffects check

@Hanmac Re-ran the same profiling on the current head. Most of the slowdown is gone, but it is still behind the base:

Totals, mean of four repetitions (range), change against the base (1679040):

Workload	Base	This PR
Goblin tokens, 20 games	50.3 s (48.1–53.6)	53.8 s (53.0–55.1), +7.0%
Red-green creatures, 20 games	37.7 s (35.9–39.4)	40.1 s (40.0–40.5), +6.5%
Four-player Commander, 3 games	75.3 s (73.3–77.9)	78.9 s (77.5–80.8), +4.8%

The base's ranges were wider in this run, so these differences sit inside run-to-run noise on their own. Per game, though, this PR is slower on 18 of the 20 token games and on all 3 Commander games.

JFR on the token batch, compared by sample count across the same 20 games (base 4,211 samples, this PR 4,755):

• GameEntity.staticDamagePrevention rises from 241 samples to 557. It has no empty-card skip, so it still builds a stream for every card in the game. Card.streamReplacementEffects is on the stack in 265 samples and Stream.concat in 51, against none on the base.
• ReplacementHandler.getReplacementList rises from 656 to 826. The restored skip works, but Card.hasReplacementEffects (660 samples) checks the subtypes and counters before the cheap isEmpty(). So the three CardType.hasSubtype lookups run on every card, including cards that have effects of their own (CardState.hasRuleReplacementEffects, 423 samples).

Also ran a variant with further changes proposed by Claude, retaining the streaming architecture. Seems like this is basically level with master with any variation within ordinary game-to-game noise:

Totals, same run, change against the base:

Workload	Base	This PR	Variant
Goblin tokens, 20 games	50.3 s (48.1–53.6)	53.8 s (53.0–55.1), +7.0%	50.1 s (48.9–52.5), −0.3%
Red-green creatures, 20 games	37.7 s (35.9–39.4)	40.1 s (40.0–40.5), +6.5%	37.3 s (37.0–38.4), −0.8%
Four-player Commander, 3 games	75.3 s (73.3–77.9)	78.9 s (77.5–80.8), +4.8%	76.9 s (75.4–80.0), +2.1%

The variant is level with the base within noise. Per game it is slower on 9 of the 20 token games.

JFR on the same token batch: 4,303 samples against the base's 4,211. staticDamagePrevention is back to 265 samples, Card.streamReplacementEffects is down to 11, and Stream.concat does not appear. CardState.hasRuleReplacementEffects falls to 253. What remains is the empty check itself, which the base folded into its cache key.

The variant's changes

All three keep streamReplacementEffects as the way to read the effects:

1. staticDamagePrevention skips cards with no replacement effects, the same way getReplacementList now does.
2. CardState.streamReplacementEffects returns the cached list's stream directly when the card has none of the three subtypes, and Card.streamReplacementEffects does the same when it has none of the three counters. Most cards then skip the builder and the concat.
3. hasReplacementEffects checks isEmpty() before the subtypes and counters.

Diff against 46b56c4:

--- a/forge-game/src/main/java/forge/game/GameEntity.java
+++ b/forge-game/src/main/java/forge/game/GameEntity.java
@@ -105,6 +105,9 @@ public abstract class GameEntity implements GameObject, IIdentifiable {
         }

         for (final Card ca : getGame().getCardsIn(ZoneType.STATIC_ABILITIES_SOURCE_ZONES)) {
+            if (!ca.hasReplacementEffects()) {
+                continue;
+            }
             Stream<ReplacementEffect> filteredStream = ca.streamReplacementEffects().filter(re -> {
                 if (!re.getMode().equals(ReplacementType.DamageDone) ||
                         (!re.hasParam("PreventionEffect") && !re.hasParam("Prevent"))) {
--- a/forge-game/src/main/java/forge/game/card/Card.java
+++ b/forge-game/src/main/java/forge/game/card/Card.java
@@ -7171,8 +7171,23 @@ public class Card extends GameEntity implements Comparable<Card>, IHasSVars, ITr
         return currentState.getReplacementEffects();
     }

+    /** Whether the card has any replacement effect, counting those its subtypes and counters add. */
+    public boolean hasReplacementEffects() {
+        return !currentState.getReplacementEffects().isEmpty() || currentState.hasRuleReplacementEffects()
+                || hasCounterReplacementEffects();
+    }
+
+    private boolean hasCounterReplacementEffects() {
+        return getCounters(CounterEnumType.SHIELD) > 0 || getCounters(CounterEnumType.STUN) > 0
+                || getCounters(CounterEnumType.FINALITY) > 0;
+    }
+
     public Stream<ReplacementEffect> streamReplacementEffects() {
         Stream<ReplacementEffect> result = currentState.streamReplacementEffects();
+        // most cards have none of these counters, so they skip the builder and the concat
+        if (!hasCounterReplacementEffects()) {
+            return result;
+        }

         Stream.Builder<ReplacementEffect> sb = Stream.builder();

@@ -7235,19 +7250,6 @@ public class Card extends GameEntity implements Comparable<Card>, IHasSVars, ITr
         getUnhiddenKeywords(state).applyReplacementEffect(list);
     }

-    /** Whether the card has any replacement effect, counting those its subtypes and counters add. */
-    public boolean hasReplacementEffects() {
-        return hasRuleReplacementEffects() || !currentState.getReplacementEffects().isEmpty();
-    }
-
-    public boolean hasRuleReplacementEffects() {
-        if (currentState.hasRuleReplacementEffects()) {
-            return true;
-        }
-        return getCounters(CounterEnumType.SHIELD) > 0 || getCounters(CounterEnumType.STUN) > 0
-                || getCounters(CounterEnumType.FINALITY) > 0;
-    }
-
     /**
      * Returns what zone this card was cast from (from what zone it was moved to the stack).
      */
--- a/forge-game/src/main/java/forge/game/card/CardState.java
+++ b/forge-game/src/main/java/forge/game/card/CardState.java
@@ -897,6 +897,10 @@ public class CardState implements GameObject, IHasSVars, ITranslatable {

     public Stream<ReplacementEffect> streamReplacementEffects() {
         Stream<ReplacementEffect> result = getReplacementEffects().stream();
+        // most cards have none of these subtypes, so they skip the builder and the concat
+        if (!hasRuleReplacementEffects()) {
+            return result;
+        }

         Stream.Builder<ReplacementEffect> sb = Stream.builder();

@MostCromulent applied most suggestions.

I'm unsure about:

hasReplacementEffects checks isEmpty() before the subtypes and counters.