Performance: fixed server's big memory usage in long games and in big stack sizes (related to #11285, fixes #9302)

Mage.Tests/src/test/java/org/mage/test/AI/basic/GameStatePerformanceTest.java

@@ -0,0 +1,46 @@
+package org.mage.test.AI.basic;
+
+import mage.constants.PhaseStep;
+import mage.constants.Zone;
+import org.junit.Test;
+import org.mage.test.serverside.base.CardTestPlayerBaseWithAIHelps;
+
+/**
+ * @author JayDi85
+ */
+public class GameStatePerformanceTest extends CardTestPlayerBaseWithAIHelps {
+
+    @Test
+    public void test_StackObjects_BookmarkMustCleanDataAfterTriggerResolve() {
+        // memory overflow problem on too much stack objects
+        // related bug: https://github.com/magefree/mage/issues/9302
+        // go to "save(GameState gameState)" and enable size logs to test real usage
+        final int MAX_STACK_OBJECTS = 300;
+
+        // Whenever an artifact creature you control deals combat damage to a player,
+        // you may create a 1/1 blue Thopter artifact creature token with flying.
+        addCard(Zone.BATTLEFIELD, playerA, "Sharding Sphinx", MAX_STACK_OBJECTS);
+        //
+        // You can’t lose the game and your opponents can’t win the game.
+        addCard(Zone.BATTLEFIELD, playerB, "Platinum Angel", 1);
+        //
+        // Creatures can’t block this turn.
+        addCard(Zone.HAND, playerA, "Order // Chaos", 1); // instant, {2}{R}
+        addCard(Zone.BATTLEFIELD, playerA, "Mountain", 3);
+
+        // prepare not loose
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Chaos");
+        waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN);
+
+        // attack and use triggers
+        attack(1, playerA, "Sharding Sphinx");
+        setChoice(playerA, "Whenever an artifact creature you control", MAX_STACK_OBJECTS - 1); // triggers order
+        setChoice(playerA, true, MAX_STACK_OBJECTS); // triggers use
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+
+        assertPermanentCount(playerA, "Thopter Token", MAX_STACK_OBJECTS);
+    }
+}

Mage/src/main/java/mage/game/Game.java

