From 69261303c5c30d4208c8aab0f8e8ccfb7f49439b Mon Sep 17 00:00:00 2001 From: kske Date: Thu, 19 Sep 2019 21:31:24 +0200 Subject: [PATCH] Fixed memory leak, improved copy constructors --- src/dev/kske/chess/board/Board.java | 53 ++++++++++------------ src/dev/kske/chess/board/Log.java | 46 +++++++++++++------ src/dev/kske/chess/game/Game.java | 3 ++ src/dev/kske/chess/game/NaturalPlayer.java | 2 +- src/dev/kske/chess/game/ai/AIPlayer.java | 4 +- test/dev/kske/chess/board/BoardTest.java | 5 +- test/dev/kske/chess/board/LogTest.java | 10 +++- 7 files changed, 71 insertions(+), 52 deletions(-) diff --git a/src/dev/kske/chess/board/Board.java b/src/dev/kske/chess/board/Board.java index 1a21ff4..a0b86ee 100644 --- a/src/dev/kske/chess/board/Board.java +++ b/src/dev/kske/chess/board/Board.java @@ -15,7 +15,7 @@ import dev.kske.chess.board.Piece.Type; * Created: 01.07.2019
* Author: Kai S. K. Engelbart */ -public class Board implements Cloneable { +public class Board { private Piece[][] boardArr = new Piece[8][8]; private Map kingPos = new HashMap<>(); @@ -75,6 +75,30 @@ public class Board implements Cloneable { initFromFEN(fen); } + /** + * Creates a copy of another {@link Board} instance.
+ * The created object is a deep copy, but does not contain any move history + * apart from the current {@link MoveNode}. + * + * @param other The {@link Board} instance to copy + */ + public Board(Board other) { + boardArr = new Piece[8][8]; + for (int i = 0; i < 8; i++) + for (int j = 0; j < 8; j++) { + if (other.boardArr[i][j] == null) continue; + boardArr[i][j] = (Piece) other.boardArr[i][j].clone(); + boardArr[i][j].board = this; + } + + kingPos.putAll(other.kingPos); + Map whiteCastling = new HashMap<>(other.castlingRights.get(Color.WHITE)), + blackCastling = new HashMap<>(other.castlingRights.get(Color.BLACK)); + castlingRights.put(Color.WHITE, whiteCastling); + castlingRights.put(Color.BLACK, blackCastling); + log = new Log(other.log, false); + } + /** * Moves a piece across the board if the move is legal. * @@ -523,33 +547,6 @@ public class Board implements Cloneable { return sb.toString(); } - /** - * @return A new instance of this class with a shallow copy of both - * {@code kingPos} and {code boardArr} - */ - @Override - public Object clone() { - Board board = null; - try { - board = (Board) super.clone(); - } catch (CloneNotSupportedException ex) { - ex.printStackTrace(); - } - board.boardArr = new Piece[8][8]; - for (int i = 0; i < 8; i++) - for (int j = 0; j < 8; j++) { - if (boardArr[i][j] == null) continue; - board.boardArr[i][j] = (Piece) boardArr[i][j].clone(); - board.boardArr[i][j].board = board; - } - - board.kingPos = new HashMap<>(); - board.kingPos.putAll(kingPos); - board.log = new Log(log); - - return board; - } - /** * @param pos The position from which to return a piece * @return The piece at the position diff --git a/src/dev/kske/chess/board/Log.java b/src/dev/kske/chess/board/Log.java index c05ae3f..95f9e43 100644 --- a/src/dev/kske/chess/board/Log.java +++ b/src/dev/kske/chess/board/Log.java @@ -24,12 +24,15 @@ public class Log { } /** - * Creates a partial deep copy of another {@link Log} instance which begins with - * the current node. + * Creates a (partially deep) copy of another {@link Log} instance which begins + * with the current {@link MoveNode}. * - * @param other The {@link Log} instance to copy + * @param other The {@link Log} instance to copy + * @param copyVariations If set to {@code true}, subsequent variations of the + * current {@link MoveNode} are copied with the + * {@link Log} */ - public Log(Log other) { + public Log(Log other, boolean copyVariations) { enPassant = other.enPassant; activeColor = other.activeColor; fullmoveCounter = other.fullmoveCounter; @@ -37,7 +40,7 @@ public class Log { // The new root is the current node of the copied instance if (!other.isEmpty()) { - root = new MoveNode(other.current); + root = new MoveNode(other.current, copyVariations); root.parent = null; current = root; } @@ -73,6 +76,7 @@ public class Log { */ public void removeLast() { if (!isEmpty() && current.parent != null) { + current.parent.variations.remove(current); current = current.parent; activeColor = current.activeColor; enPassant = current.enPassant; @@ -131,10 +135,10 @@ public class Log { public final int fullmoveCounter, halfmoveClock; private MoveNode parent; - private List variations = new ArrayList<>(); + private List variations; /** - * Creates a new {@link MoveNode} + * Creates a new {@link MoveNode}. * * @param move The logged {@link Move} * @param capturedPiece The {@link Piece} captures by the logged {@link Move} @@ -155,18 +159,24 @@ public class Log { } /** - * Creates a deep copy of another {@link MoveNode}. + * Creates a (deep) copy of another {@link MoveNode}. * - * @param other The {@link MoveNode} to copy + * @param other The {@link MoveNode} to copy + * @param copyVariations When this is set to {@code true} a deep copy is + * created, which + * considers subsequent variations */ - public MoveNode(MoveNode other) { + public MoveNode(MoveNode other, boolean copyVariations) { this(other.move, other.capturedPiece, other.enPassant, other.activeColor, other.fullmoveCounter, other.halfmoveClock); - other.variations.forEach(variation -> { - MoveNode copy = new MoveNode(variation); - copy.parent = this; - variations.add(copy); - }); + if (copyVariations && other.variations != null) { + if (variations == null) variations = new ArrayList<>(); + other.variations.forEach(variation -> { + MoveNode copy = new MoveNode(variation, true); + copy.parent = this; + variations.add(copy); + }); + } } /** @@ -175,10 +185,16 @@ public class Log { * @param variation The {@link MoveNode} to append to this {@link MoveNode} */ public void addVariation(MoveNode variation) { + if (variations == null) variations = new ArrayList<>(); if (!variations.contains(variation)) { variations.add(variation); variation.parent = this; } } + + /** + * @return A list of all variations associated with this {@link MoveNode} + */ + public List getVariations() { return variations; } } } \ No newline at end of file diff --git a/src/dev/kske/chess/game/Game.java b/src/dev/kske/chess/game/Game.java index f2c8b66..9e99779 100644 --- a/src/dev/kske/chess/game/Game.java +++ b/src/dev/kske/chess/game/Game.java @@ -77,6 +77,9 @@ public class Game { boardComponent.repaint(); overlayComponent.displayArrow(move); + // Run garbage collection + System.gc(); + System.out.printf("%s: %s%n", player.color, move); System.out.println("FEN: " + board.toFEN()); EventBus.getInstance().dispatch(new MoveEvent(move)); diff --git a/src/dev/kske/chess/game/NaturalPlayer.java b/src/dev/kske/chess/game/NaturalPlayer.java index 9d12e61..bf4bae8 100644 --- a/src/dev/kske/chess/game/NaturalPlayer.java +++ b/src/dev/kske/chess/game/NaturalPlayer.java @@ -56,7 +56,7 @@ public class NaturalPlayer extends Player implements MouseListener { pos = new Position(evt.getPoint().x / overlayComponent.getTileSize(), evt.getPoint().y / overlayComponent.getTileSize()); - Board board = (Board) NaturalPlayer.this.board.clone(); + Board board = new Board(this.board); if (board.get(pos) != null && board.get(pos).getColor() == color) { List positions = board.getMoves(pos) .stream() diff --git a/src/dev/kske/chess/game/ai/AIPlayer.java b/src/dev/kske/chess/game/ai/AIPlayer.java index 0a386a3..dcae0c5 100644 --- a/src/dev/kske/chess/game/ai/AIPlayer.java +++ b/src/dev/kske/chess/game/ai/AIPlayer.java @@ -50,7 +50,7 @@ public class AIPlayer extends Player { /* * Get a copy of the board and the available moves. */ - Board board = (Board) AIPlayer.this.board.clone(); + Board board = new Board(this.board); List moves = board.getMoves(color); /* @@ -64,7 +64,7 @@ public class AIPlayer extends Player { for (int i = 0; i < numThreads; i++) { if (rem-- > 0) ++endIndex; endIndex += step; - processors.add(new MoveProcessor((Board) board.clone(), moves.subList(beginIndex, endIndex), color, + processors.add(new MoveProcessor(new Board(board), moves.subList(beginIndex, endIndex), color, maxDepth, alphaBetaThreshold)); beginIndex = endIndex; } diff --git a/test/dev/kske/chess/board/BoardTest.java b/test/dev/kske/chess/board/BoardTest.java index 35bdc96..b647632 100644 --- a/test/dev/kske/chess/board/BoardTest.java +++ b/test/dev/kske/chess/board/BoardTest.java @@ -6,10 +6,7 @@ import static org.junit.Assert.assertNotSame; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import dev.kske.chess.board.Board; -import dev.kske.chess.board.Move; import dev.kske.chess.board.Piece.Color; -import dev.kske.chess.board.Queen; /** * Project: Chess
@@ -34,7 +31,7 @@ class BoardTest { */ @Test void testClone() { - Board clone = (Board) board.clone(); + Board clone = new Board(board); assertNotSame(clone, board); assertNotSame(clone.getBoardArr(), board.getBoardArr()); diff --git a/test/dev/kske/chess/board/LogTest.java b/test/dev/kske/chess/board/LogTest.java index 2185ec8..bd688c2 100644 --- a/test/dev/kske/chess/board/LogTest.java +++ b/test/dev/kske/chess/board/LogTest.java @@ -39,10 +39,16 @@ class LogTest { */ @Test void testClone() { - Log other = new Log(log); + Log other = new Log(log, false); log.setActiveColor(Color.WHITE); other.setActiveColor(Color.BLACK); - assertNotEquals(other.getActiveColor(), log.getActiveColor()); + assertNotEquals(log.getActiveColor(), other.getActiveColor()); + log.add(Move.fromSAN("a2a4"), null, true); + log.add(Move.fromSAN("a4a5"), null, true); + other.add(Move.fromSAN("a2a4"), null, true); + other.add(Move.fromSAN("a4a5"), null, true); + assertNotEquals(log.getRoot(), other.getRoot()); + assertNotEquals(log.getRoot().getVariations(), other.getRoot().getVariations()); } /**