From 841c98dc125ebac4ecf18bf95fd11b283b097783 Mon Sep 17 00:00:00 2001 From: leriomaggio Date: Wed, 25 Aug 2021 18:15:28 +0100 Subject: [PATCH] First round of optimisation to CardDb This optimisation removes redundant queries when looking for cards by specificed Art Preference. getCardsFromSet has been worked-around, avoiding querying for potential candidates already in memory. A preliminary benchmark/tests is implemented too. --- .../src/main/java/forge/card/CardDb.java | 43 ++++++---- .../forge/card/CardDbPerformanceTests.java | 86 +++++++++++++++++++ 2 files changed, 112 insertions(+), 17 deletions(-) create mode 100644 forge-gui-desktop/src/test/java/forge/card/CardDbPerformanceTests.java diff --git a/forge-core/src/main/java/forge/card/CardDb.java b/forge-core/src/main/java/forge/card/CardDb.java index a1dd0cc8b98..c57fb351c73 100644 --- a/forge-core/src/main/java/forge/card/CardDb.java +++ b/forge-core/src/main/java/forge/card/CardDb.java @@ -465,7 +465,7 @@ public final class CardDb implements ICardDatabase, IDeckGenPool { // So now check whether the cards exists in the DB first, // and select pick the card based on current SetPreference policy as a fallback Collection cards = getAllCards(request.cardName); - if (cards == null) + if (cards.isEmpty()) // Never null being this a view in MultiMap return null; // Either No Edition has been specified OR as a fallback in case of any error! // get card using the default card art preference @@ -640,6 +640,7 @@ public final class CardDb implements ICardDatabase, IDeckGenPool { @Override public boolean apply(PaperCard c) { CardEdition ed = editions.get(c.getEdition()); + if (ed == null) return false; if (releasedBeforeFlag) return ed.getDate().before(releaseDate); else @@ -648,22 +649,24 @@ public final class CardDb implements ICardDatabase, IDeckGenPool { })); } - if (cards.size() == 0) // Don't bother continuing! No card has been found! - return null; - /* 2. Retrieve cards based of [Frame]Set Preference ================================================ */ // Collect the list of all editions found for target card - LinkedHashSet cardEditions = new LinkedHashSet<>(); + List cardEditions = new ArrayList<>(); + Map candidatesCard = new HashMap<>(); for (PaperCard card : cards) { + if (card.getArtIndex() != cr.artIndex) + continue; String setCode = card.getEdition(); + CardEdition ed = null; if (setCode.equals(CardEdition.UNKNOWN.getCode())) - cardEditions.add(CardEdition.UNKNOWN); - else { - CardEdition ed = editions.get(card.getEdition()); - if (ed != null) - cardEditions.add(ed); + ed = CardEdition.UNKNOWN; + else + ed = editions.get(card.getEdition()); + if (ed != null) { + cardEditions.add(ed); + candidatesCard.put(setCode, card); } } // Filter Cards Editions based on set preferences @@ -681,16 +684,19 @@ public final class CardDb implements ICardDatabase, IDeckGenPool { If this happens, we won't try to iterate over an empty list. Instead, we will fall back to original lists of editions (unfiltered, of course) AND STILL sorted according to chosen art preference. */ - if (acceptedEditions.size() == 0) + if (acceptedEditions.isEmpty()) acceptedEditions.addAll(cardEditions); - Collections.sort(acceptedEditions); // CardEdition correctly sort by (release) date - if (artPref.latestFirst) - Collections.reverse(acceptedEditions); // newest editions first + if (acceptedEditions.size() > 1) { + Collections.sort(acceptedEditions); // CardEdition correctly sort by (release) date + if (artPref.latestFirst) + Collections.reverse(acceptedEditions); // newest editions first + } + PaperCard candidate = null; for (CardEdition ed : acceptedEditions) { - PaperCard cardFromSet = getCardFromSet(cr.cardName, ed, cr.artIndex, cr.isFoil); - if (candidate == null && cardFromSet != null) + PaperCard cardFromSet = candidatesCard.get(ed.getCode()); //getCardFromSet(cr.cardName, ed, cr.artIndex, cr.isFoil); + if (candidate == null) // save the first card found, as the last backup in case no other candidate *with image* will be found candidate = cardFromSet; @@ -699,8 +705,11 @@ public final class CardDb implements ICardDatabase, IDeckGenPool { break; // we're done here: found card **with Image** } } + if (candidate == null) + return null; + //If any, we're sure that at least one candidate is always returned nevertheless it has image or not - return candidate; // any foil request already handled in getCardFromSet + return cr.isFoil ? candidate.getFoiled() : candidate; } @Override diff --git a/forge-gui-desktop/src/test/java/forge/card/CardDbPerformanceTests.java b/forge-gui-desktop/src/test/java/forge/card/CardDbPerformanceTests.java new file mode 100644 index 00000000000..2e8129862c0 --- /dev/null +++ b/forge-gui-desktop/src/test/java/forge/card/CardDbPerformanceTests.java @@ -0,0 +1,86 @@ +package forge.card; + +import forge.ImageKeys; +import forge.item.PaperCard; +import org.mockito.Mockito; +import org.powermock.api.mockito.PowerMockito; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import java.util.Collection; +import java.util.Set; +import java.util.TreeSet; + +import static org.testng.Assert.assertNotNull; + +public class CardDbPerformanceTests extends CardDbTestCase { + + private Set fullDbCardNames = new TreeSet<>(); + + @Override + @BeforeMethod + public void setup() { + super.setup(); + Collection uniqueCards = this.cardDb.getUniqueCards(); + for (PaperCard card : uniqueCards) + this.fullDbCardNames.add(card.getName()); + } + + @Test + public void testBenchmarkFullDbGetCardLegacyImplementation() { + int nRuns = 100; + long averageTime = 0; + long minTime = 10000; // 10 secs + long maxTime = 0; + for (int r = 1; r <= nRuns; r++) { + long start = System.currentTimeMillis(); + for (String name : this.fullDbCardNames) { + PaperCard card = this.legacyCardDb.getCard(name); + assertNotNull(card); + } + long timeRun = System.currentTimeMillis() - start; + averageTime += timeRun; + if (timeRun < minTime) + minTime = timeRun; + if (timeRun > maxTime) + maxTime = timeRun; + } + System.out.println("[LEGACY] Total Time (in sec): " + ((double) averageTime)/ 1000); + System.out.println("[LEGACY] Average Time (in sec): " + ((double) averageTime / nRuns)/ 1000); + System.out.println("[LEGACY] Best Time (in sec): " + ((double) minTime)/ 1000); + System.out.println("[LEGACY] Worst Time (in sec): " + ((double) maxTime)/ 1000); + } + + @Test + public void testBenchmarkFullDbGetCardNewDbImplementation() { + int nRuns = 100; + long averageTime = 0; + long minTime = 10000; // 10 secs + long maxTime = 0; + for (int r = 1; r <= nRuns; r++) { + long start = System.currentTimeMillis(); + for (String name : this.fullDbCardNames) { + PaperCard card = this.cardDb.getCard(name); + assertNotNull(card); + } + long timeRun = System.currentTimeMillis() - start; + averageTime += timeRun; + if (timeRun < minTime) + minTime = timeRun; + if (timeRun > maxTime) + maxTime = timeRun; + } + System.out.println("[NEW] Total Time (in sec): " + ((double) averageTime)/ 1000); + System.out.println("[NEW] Average Time (in sec): " + ((double) averageTime / nRuns)/ 1000); + System.out.println("[NEW] Best Time (in sec): " + ((double) minTime)/ 1000); + System.out.println("[NEW] Worst Time (in sec): " + ((double) maxTime)/ 1000); + } + + @Test + public void testGetCardFullDbNewImplementationToProfile(){ + for (String name : this.fullDbCardNames) { + PaperCard card = this.cardDb.getCard(name); + assertNotNull(card); + } + } +}