Skip to content

Proposition: Put some expensive queries into cache #8916

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.

Sign up for GitHub

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

Draft
Eradev wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Proposition: Put some expensive queries into cache #8916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2565676
Put some XCount into a cache
Eradev Oct 13, 2025
bf1ce2f
Update Realmbreaker, the Invasion Tree (Warning because SVar starts w…
Eradev Oct 13, 2025
476e092
Ensure player zones clear cache
Eradev Oct 13, 2025
bc2962d
Cleanup
Eradev Oct 13, 2025
a4d4e5a
Cache clear + do not cache certain on keywords
Eradev Oct 13, 2025
2662a04
Cleanup
Eradev Oct 13, 2025
7de9b48
Update The Death of Gwen Stacy
Eradev Oct 13, 2025
a2544c6
Update LTR
Eradev Oct 13, 2025
dd56ac3
Cache cantAttack CardCollectionView
Eradev Oct 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions forge-game/src/main/java/forge/game/Game.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,7 @@ public int getPosition(Player player, Player startingPlayer) {
}

public void onPlayerLost(Player p) {
clearShortLivedCaches();
//set for Avatar
p.setHasLost(true);
// Rule 800.4 Losing a Multiplayer game
Expand Down Expand Up @@ -1378,4 +1379,12 @@ public int getAITimeout() {
public boolean canUseTimeout() {
return AI_CAN_USE_TIMEOUT;
}

/**
* Reset short lived caches that should not persist between turns or phases.
*/
public void clearShortLivedCaches() {
getPlayers().forEach(p -> p.clearCache());
phaseHandler.clearCache();
}
}
2 changes: 1 addition & 1 deletion forge-game/src/main/java/forge/game/ability/AbilityFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ private static AbilitySub getSubAbility(CardState state, String sSub, final IHas
if (sVarHolder.hasSVar(sSub)) {
return (AbilitySub) AbilityFactory.getAbility(state, sSub, sVarHolder);
}
System.out.println("SubAbility '"+ sSub +"' not found for: " + state.getName());
System.err.println("SubAbility '"+ sSub +"' not found for: " + state.getName());

return null;
}
Expand Down
61 changes: 49 additions & 12 deletions forge-game/src/main/java/forge/game/ability/AbilityUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ else if (ability != null) {
svarval = ability.getSVar(amount);
}
if (StringUtils.isBlank(svarval)) {
if ((ability != null) && (ability instanceof SpellAbility) && !(ability instanceof SpellPermanent)) {
if ((ability instanceof SpellAbility) && !(ability instanceof SpellPermanent)) {
System.err.printf("SVar '%s' not found in ability, fallback to Card (%s). Ability is (%s)%n", amount, card.getName(), ability);
}
svarval = card.getSVar(amount);
Expand Down Expand Up @@ -1583,6 +1583,7 @@ public static void handleRemembering(final SpellAbility sa) {
* @return a int.
*/
public static int xCount(Card c, final String s, final CardTraitBase ctb) {
String cacheKey = "xCount_" + s;
final String s2 = applyAbilityTextChangeEffects(s, ctb);
final String[] l = s2.split("/");
final String expr = CardFactoryUtil.extractOperators(s2);
Expand All @@ -1597,6 +1598,13 @@ public static int xCount(Card c, final String s, final CardTraitBase ctb) {
}
}

if (player != null) {
Integer cachedValue = player.getFromCache(cacheKey);
if (cachedValue != null) {
return cachedValue;
}
}

// accept straight numbers
if (l[0].startsWith("Number$")) {
final String number = l[0].substring(7);
Expand All @@ -1610,6 +1618,7 @@ public static int xCount(Card c, final String s, final CardTraitBase ctb) {
if (l[0].startsWith("SVar$")) {
String n = l[0].substring(5);
String v = ctb == null ? c.getSVar(n) : ctb.getSVar(n);

return doXMath(xCount(c, v, ctb), expr, c, ctb);
}

Expand Down Expand Up @@ -1947,7 +1956,8 @@ public static int xCount(Card c, final String s, final CardTraitBase ctb) {
}
}
colorOccurrences += player.getDevotionMod();
return doXMath(colorOccurrences, expr, c, ctb);

return computeAndCache(player, cacheKey, doXMath(colorOccurrences, expr, c, ctb));
}
} // end ctb != null

Expand Down Expand Up @@ -2586,7 +2596,8 @@ public static int xCount(Card c, final String s, final CardTraitBase ctb) {
count++;
}
}
return doXMath(count, expr, c, ctb);

return computeAndCache(player, cacheKey, doXMath(count, expr, c, ctb));
}

if (sq[0].contains("Party")) {
Expand Down Expand Up @@ -2718,7 +2729,7 @@ public static int xCount(Card c, final String s, final CardTraitBase ctb) {
}
}

return doXMath(colorOcurrencices, expr, c, ctb);
return computeAndCache(player, cacheKey, doXMath(colorOcurrencices, expr, c, ctb));
}

if (l[0].contains("ExactManaCost")) {
Expand All @@ -2737,7 +2748,7 @@ public static int xCount(Card c, final String s, final CardTraitBase ctb) {
}
manaCost.remove(ManaCost.NO_COST.getShortString());

return doXMath(manaCost.size(), expr, c, ctb);
return computeAndCache(player, cacheKey, doXMath(manaCost.size(), expr, c, ctb));
}

if (sq[0].equals("StormCount")) {
Expand Down Expand Up @@ -2866,7 +2877,8 @@ public static int xCount(Card c, final String s, final CardTraitBase ctb) {
max = entry.getValue();
}
}
return max;

return computeAndCache(player, cacheKey, max);
}

