Remove HashmapChangedListener::onBatch operations

Now that the only user of this interface has been removed, go ahead and delete it. This is a partial revert of f5d75c4 that includes the code that was added into ProposalService that subscribed to the P2PDataStore.
julianknutsen · Nov 19, 2019 · a8139f3 · a8139f3
1 parent a50e59f
commit a8139f3
Show file tree

Hide file tree

Showing 3 changed files with 3 additions and 65 deletions.
diff --git a/core/src/main/java/bisq/core/dao/governance/proposal/ProposalListPresentation.java b/core/src/main/java/bisq/core/dao/governance/proposal/ProposalListPresentation.java
@@ -20,16 +20,11 @@
 import bisq.core.btc.wallet.BsqWalletService;
 import bisq.core.dao.DaoSetupService;
 import bisq.core.dao.governance.proposal.storage.appendonly.ProposalPayload;
-import bisq.core.dao.governance.proposal.storage.temp.TempProposalPayload;
 import bisq.core.dao.state.DaoStateListener;
 import bisq.core.dao.state.DaoStateService;
 import bisq.core.dao.state.model.blockchain.Block;
 import bisq.core.dao.state.model.governance.Proposal;
 
-import bisq.network.p2p.storage.HashMapChangedListener;
-import bisq.network.p2p.storage.P2PDataStorage;
-import bisq.network.p2p.storage.payload.ProtectedStorageEntry;
-
 import bisq.common.UserThread;
 
 import org.bitcoinj.core.TransactionConfidence;
@@ -41,7 +36,6 @@
 import javafx.collections.ObservableList;
 import javafx.collections.transformation.FilteredList;
 
-import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -56,8 +50,7 @@
  * our own proposal that is not critical). Foreign proposals are only shown if confirmed and fully validated.
  */
 @Slf4j
-public class ProposalListPresentation implements DaoStateListener, HashMapChangedListener,
-        MyProposalListService.Listener, DaoSetupService {
+public class ProposalListPresentation implements DaoStateListener, MyProposalListService.Listener, DaoSetupService {
     private final ProposalService proposalService;
     private final DaoStateService daoStateService;
     private final MyProposalListService myProposalListService;
@@ -67,7 +60,6 @@ public class ProposalListPresentation implements DaoStateListener, HashMapChange
     @Getter
     private final FilteredList<Proposal> activeOrMyUnconfirmedProposals = new FilteredList<>(allProposals);
     private final ListChangeListener<Proposal> proposalListChangeListener;
-    private boolean tempProposalsChanged;
 
 
     ///////////////////////////////////////////////////////////////////////////////////////////
@@ -77,7 +69,6 @@ public class ProposalListPresentation implements DaoStateListener, HashMapChange
     @Inject
     public ProposalListPresentation(ProposalService proposalService,
                                     DaoStateService daoStateService,
-                                    P2PDataStorage p2PDataStorage,
                                     MyProposalListService myProposalListService,
                                     BsqWalletService bsqWalletService,
                                     ProposalValidatorProvider validatorProvider) {
@@ -88,7 +79,6 @@ public ProposalListPresentation(ProposalService proposalService,
         this.validatorProvider = validatorProvider;
 
         daoStateService.addDaoStateListener(this);
-        p2PDataStorage.addHashMapChangedListener(this);
         myProposalListService.addListener(this);
 
         proposalListChangeListener = c -> updateLists();
@@ -125,48 +115,6 @@ public void onParseBlockCompleteAfterBatchProcessing(Block block) {
         updateLists();
     }
 
-
-    ///////////////////////////////////////////////////////////////////////////////////////////
-    // HashMapChangedListener
-    ///////////////////////////////////////////////////////////////////////////////////////////
-
-    @Override
-    public void onAdded(Collection<ProtectedStorageEntry> protectedStorageEntries) {
-        protectedStorageEntries.forEach(protectedStorageEntry -> {
-            if (protectedStorageEntry.getProtectedStoragePayload() instanceof TempProposalPayload) {
-                tempProposalsChanged = true;
-            }
-        });
-    }
-
-    @Override
-    public void onRemoved(Collection<ProtectedStorageEntry> protectedStorageEntries) {
-        protectedStorageEntries.forEach(protectedStorageEntry -> {
-            if (protectedStorageEntry.getProtectedStoragePayload() instanceof TempProposalPayload) {
-                tempProposalsChanged = true;
-            }
-        });
-    }
-
-    @Override
-    public void onBatchRemoveExpiredDataStarted() {
-        // We temporary remove the listener when batch processing starts to avoid that we rebuild our lists at each
-        // remove call. After batch processing at onBatchRemoveExpiredDataCompleted we add again our listener and call
-        // the updateLists method.
-        proposalService.getTempProposals().removeListener(proposalListChangeListener);
-    }
-
-    @Override
-    public void onBatchRemoveExpiredDataCompleted() {
-        proposalService.getTempProposals().addListener(proposalListChangeListener);
-        // We only call updateLists if tempProposals have changed. updateLists() is an expensive call and takes 200 ms.
-        if (tempProposalsChanged) {
-            updateLists();
-            tempProposalsChanged = false;
-        }
-    }
-
-
     ///////////////////////////////////////////////////////////////////////////////////////////
     // MyProposalListService.Listener
     ///////////////////////////////////////////////////////////////////////////////////////////

diff --git a/p2p/src/main/java/bisq/network/p2p/storage/HashMapChangedListener.java b/p2p/src/main/java/bisq/network/p2p/storage/HashMapChangedListener.java
@@ -26,12 +26,4 @@ public interface HashMapChangedListener {
 
     @SuppressWarnings("UnusedParameters")
     void onRemoved(Collection<ProtectedStorageEntry> protectedStorageEntries);
-
-    // We process all expired entries after a delay (60 s) after onBootstrapComplete.
-    // We notify listeners of start and completion so they can optimize to only update after batch processing is done.
-    default void onBatchRemoveExpiredDataStarted() {
-    }
-
-    default void onBatchRemoveExpiredDataCompleted() {
-    }
 }
diff --git a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java
@@ -200,15 +200,13 @@ void removeExpiredEntries() {
                 .filter(entry -> entry.getValue().isExpired(this.clock))
                 .collect(Collectors.toCollection(ArrayList::new));
 
-        // Batch processing can cause performance issues, so we give listeners a chance to deal with it by notifying
-        // about start and end of iteration.
-        hashMapChangedListeners.forEach(HashMapChangedListener::onBatchRemoveExpiredDataStarted);
+        // Batch processing can cause performance issues, so do all of the removes first, then update the listeners
+        // to let them know about the removes.
         toRemoveList.forEach(toRemoveItem -> {
             log.debug("We found an expired data entry. We remove the protectedData:\n\t" +
                     Utilities.toTruncatedString(toRemoveItem.getValue()));
         });
         removeFromMapAndDataStore(toRemoveList);
-        hashMapChangedListeners.forEach(HashMapChangedListener::onBatchRemoveExpiredDataCompleted);
 
         if (sequenceNumberMap.size() > this.maxSequenceNumberMapSizeBeforePurge)
             sequenceNumberMap.setMap(getPurgedSequenceNumberMap(sequenceNumberMap.getMap()));