@@ -488,12 +488,29 @@ public interface Game extends MageItem, Serializable, Copyable<Game> {
     //game transaction methods
     void saveState(boolean bookmark);
 
+    /**
+     * Save current game state and return bookmark to it
+     *
+     * @return
+     */
     int bookmarkState();
 
     GameState restoreState(int bookmark, String context);
 
+    /**
+     * Remove selected bookmark and all newer bookmarks and game states
+     * Part of restore/rollback lifecycle
+     *
+     * @param bookmark
+     */
     void removeBookmark(int bookmark);
 
+    /**
+     * TODO: remove logic changed, must research each usage of removeBookmark and replace it with new code
+     * @param bookmark
+     */
+    void removeBookmark_v2(int bookmark);
+
     int getSavedStateSize();
 
     boolean isSaveGame();

Mage/src/main/java/mage/game/GameImpl.java

@@ -118,7 +118,7 @@ public abstract class GameImpl implements Game {
     protected Map<UUID, Counters> enterWithCounters = new HashMap<>();
 
     protected GameState state;
-    private transient Stack<Integer> savedStates = new Stack<>();
+    private transient Stack<Integer> savedStates = new Stack<>(); // bookmarks - 0-base refs to gameStates
     protected transient GameStates gameStates = new GameStates();
 
     // game states to allow player rollback
@@ -934,8 +934,7 @@ public abstract class GameImpl implements Game {
     }
 
     @Override
-    public void removeBookmark(int bookmark
+    public void removeBookmark(int bookmark) {
-    ) {
         if (!simulation) {
             if (bookmark != 0) {
                 while (savedStates.size() > bookmark) {
@@ -946,6 +945,20 @@ public abstract class GameImpl implements Game {
         }
     }
 
+    @Override
+    public void removeBookmark_v2(int bookmark) {
+        if (!simulation) {
+            if (bookmark != 0) {
+                while (savedStates.size() >= bookmark) {
+                    int outdatedIndex = savedStates.pop();
+                    while (gameStates.getSize() - 1 >= outdatedIndex) {
+                        gameStates.remove(gameStates.getSize() - 1);
+                    }
+                }
+            }
+        }
+    }
+
     private void clearAllBookmarks() {
         if (!simulation) {
             while (!savedStates.isEmpty()) {

Mage/src/main/java/mage/game/GameStates.java

@@ -2,7 +2,7 @@
 package mage.game;
 
 import java.io.Serializable;
-import java.util.LinkedList;
+import java.util.ArrayList;
 import java.util.List;
 import org.apache.log4j.Logger;
 
@@ -16,13 +16,12 @@ public class GameStates implements Serializable {
     private final List<GameState> states;
 
     public GameStates() {
-        this.states = new LinkedList<>();
+        this.states = new ArrayList<>();
     }
 
     public void save(GameState gameState) {
-//        states.add(new Copier<GameState>().copyCompressed(gameState));
         states.add(gameState.copy());
-        logger.trace("Saved game state: " + states.size());
+        //logger.warn("states size: " + states.size());
     }
 
     public int getSize() {
@@ -35,7 +34,6 @@ public class GameStates implements Serializable {
                 states.remove(states.size() - 1);
             }
             logger.trace("Rolling back state: " + index);
-//            return new Copier<GameState>().uncompressCopy(states.get(index));
             return states.get(index);
         }
         return null;
@@ -52,7 +50,6 @@ public class GameStates implements Serializable {
 
     public GameState get(int index) {
         if (index < states.size()) {
-//             return new Copier<GameState>().uncompressCopy(states.get(index));
             return states.get(index);
         }
         return null;

Mage/src/main/java/mage/players/Player.java

@@ -38,7 +38,10 @@ import mage.util.Copyable;
 import mage.util.MultiAmountMessage;
 
 import java.io.Serializable;
-import java.util.*;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
 import java.util.stream.Collectors;
 
 /**
@@ -56,10 +59,7 @@ public interface Player extends MageItem, Copyable<Player> {
      * Default is PayLifeCostLevel.allAbilities.
      */
     enum PayLifeCostLevel {
-        allAbilities,
+        allAbilities, nonSpellnonActivatedAbilities, onlyManaAbilities, none
-        nonSpellnonActivatedAbilities,
-        onlyManaAbilities,
-        none
     }
 
     /**
@@ -209,6 +209,7 @@ public interface Player extends MageItem, Copyable<Player> {
     Cards getHand();
 
     void incrementLandsPlayed();
+
     void resetLandsPlayed();
 
     int getLandsPlayed();
@@ -319,7 +320,7 @@ public interface Player extends MageItem, Copyable<Player> {
      *
      * @param game
      * @param playerUnderControlId
-     * @param info additional info to show in game logs like source
+     * @param info                 additional info to show in game logs like source
      */
     void controlPlayersTurn(Game game, UUID playerUnderControlId, String info);
 
@@ -556,8 +557,18 @@ public interface Player extends MageItem, Copyable<Player> {
 
     int getStoredBookmark();
 
+    /**
+     * Save player's bookmark for undo, e.g. enable undo button on mana payment
+     *
+     * @param bookmark
+     */
     void setStoredBookmark(int bookmark);
 
+    /**
+     * Reset player's bookmark, e.g. disable undo button
+     *
+     * @param game
+     */
     void resetStoredBookmark(Game game);
 
     default GameState restoreState(int bookmark, String text, Game game) {
@@ -746,10 +757,8 @@ public interface Player extends MageItem, Copyable<Player> {
      * @param game     Game
      * @return List of integers with size equal to messages.size().  The sum of the integers is equal to max.
      */
-    default List<Integer> getMultiAmount(Outcome outcome, List<String> messages, int min, int max, MultiAmountType type,
+    default List<Integer> getMultiAmount(Outcome outcome, List<String> messages, int min, int max, MultiAmountType type, Game game) {
-            Game game) {
+        List<MultiAmountMessage> constraints = messages.stream().map(s -> new MultiAmountMessage(s, 0, max)).collect(Collectors.toList());
-        List<MultiAmountMessage> constraints = messages.stream().map(s -> new MultiAmountMessage(s, 0, max))
-                .collect(Collectors.toList());
 
         return getMultiAmountWithIndividualConstraints(outcome, constraints, min, max, type, game);
     }
@@ -765,8 +774,7 @@ public interface Player extends MageItem, Copyable<Player> {
      * @param game     Game
      * @return List of integers with size equal to messages.size().  The sum of the integers is equal to max.
      */
-    List<Integer> getMultiAmountWithIndividualConstraints(Outcome outcome, List<MultiAmountMessage> messages, int min,
+    List<Integer> getMultiAmountWithIndividualConstraints(Outcome outcome, List<MultiAmountMessage> messages, int min, int max, MultiAmountType type, Game game);
-            int max, MultiAmountType type, Game game);
 
     void sideboard(Match match, Deck deck);
 
@@ -1060,10 +1068,10 @@ public interface Player extends MageItem, Copyable<Player> {
      * without mana (null) or the mana set to manaCosts instead of its normal
      * mana costs.
      *
-     * @param sourceId  the source that can be cast without mana
+     * @param sourceId   the source that can be cast without mana
-     * @param manaCosts alternate ManaCost, null if it can be cast without mana
+     * @param manaCosts  alternate ManaCost, null if it can be cast without mana
-     *                  cost
+     *                   cost
-     * @param costs     alternate other costs you need to pay
+     * @param costs      alternate other costs you need to pay
      * @param identifier if not using the MageIdentifier.Default, only apply the alternate mana when ApprovingSource if of that kind.
      */
     void setCastSourceIdWithAlternateMana(UUID sourceId, ManaCosts<ManaCost> manaCosts, Costs<Cost> costs, MageIdentifier identifier);

Mage/src/main/java/mage/players/PlayerImpl.java

@@ -1613,7 +1613,7 @@ public abstract class PlayerImpl implements Player, Serializable {
                             ability.getId(), ability, ability.getControllerId()
                     ));
                 }
-                game.removeBookmark(bookmark);
+                game.removeBookmark_v2(bookmark);
                 return true;
             }
         }

Mage/src/main/java/mage/util/Copier.java

@@ -51,39 +51,4 @@ public class Copier<T> {
         return copy;
 
     }
-
-    public byte[] copyCompressed(T obj) {
-        FastByteArrayOutputStream fbos = null;
-        ObjectOutputStream out = null;
-        try {
-            fbos = new FastByteArrayOutputStream();
-            out = new ObjectOutputStream(new GZIPOutputStream(fbos));
-
-            // Write the object out to a byte array
-            out.writeObject(obj);
-            out.flush();
-
-            byte[] copy = new byte[fbos.getSize()];
-            System.arraycopy(fbos.getByteArray(), 0, copy, 0, fbos.getSize());
-            return copy;
-        }
-        catch(IOException e) {
-            e.printStackTrace();
-        } finally {
-            StreamUtils.closeQuietly(fbos);
-            StreamUtils.closeQuietly(out);
-        }
-        return null;
-    }
-
-    public T uncompressCopy(byte[] buffer) {
-        T copy = null;
-        try (ObjectInputStream in = new CopierObjectInputStream(loader, new GZIPInputStream(new ByteArrayInputStream(buffer)))) {
-            copy = (T) in.readObject();
-        }
-        catch(IOException | ClassNotFoundException e) {
-            e.printStackTrace();
-        }
-        return copy;
-    }
 }