if (sq[0].startsWith("MostProminentCreatureType")) {
Expand All @@ -2889,7 +2901,8 @@ public static int xCount(Card c, final String s, final CardTraitBase ctb) {
.filter(CardPredicates.restriction(restriction, player, c, ctb))
.map(Card::getNetPower)
.distinct().count();
return doXMath(uniquePowers, expr, c, ctb);

return computeAndCache(player, cacheKey, doXMath(uniquePowers, expr, c, ctb));
}
if (sq[0].startsWith("DifferentCounterKinds_")) {
final Set<CounterType> kinds = Sets.newHashSet();
Expand All @@ -2898,7 +2911,8 @@ public static int xCount(Card c, final String s, final CardTraitBase ctb) {
for (final Card card : list) {
kinds.addAll(card.getCounters().keySet());
}
return doXMath(kinds.size(), expr, c, ctb);

return computeAndCache(player, cacheKey, doXMath(kinds.size(), expr, c, ctb));
}

// Complex counting methods
Expand All @@ -2912,7 +2926,26 @@ public static int xCount(Card c, final String s, final CardTraitBase ctb) {
num = Iterables.size(someCards);
}

return doXMath(num, expr, c, ctb);
return computeAndCache(player, cacheKey, doXMath(num, expr, c, ctb));
}

/**
Caches the computed value if possible and returns it.
*/
private static Integer computeAndCache(Player p, String key, int value) {
if (p == null){
return value;
}

if (key.contains("Remembered") ||
key.contains("Triggered") ||
key.contains("Chosen")) {
return value;
}

p.putInCache(key, value);

return value;
}

public static final void applyManaColorConversion(ManaConversionMatrix matrix, String conversion) {
Expand Down Expand Up @@ -3402,17 +3435,21 @@ public static int playerXCount(final List<Player> players, final String s, final
}

public static int playerXProperty(final Player player, final String s, final Card source, CardTraitBase ctb) {

final String cacheKey = "playerXProperty_" + s;
final String[] l = s.split("/");
final String m = CardFactoryUtil.extractOperators(s);

final Game game = player.getGame();

if (player.getFromCache(cacheKey) != null) {
return player.getFromCache(cacheKey);
}

// count valid cards on the battlefield
if (l[0].startsWith("Valid ")) {
final String restrictions = l[0].substring(6);
int num = CardLists.getValidCardCount(game.getCardsIn(ZoneType.Battlefield), restrictions, player, source, ctb);
return doXMath(num, m, source, ctb);
return computeAndCache(player, cacheKey, doXMath(num, m, source, ctb));
}

// count valid cards in any specified zone/s
Expand All @@ -3421,7 +3458,7 @@ public static int playerXProperty(final Player player, final String s, final Car
final List<ZoneType> vZone = ZoneType.listValueOf(lparts[0].split("Valid")[1]);
String restrictions = TextUtil.fastReplace(l[0], TextUtil.addSuffix(lparts[0]," "), "");
int num = CardLists.getValidCardCount(game.getCardsIn(vZone), restrictions, player, source, ctb);
return doXMath(num, m, source, ctb);
return computeAndCache(player, cacheKey, doXMath(num, m, source, ctb));
}

if (l[0].startsWith("ThisTurnEntered")) {
Expand Down
20 changes: 20 additions & 0 deletions forge-game/src/main/java/forge/game/phase/PhaseHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ public class PhaseHandler implements java.io.Serializable {

private final transient Game game;

// Cached values for this phase
private transient Map<String, CardCollectionView> cachedValuesMap = Maps.newHashMap();

public PhaseHandler(final Game game0) {
game = game0;
Expand Down Expand Up @@ -150,6 +152,8 @@ private void advanceToNextPhase() {
boolean isTopsy = playerTurn.isPhasesReversed();
boolean turnEnded = false;

cachedValuesMap.clear();

game.getStack().clearUndoStack(); //can't undo action from previous phase

if (bRepeatCleanup) { // for when Cleanup needs to repeat itself
Expand All @@ -175,6 +179,7 @@ private void advanceToNextPhase() {
if (turnEnded) {
turn++;
extraPhases.clear();
game.clearShortLivedCaches();
game.updateTurnForView();
game.fireEvent(new GameEventTurnBegan(playerTurn, turn));

Expand Down Expand Up @@ -1324,4 +1329,19 @@ private void handleMultiplayerEffects() {
}
}
}

/**
* Get the cache of CardCollectionView objects used to optimize repeated calls
* @return
*/
public Map<String, CardCollectionView> getCache() {
return cachedValuesMap;
}

/**
* Clear the cache of CardCollectionView objects
*/
public void clearCache() {
cachedValuesMap.clear();
}
}
16 changes: 16 additions & 0 deletions forge-game/src/main/java/forge/game/player/Player.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,9 @@ public class Player extends GameEntity implements Comparable<Player> {
private final AchievementTracker achievementTracker = new AchievementTracker();
private final PlayerView view;

// Query cache to avoid repeated expensive queries
private final Map<String, Integer> queryCache = new HashMap<>();

public Player(String name0, Game game0, final int id0) {
super(id0);

Expand Down Expand Up @@ -433,6 +436,7 @@ else if (newLife > life) {
else { // life == newLife
change = false;
}

return change;
}

Expand Down Expand Up @@ -4088,4 +4092,16 @@ public void triggerElementalBend(TriggerType type) {
public boolean hasAllElementBend() {
return elementalBendThisTurn.size() >= 4;
}

public Integer getFromCache(String queryKey) {
return queryCache.get(queryKey);
}

public void putInCache(String queryKey, int value) {
queryCache.put(queryKey, value);
}

public void clearCache() {
queryCache.clear();
}
}
23 changes: 21 additions & 2 deletions forge-game/src/main/java/forge/game/staticability/StaticAbilityCantAttackBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
*/
package forge.game.staticability;

import java.util.Map;
import java.util.stream.Collectors;

import org.apache.commons.lang3.tuple.MutablePair;
import org.apache.commons.lang3.tuple.Pair;

Expand All @@ -25,13 +28,14 @@
import forge.game.ability.AbilityUtils;
import forge.game.card.Card;
import forge.game.card.CardCollection;
import forge.game.card.CardCollectionView;
import forge.game.cost.Cost;
import forge.game.keyword.Keyword;
import forge.game.player.Player;
import forge.game.zone.ZoneType;

/**
* The Class StaticAbility_CantBeCast.
* The Class StaticAbility_CantAttackBlock.
*/
public class StaticAbilityCantAttackBlock {

Expand All @@ -42,7 +46,22 @@ public static boolean cantAttack(final Card attacker, final GameEntity defender)
return true;
}

for (final Card ca : attacker.getGame().getCardsIn(ZoneType.STATIC_ABILITIES_SOURCE_ZONES)) {
final String cacheKey = "cantAttack_CantAttackAbilities_" + attacker.getGame().getPhaseHandler().getPhase();
final Map<String, CardCollectionView> cache = attacker.getGame().getPhaseHandler().getCache();
CardCollectionView collectionView = null;

if (cache.containsKey(cacheKey)) {
collectionView = cache.get(cacheKey);
} else {
collectionView = attacker.getGame().getCardsIn(ZoneType.STATIC_ABILITIES_SOURCE_ZONES)
.stream()
.filter(c -> c.getStaticAbilities().stream()
.anyMatch(ab -> ab.checkConditions(StaticAbilityMode.CantAttack)))
.collect(Collectors.toCollection(CardCollection::new));
cache.put(cacheKey, collectionView);
}

for (final Card ca : collectionView) {
for (final StaticAbility stAb : ca.getStaticAbilities()) {
if (!stAb.checkConditions(StaticAbilityMode.CantAttack)) {
continue;
Expand Down
2 changes: 2 additions & 0 deletions forge-game/src/main/java/forge/game/zone/PlayerZone.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ public PlayerZone(final ZoneType zone, final Player inPlayer) {

@Override
protected void onChanged() {
super.onChanged();

if (getZoneType() == ZoneType.Hand && player.getController().isOrderedZone()) {
sort();
}
Expand Down
1 change: 1 addition & 0 deletions forge-game/src/main/java/forge/game/zone/Zone.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public Zone(final ZoneType zone0, Game game0) {
}

protected void onChanged() {
game.clearShortLivedCaches();
}

public Player getPlayer() { // generic zones like stack have no player associated
Expand Down
4 changes: 2 additions & 2 deletions forge-gui/res/cardsfolder/r/realmbreaker_the_invasion_tree.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ SVar:DBChangeZone:DB$ ChangeZone | Origin$ Graveyard | Destination$ Battlefield
SVar:DBAnimate:DB$ Animate | Replacements$ ReplaceLeaves | Defined$ Remembered | Duration$ Permanent | SubAbility$ DBCleanup | StackDescription$ None
SVar:ReplaceLeaves:Event$ Moved | ActiveZones$ Battlefield | Origin$ Battlefield | ValidCard$ Card.Self | ReplaceWith$ Exile | Description$ If this land would leave the battlefield, exile it instead of putting it anywhere else.
SVar:Exile:DB$ ChangeZone | Origin$ Battlefield | Destination$ Exile | Defined$ ReplacedCard
A:AB$ ChangeZone | Cost$ 10 T Sac<1/CARDNAME> | Origin$ Library | Destination$ Battlefield | ChangeType$ Praetor | ChangeNum$ XFetch | StackDescription$ {p:You} searches their library for any number of Praetor cards, puts them onto the battlefield, then shuffles. | SpellDescription$ Search your library for any number of Praetor cards, put them onto the battlefield, then shuffle.
A:AB$ ChangeZone | Cost$ 10 T Sac<1/CARDNAME> | Origin$ Library | Destination$ Battlefield | ChangeType$ Praetor | ChangeNum$ FetchCount | StackDescription$ {p:You} searches their library for any number of Praetor cards, puts them onto the battlefield, then shuffles. | SpellDescription$ Search your library for any number of Praetor cards, put them onto the battlefield, then shuffle.
SVar:DBCleanup:DB$ Cleanup | ClearRemembered$ True
SVar:XFetch:Count$ValidLibrary Praetor.YouCtrl
SVar:FetchCount:Count$ValidLibrary Praetor.YouCtrl
DeckHas:Ability$Mill|Sacrifice
DeckHints:Type$Praetor
Oracle:{2}, {T}: Target opponent mills three cards. Put a land card from their graveyard onto the battlefield tapped under your control. It gains "If this land would leave the battlefield, exile it instead of putting it anywhere else."\n{10}, {T}, Sacrifice Realmbreaker, the Invasion Tree: Search your library for any number of Praetor cards, put them onto the battlefield, then shuffle.
2 changes: 1 addition & 1 deletion forge-gui/res/cardsfolder/t/the_death_of_gwen_stacy.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ SVar:DBDiscard:DB$ Discard | Defined$ Player.NotedForDiscard | Mode$ TgtChoose |
SVar:DBLoseLife:DB$ LoseLife | Defined$ NonRememberedOwner | LifeAmount$ 3 | SubAbility$ DBCleanup
SVar:DBCleanup:DB$ Cleanup | ClearRemembered$ True | SubAbility$ DBClearNotes
SVar:DBClearNotes:DB$ Pump | Defined$ Player | ClearNotedCardsFor$ Discard
SVar:DBExile:DB$ ChangeZoneAll | Origin$ Graveyard | Destination$ Exile | ValidTgts$ Player | TgtPrompt$ Select any number of target players | TargetMin$ 0 | TargetMax$ MaxTgt | ChangeType$ Card | SubAbility$ DBDraw | StackDescription$ Exile graveyards ({p:Targeted}). | SpellDescription$ Exile any number of target players' graveyards.
SVar:DBExile:DB$ ChangeZoneAll | Origin$ Graveyard | Destination$ Exile | ValidTgts$ Player | TgtPrompt$ Select any number of target players | TargetMin$ 0 | TargetMax$ MaxTgt | ChangeType$ Card | StackDescription$ Exile graveyards ({p:Targeted}). | SpellDescription$ Exile any number of target players' graveyards.
DeckHas:Ability$Discard
SVar:MaxTgt:PlayerCountPlayers$Amount
Oracle:(As this Saga enters and after your draw step, add a lore counter. Sacrifice after III.)\nI — Destroy target creature.\nII — Each player may discard a card. Each player who doesn't loses 3 life.\nIII — Exile any number of target players' graveyards.
3 changes: 3 additions & 0 deletions forge-gui/res/editions/The Lord of the Rings Tales of Middle-earth.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -878,3 +878,6 @@ A-246 M A-The One Ring @Veli Nyström
10 c_a_food_sac @Randy Gallegos
11 c_a_food_sac @Randy Gallegos
12 c_a_treasure_sac @Valera Lutfullina

[other]
13 the_ring @Viko Menezes