From 6ef6bf24d9c72b179bb25ea487833f3e3b145f7c Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Fri, 6 Mar 2020 17:03:55 +0800 Subject: [PATCH 1/2] Avoid raw use of Overlay --- .../java/bisq/desktop/main/MainViewModel.java | 22 +++++++++---------- .../monitor/daostate/DaoStateMonitorView.java | 18 +++++++-------- .../pendingtrades/steps/TradeStepView.java | 3 +-- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/desktop/src/main/java/bisq/desktop/main/MainViewModel.java b/desktop/src/main/java/bisq/desktop/main/MainViewModel.java index 0766f96c43c..f439cfc0cd9 100644 --- a/desktop/src/main/java/bisq/desktop/main/MainViewModel.java +++ b/desktop/src/main/java/bisq/desktop/main/MainViewModel.java @@ -133,7 +133,7 @@ public class MainViewModel implements ViewModel, BisqSetup.BisqSetupListener { private Timer checkNumberOfP2pNetworkPeersTimer; @SuppressWarnings("FieldCanBeLocal") private MonadicBinding tradesAndUIReady; - private Queue popupQueue = new PriorityQueue<>(Comparator.comparing(Overlay::getDisplayOrderPriority)); + private Queue> popupQueue = new PriorityQueue<>(Comparator.comparing(Overlay::getDisplayOrderPriority)); /////////////////////////////////////////////////////////////////////////////////////////// @@ -362,7 +362,7 @@ private void setupHandlers() { .show()); bisqSetup.setDisplayLocalhostHandler(key -> { if (!DevEnv.isDevMode()) { - Overlay popup = new Popup().backgroundInfo(Res.get("popup.bitcoinLocalhostNode.msg") + + Popup popup = new Popup().backgroundInfo(Res.get("popup.bitcoinLocalhostNode.msg") + Res.get("popup.bitcoinLocalhostNode.additionalRequirements")) .dontShowAgainId(key); popup.setDisplayOrderPriority(5); @@ -396,14 +396,14 @@ private void setupHandlers() { tradeManager.getTradesWithoutDepositTx().addListener((ListChangeListener) c -> { c.next(); if (c.wasAdded()) { - c.getAddedSubList().forEach(trade -> { - new Popup().warning(Res.get("popup.warning.trade.depositTxNull", trade.getShortId())) - .actionButtonText(Res.get("popup.warning.trade.depositTxNull.shutDown")) - .onAction(() -> BisqApp.getShutDownHandler().run()) - .secondaryActionButtonText(Res.get("popup.warning.trade.depositTxNull.moveToFailedTrades")) - .onSecondaryAction(() -> tradeManager.addTradeToFailedTrades(trade)) - .show(); - }); + c.getAddedSubList().forEach(trade -> + new Popup().warning(Res.get("popup.warning.trade.depositTxNull", trade.getShortId())) + .actionButtonText(Res.get("popup.warning.trade.depositTxNull.shutDown")) + .onAction(() -> BisqApp.getShutDownHandler().run()) + .secondaryActionButtonText(Res.get("popup.warning.trade.depositTxNull.moveToFailedTrades")) + .onSecondaryAction(() -> tradeManager.addTradeToFailedTrades(trade)) + .show() + ); } }); @@ -682,7 +682,7 @@ private void maybeAddNewTradeProtocolLaunchWindowToQueue() { private void maybeShowPopupsFromQueue() { if (!popupQueue.isEmpty()) { - Overlay overlay = popupQueue.poll(); + Overlay overlay = popupQueue.poll(); overlay.getIsHiddenProperty().addListener((observable, oldValue, newValue) -> { if (newValue) { UserThread.runAfter(this::maybeShowPopupsFromQueue, 2); diff --git a/desktop/src/main/java/bisq/desktop/main/dao/monitor/daostate/DaoStateMonitorView.java b/desktop/src/main/java/bisq/desktop/main/dao/monitor/daostate/DaoStateMonitorView.java index 567c4e3698a..1166f41c13f 100644 --- a/desktop/src/main/java/bisq/desktop/main/dao/monitor/daostate/DaoStateMonitorView.java +++ b/desktop/src/main/java/bisq/desktop/main/dao/monitor/daostate/DaoStateMonitorView.java @@ -19,7 +19,6 @@ import bisq.desktop.common.view.FxmlView; import bisq.desktop.main.dao.monitor.StateMonitorView; -import bisq.desktop.main.overlays.Overlay; import bisq.desktop.main.overlays.popups.Popup; import bisq.desktop.util.FormBuilder; @@ -53,7 +52,7 @@ public class DaoStateMonitorView extends StateMonitorView utxoMismatchListChangeListener; - private Overlay warningPopup; + private Popup warningPopup; /////////////////////////////////////////////////////////////////////////////////////////// @@ -208,17 +207,16 @@ protected void requestHashesFromGenesisBlockHeight(String peerAddress) { private void updateUtxoMismatches() { if (!daoStateMonitoringService.getUtxoMismatches().isEmpty()) { StringBuilder sb = new StringBuilder(); - daoStateMonitoringService.getUtxoMismatches().forEach(e -> { - sb.append("\n").append(Res.get("dao.monitor.daoState.utxoConflicts.blockHeight", e.getHeight())).append("\n") - .append(Res.get("dao.monitor.daoState.utxoConflicts.sumUtxo", e.getSumUtxo() / 100)).append("\n") - .append(Res.get("dao.monitor.daoState.utxoConflicts.sumBsq", e.getSumBsq() / 100)); - }); + daoStateMonitoringService.getUtxoMismatches().forEach(e -> sb.append("\n") + .append(Res.get("dao.monitor.daoState.utxoConflicts.blockHeight", e.getHeight())).append("\n") + .append(Res.get("dao.monitor.daoState.utxoConflicts.sumUtxo", e.getSumUtxo() / 100)).append("\n") + .append(Res.get("dao.monitor.daoState.utxoConflicts.sumBsq", e.getSumBsq() / 100)) + ); if (warningPopup == null) { warningPopup = new Popup().headLine(Res.get("dao.monitor.daoState.utxoConflicts")) - .warning(Utilities.toTruncatedString(sb.toString(), 500, false)).onClose(() -> { - warningPopup = null; - }); + .warning(Utilities.toTruncatedString(sb.toString(), 500, false)) + .onClose(() -> warningPopup = null); warningPopup.show(); } } diff --git a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/TradeStepView.java b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/TradeStepView.java index caf6a7ed3f7..ef82e67448d 100644 --- a/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/TradeStepView.java +++ b/desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/TradeStepView.java @@ -20,7 +20,6 @@ import bisq.desktop.components.InfoTextField; import bisq.desktop.components.TitledGroupBg; import bisq.desktop.components.TxIdTextField; -import bisq.desktop.main.overlays.Overlay; import bisq.desktop.main.overlays.popups.Popup; import bisq.desktop.main.portfolio.pendingtrades.PendingTradesViewModel; import bisq.desktop.main.portfolio.pendingtrades.TradeStepInfo; @@ -93,7 +92,7 @@ public abstract class TradeStepView extends AnchorPane { private ClockWatcher.Listener clockListener; private final ChangeListener errorMessageListener; protected Label infoLabel; - private Overlay acceptMediationResultPopup; + private Popup acceptMediationResultPopup; private BootstrapListener bootstrapListener; From c26193392830977e368e0794e672d8a18002f2bb Mon Sep 17 00:00:00 2001 From: Steven Barclay Date: Fri, 6 Mar 2020 17:57:08 +0800 Subject: [PATCH 2/2] Compute height cells lazily to speed up DaoStateMonitorView Avoid a bottleneck computing the cycle index & calling 'Res.get(..)' for every block since genesis in the DAO state monitor view, when building the DaoStateBlockListItem objects, by making the 'height' field lazy. To do this, pass the cycle index into the constructor using an IntSupplier and make the height a memoised 'Supplier' with a custom getter. Also add a unit test to check that the auto-generated equals & hashCode methods still work as expected, as it isn't totally clear what Lombok would do when a field type differs from its getter return type. --- .../main/dao/monitor/StateBlockListItem.java | 31 ++++++--- .../daostate/DaoStateBlockListItem.java | 7 ++- .../monitor/daostate/DaoStateMonitorView.java | 7 ++- .../daostate/DaoStateBlockListItemTest.java | 63 +++++++++++++++++++ 4 files changed, 95 insertions(+), 13 deletions(-) create mode 100644 desktop/src/test/java/bisq/desktop/main/dao/monitor/daostate/DaoStateBlockListItemTest.java diff --git a/desktop/src/main/java/bisq/desktop/main/dao/monitor/StateBlockListItem.java b/desktop/src/main/java/bisq/desktop/main/dao/monitor/StateBlockListItem.java index 7062fa726e7..eff55bb1868 100644 --- a/desktop/src/main/java/bisq/desktop/main/dao/monitor/StateBlockListItem.java +++ b/desktop/src/main/java/bisq/desktop/main/dao/monitor/StateBlockListItem.java @@ -23,6 +23,11 @@ import bisq.common.util.Utilities; +import com.google.common.base.Suppliers; + +import java.util.function.IntSupplier; +import java.util.function.Supplier; + import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.extern.slf4j.Slf4j; @@ -31,17 +36,27 @@ @Getter @EqualsAndHashCode public abstract class StateBlockListItem> { - protected final StateBlock stateBlock; - protected final String height; - protected final String hash; - protected final String prevHash; - protected final String numNetworkMessages; - protected final String numMisMatches; - protected final boolean isInSync; + private final StateBlock stateBlock; + private final Supplier height; + private final String hash; + private final String prevHash; + private final String numNetworkMessages; + private final String numMisMatches; + private final boolean isInSync; + + public String getHeight() { + return height.get(); + } protected StateBlockListItem(StB stateBlock, int cycleIndex) { + this(stateBlock, () -> cycleIndex); + } + + protected StateBlockListItem(StB stateBlock, IntSupplier cycleIndexSupplier) { this.stateBlock = stateBlock; - height = Res.get("dao.monitor.table.cycleBlockHeight", cycleIndex + 1, String.valueOf(stateBlock.getHeight())); + height = Suppliers.memoize(() -> + Res.get("dao.monitor.table.cycleBlockHeight", cycleIndexSupplier.getAsInt() + 1, + String.valueOf(stateBlock.getHeight())))::get; hash = Utilities.bytesAsHexString(stateBlock.getHash()); prevHash = stateBlock.getPrevHash().length > 0 ? Utilities.bytesAsHexString(stateBlock.getPrevHash()) : "-"; numNetworkMessages = String.valueOf(stateBlock.getPeersMap().size()); diff --git a/desktop/src/main/java/bisq/desktop/main/dao/monitor/daostate/DaoStateBlockListItem.java b/desktop/src/main/java/bisq/desktop/main/dao/monitor/daostate/DaoStateBlockListItem.java index d4ec7fdd369..18636794709 100644 --- a/desktop/src/main/java/bisq/desktop/main/dao/monitor/daostate/DaoStateBlockListItem.java +++ b/desktop/src/main/java/bisq/desktop/main/dao/monitor/daostate/DaoStateBlockListItem.java @@ -22,6 +22,8 @@ import bisq.core.dao.monitoring.model.DaoStateBlock; import bisq.core.dao.monitoring.model.DaoStateHash; +import java.util.function.IntSupplier; + import lombok.EqualsAndHashCode; import lombok.Value; import lombok.extern.slf4j.Slf4j; @@ -30,8 +32,7 @@ @Value @EqualsAndHashCode(callSuper = true) class DaoStateBlockListItem extends StateBlockListItem { - DaoStateBlockListItem(DaoStateBlock stateBlock, int cycleIndex) { - super(stateBlock, cycleIndex); + DaoStateBlockListItem(DaoStateBlock stateBlock, IntSupplier cycleIndexSupplier) { + super(stateBlock, cycleIndexSupplier); } } - diff --git a/desktop/src/main/java/bisq/desktop/main/dao/monitor/daostate/DaoStateMonitorView.java b/desktop/src/main/java/bisq/desktop/main/dao/monitor/daostate/DaoStateMonitorView.java index 1166f41c13f..a1862413e4a 100644 --- a/desktop/src/main/java/bisq/desktop/main/dao/monitor/daostate/DaoStateMonitorView.java +++ b/desktop/src/main/java/bisq/desktop/main/dao/monitor/daostate/DaoStateMonitorView.java @@ -45,6 +45,7 @@ import java.io.File; import java.util.Map; +import java.util.function.IntSupplier; import java.util.stream.Collectors; @FxmlView @@ -126,8 +127,10 @@ public void onCheckpointFail() { @Override protected DaoStateBlockListItem getStateBlockListItem(DaoStateBlock daoStateBlock) { - int cycleIndex = periodService.getCycle(daoStateBlock.getHeight()).map(cycleService::getCycleIndex).orElse(0); - return new DaoStateBlockListItem(daoStateBlock, cycleIndex); + IntSupplier cycleIndexSupplier = () -> periodService.getCycle(daoStateBlock.getHeight()) + .map(cycleService::getCycleIndex) + .orElse(0); + return new DaoStateBlockListItem(daoStateBlock, cycleIndexSupplier); } @Override diff --git a/desktop/src/test/java/bisq/desktop/main/dao/monitor/daostate/DaoStateBlockListItemTest.java b/desktop/src/test/java/bisq/desktop/main/dao/monitor/daostate/DaoStateBlockListItemTest.java new file mode 100644 index 00000000000..182be1ba09e --- /dev/null +++ b/desktop/src/test/java/bisq/desktop/main/dao/monitor/daostate/DaoStateBlockListItemTest.java @@ -0,0 +1,63 @@ +/* + * This file is part of Bisq. + * + * Bisq is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or (at + * your option) any later version. + * + * Bisq is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public + * License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with Bisq. If not, see . + */ + +package bisq.desktop.main.dao.monitor.daostate; + +import bisq.core.dao.monitoring.model.DaoStateBlock; +import bisq.core.dao.monitoring.model.DaoStateHash; +import bisq.core.locale.Res; + +import java.util.Locale; +import java.util.function.IntSupplier; + +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; + +public class DaoStateBlockListItemTest { + + @Before + public void setup() { + Locale.setDefault(new Locale("en", "US")); + Res.setBaseCurrencyCode("BTC"); + Res.setBaseCurrencyName("Bitcoin"); + } + + @Test + public void testEqualsAndHashCode() { + var block = new DaoStateBlock(new DaoStateHash(0, new byte[0], new byte[0])); + var item1 = new DaoStateBlockListItem(block, newSupplier(1)); + var item2 = new DaoStateBlockListItem(block, newSupplier(2)); + var item3 = new DaoStateBlockListItem(block, newSupplier(1)); + assertNotEquals(item1, item2); + assertNotEquals(item2, item3); + assertEquals(item1, item3); + assertEquals(item1.hashCode(), item3.hashCode()); + } + + private IntSupplier newSupplier(int i) { + //noinspection Convert2Lambda + return new IntSupplier() { + @Override + public int getAsInt() { + return i; + } + }; + } +}