From fd3dce4c6212e67545cbce07496e4252b0c4e976 Mon Sep 17 00:00:00 2001 From: Andy Janata Date: Sun, 16 Feb 2014 00:18:54 -0800 Subject: [PATCH] Reduce Hibernate Session lifetimes for games, which may be related to #89. Now, we're loading the cards out of the database when the game host tells us which card sets to use, and never touching the database again if the card sets never change. Also, don't load every card in every card set when loading the card sets when a player connects to the server. We only need a count of the cards in the card set at that point. --- src/net/socialgamer/cah/data/Game.java | 25 +++++------ src/net/socialgamer/cah/db/CardSet.java | 37 ++++++++++++++-- .../cah/handlers/ChangeGameOptionHandler.java | 43 ++++++++++++++++--- .../cah/handlers/FirstLoadHandler.java | 33 +++++++------- .../socialgamer/cah/data/GameManagerTest.java | 10 ++--- test/net/socialgamer/cah/data/GameTest.java | 2 +- 6 files changed, 107 insertions(+), 43 deletions(-) diff --git a/src/net/socialgamer/cah/data/Game.java b/src/net/socialgamer/cah/data/Game.java index 2ac68c5..301e4ae 100644 --- a/src/net/socialgamer/cah/data/Game.java +++ b/src/net/socialgamer/cah/data/Game.java @@ -56,7 +56,6 @@ import net.socialgamer.cah.db.CardSet; import net.socialgamer.cah.db.WhiteCard; import org.apache.log4j.Logger; -import org.hibernate.Session; import com.google.inject.Inject; @@ -154,7 +153,6 @@ public class Game { private final Set cardSets = new HashSet(); private String password = ""; private boolean useIdleTimer = true; - private final Session hibernateSession; /** * Create a new game. @@ -171,19 +169,14 @@ public class Game { */ @Inject public Game(@GameId final Integer id, final ConnectedUsers connectedUsers, - final GameManager gameManager, final Session hibernateSession, final Timer globalTimer) { + final GameManager gameManager, final Timer globalTimer) { this.id = id; this.connectedUsers = connectedUsers; this.gameManager = gameManager; - this.hibernateSession = hibernateSession; this.globalTimer = globalTimer; state = GameState.LOBBY; } - public Session getHibernateSession() { - return hibernateSession; - } - /** * Add a player to the game. * @@ -297,9 +290,6 @@ public class Game { } // this seems terrible if (players.size() == 0) { - if (null != hibernateSession) { - hibernateSession.close(); - } gameManager.destroyGame(id); } if (players.size() < 3 && state != GameState.LOBBY) { @@ -446,8 +436,19 @@ public class Game { return password; } + /** + * + * @param newScoreGoal + * @param newMaxPlayers + * @param newMaxSpectators + * @param newCardSets Card sets to use in this game. These should be fully loaded from the + * database, as there might not be a {@link org.hibernate.Session} from which to load the cards. + * @param newMaxBlanks + * @param newPassword + * @param newUseTimer + */ public void updateGameSettings(final int newScoreGoal, final int newMaxPlayers, - final int newMaxSpectators, final Set newCardSets, final int newMaxBlanks, + final int newMaxSpectators, final List newCardSets, final int newMaxBlanks, final String newPassword, final boolean newUseTimer) { this.scoreGoal = newScoreGoal; this.playerLimit = newMaxPlayers; diff --git a/src/net/socialgamer/cah/db/CardSet.java b/src/net/socialgamer/cah/db/CardSet.java index 7e77b99..dd4c887 100644 --- a/src/net/socialgamer/cah/db/CardSet.java +++ b/src/net/socialgamer/cah/db/CardSet.java @@ -16,6 +16,10 @@ import javax.persistence.Table; import net.socialgamer.cah.Constants.CardSetData; +import org.hibernate.Session; +import org.hibernate.annotations.LazyCollection; +import org.hibernate.annotations.LazyCollectionOption; + @Entity @Table(name = "card_set") @@ -36,6 +40,7 @@ public class CardSet { name = "card_set_black_card", joinColumns = { @JoinColumn(name = "card_set_id") }, inverseJoinColumns = { @JoinColumn(name = "black_card_id") }) + @LazyCollection(LazyCollectionOption.TRUE) private final Set blackCards; @ManyToMany @@ -43,6 +48,7 @@ public class CardSet { name = "card_set_white_card", joinColumns = { @JoinColumn(name = "card_set_id") }, inverseJoinColumns = { @JoinColumn(name = "white_card_id") }) + @LazyCollection(LazyCollectionOption.TRUE) private final Set whiteCards; public CardSet() { @@ -103,17 +109,42 @@ public class CardSet { } /** + * Get the JSON representation of this card set's metadata. This method will cause lazy-loading of + * the card collections. * @return Client representation of this card set. */ - public Map getClientData() { + public Map getClientMetadata() { + final Map cardSetData = getCommonClientMetadata(); + cardSetData.put(CardSetData.BLACK_CARDS_IN_DECK, getBlackCards().size()); + cardSetData.put(CardSetData.WHITE_CARDS_IN_DECK, getWhiteCards().size()); + return cardSetData; + } + + /** + * Get the JSON representation of this card set's metadata. This method will not cause + * lazy-loading of the card collections. + * @return Client representation of this card set. + */ + public Map getClientMetadata(final Session hibernateSession) { + final Map cardSetData = getCommonClientMetadata(); + final Number blackCount = (Number) hibernateSession + .createQuery("select count(*) from CardSet cs join cs.blackCards where cs.id = :id") + .setParameter("id", id).uniqueResult(); + cardSetData.put(CardSetData.BLACK_CARDS_IN_DECK, blackCount); + final Number whiteCount = (Number) hibernateSession + .createQuery("select count(*) from CardSet cs join cs.whiteCards where cs.id = :id") + .setParameter("id", id).uniqueResult(); + cardSetData.put(CardSetData.WHITE_CARDS_IN_DECK, whiteCount); + return cardSetData; + } + + private Map getCommonClientMetadata() { final Map cardSetData = new HashMap(); cardSetData.put(CardSetData.ID, getId()); cardSetData.put(CardSetData.CARD_SET_NAME, getName()); cardSetData.put(CardSetData.CARD_SET_DESCRIPTION, getDescription()); cardSetData.put(CardSetData.WEIGHT, getWeight()); cardSetData.put(CardSetData.BASE_DECK, isBaseDeck()); - cardSetData.put(CardSetData.BLACK_CARDS_IN_DECK, getBlackCards().size()); - cardSetData.put(CardSetData.WHITE_CARDS_IN_DECK, getWhiteCards().size()); return cardSetData; } diff --git a/src/net/socialgamer/cah/handlers/ChangeGameOptionHandler.java b/src/net/socialgamer/cah/handlers/ChangeGameOptionHandler.java index 993f611..5ee8feb 100644 --- a/src/net/socialgamer/cah/handlers/ChangeGameOptionHandler.java +++ b/src/net/socialgamer/cah/handlers/ChangeGameOptionHandler.java @@ -1,7 +1,10 @@ package net.socialgamer.cah.handlers; +import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; @@ -18,6 +21,11 @@ import net.socialgamer.cah.data.GameManager; import net.socialgamer.cah.data.User; import net.socialgamer.cah.db.CardSet; +import org.hibernate.FetchMode; +import org.hibernate.Session; +import org.hibernate.criterion.CriteriaSpecification; +import org.hibernate.criterion.Restrictions; + import com.google.inject.Inject; @@ -25,9 +33,12 @@ public class ChangeGameOptionHandler extends GameWithPlayerHandler { public static final String OP = AjaxOperation.CHANGE_GAME_OPTIONS.toString(); + private final Session hibernateSession; + @Inject - public ChangeGameOptionHandler(final GameManager gameManager) { + public ChangeGameOptionHandler(final GameManager gameManager, final Session hibernateSession) { super(gameManager); + this.hibernateSession = hibernateSession; } @Override @@ -43,15 +54,18 @@ public class ChangeGameOptionHandler extends GameWithPlayerHandler { try { final int scoreLimit = Integer.parseInt(request.getParameter(AjaxRequest.SCORE_LIMIT)); final int playerLimit = Integer.parseInt(request.getParameter(AjaxRequest.PLAYER_LIMIT)); - final int spectatorLimit = Integer.parseInt(request.getParameter(AjaxRequest.SPECTATOR_LIMIT)); + final int spectatorLimit = Integer.parseInt(request + .getParameter(AjaxRequest.SPECTATOR_LIMIT)); + final String[] cardSetsParsed = request.getParameter(AjaxRequest.CARD_SETS).split(","); - final Set cardSets = new HashSet(); + final Set cardSetIds = new HashSet(); for (final String cardSetId : cardSetsParsed) { if (!cardSetId.isEmpty()) { - cardSets.add((CardSet) game.getHibernateSession().load(CardSet.class, - Integer.parseInt(cardSetId))); + cardSetIds.add(Integer.parseInt(cardSetId)); } } + final List cardSets = getCardSets(cardSetIds); + final int blanksLimit = Integer.parseInt(request.getParameter(AjaxRequest.BLANKS_LIMIT)); final String oldPassword = game.getPassword(); String password = request.getParameter(AjaxRequest.PASSWORD); @@ -65,7 +79,8 @@ public class ChangeGameOptionHandler extends GameWithPlayerHandler { if (null != useTimerString && !"".equals(useTimerString)) { useTimer = Boolean.valueOf(useTimerString); } - game.updateGameSettings(scoreLimit, playerLimit, spectatorLimit, cardSets, blanksLimit, password, useTimer); + game.updateGameSettings(scoreLimit, playerLimit, spectatorLimit, cardSets, blanksLimit, + password, useTimer); // only broadcast an update if the password state has changed, because it needs to change // the text on the join button and the sort order @@ -74,9 +89,25 @@ public class ChangeGameOptionHandler extends GameWithPlayerHandler { } } catch (final NumberFormatException nfe) { return error(ErrorCode.BAD_REQUEST); + } finally { + hibernateSession.close(); } return data; } } + + @SuppressWarnings("unchecked") + private final List getCardSets(final Collection cardSetIds) { + if (cardSetIds.isEmpty()) { + return Collections.emptyList(); + } else { + return hibernateSession.createCriteria(CardSet.class) + .add(Restrictions.in("id", cardSetIds)) + .setFetchMode("whiteCards", FetchMode.JOIN) + .setFetchMode("blackCards", FetchMode.JOIN) + .setResultTransformer(CriteriaSpecification.DISTINCT_ROOT_ENTITY) + .list(); + } + } } diff --git a/src/net/socialgamer/cah/handlers/FirstLoadHandler.java b/src/net/socialgamer/cah/handlers/FirstLoadHandler.java index 4056c43..2de9c91 100644 --- a/src/net/socialgamer/cah/handlers/FirstLoadHandler.java +++ b/src/net/socialgamer/cah/handlers/FirstLoadHandler.java @@ -78,7 +78,6 @@ public class FirstLoadHandler extends Handler { } else { // They already have a session in progress, we need to figure out what they were doing // and tell the client where to continue from. - // Right now we just tell them what their name is. ret.put(AjaxResponse.IN_PROGRESS, Boolean.TRUE); ret.put(AjaxResponse.NICKNAME, user.getNickname()); @@ -90,22 +89,24 @@ public class FirstLoadHandler extends Handler { } } - // get the list of card sets - final Transaction transaction = hibernateSession.beginTransaction(); - @SuppressWarnings("unchecked") - final List cardSets = hibernateSession - .createQuery(CardSet.getCardsetQuery(properties)) - .setReadOnly(true) - .list(); - final List> cardSetsData = new ArrayList>( - cardSets.size()); - for (final CardSet cardSet : cardSets) { - cardSetsData.add(cardSet.getClientData()); + try { + // get the list of card sets + final Transaction transaction = hibernateSession.beginTransaction(); + @SuppressWarnings("unchecked") + final List cardSets = hibernateSession + .createQuery(CardSet.getCardsetQuery(properties)) + .setReadOnly(true) + .list(); + final List> cardSetsData = + new ArrayList>(cardSets.size()); + for (final CardSet cardSet : cardSets) { + cardSetsData.add(cardSet.getClientMetadata(hibernateSession)); + } + ret.put(AjaxResponse.CARD_SETS, cardSetsData); + transaction.commit(); + } finally { + hibernateSession.close(); } - ret.put(AjaxResponse.CARD_SETS, cardSetsData); - transaction.commit(); - hibernateSession.close(); - return ret; } diff --git a/test/net/socialgamer/cah/data/GameManagerTest.java b/test/net/socialgamer/cah/data/GameManagerTest.java index 1db752e..57e4102 100644 --- a/test/net/socialgamer/cah/data/GameManagerTest.java +++ b/test/net/socialgamer/cah/data/GameManagerTest.java @@ -118,11 +118,11 @@ public class GameManagerTest { // fill it up with 3 games assertEquals(0, gameManager.get().intValue()); - gameManager.getGames().put(0, new Game(0, cuMock, gameManager, null, timer)); + gameManager.getGames().put(0, new Game(0, cuMock, gameManager, timer)); assertEquals(1, gameManager.get().intValue()); - gameManager.getGames().put(1, new Game(1, cuMock, gameManager, null, timer)); + gameManager.getGames().put(1, new Game(1, cuMock, gameManager, timer)); assertEquals(2, gameManager.get().intValue()); - gameManager.getGames().put(2, new Game(2, cuMock, gameManager, null, timer)); + gameManager.getGames().put(2, new Game(2, cuMock, gameManager, timer)); // make sure it says it can't make any more assertEquals(-1, gameManager.get().intValue()); @@ -130,13 +130,13 @@ public class GameManagerTest { gameManager.destroyGame(1); // make sure it re-uses that id assertEquals(1, gameManager.get().intValue()); - gameManager.getGames().put(1, new Game(1, cuMock, gameManager, null, timer)); + gameManager.getGames().put(1, new Game(1, cuMock, gameManager, timer)); assertEquals(-1, gameManager.get().intValue()); // remove game 1 out from under it, to make sure it'll fix itself gameManager.getGames().remove(1); assertEquals(1, gameManager.get().intValue()); - gameManager.getGames().put(1, new Game(1, cuMock, gameManager, null, timer)); + gameManager.getGames().put(1, new Game(1, cuMock, gameManager, timer)); assertEquals(-1, gameManager.get().intValue()); gameManager.destroyGame(2); diff --git a/test/net/socialgamer/cah/data/GameTest.java b/test/net/socialgamer/cah/data/GameTest.java index a767565..972a7fe 100644 --- a/test/net/socialgamer/cah/data/GameTest.java +++ b/test/net/socialgamer/cah/data/GameTest.java @@ -61,7 +61,7 @@ public class GameTest { public void setUp() throws Exception { cuMock = createMock(ConnectedUsers.class); gmMock = createMock(GameManager.class); - game = new Game(0, cuMock, gmMock, null, timer); + game = new Game(0, cuMock, gmMock, timer); } @SuppressWarnings("unchecked")