Cache results in account witness domain

core/src/main/java/bisq/core/account/sign/SignedWitnessService.java

@@ -51,6 +51,7 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -59,9 +60,9 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.Stack;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Collectors;
 
-import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 
 @Slf4j
@@ -75,10 +76,15 @@ public class SignedWitnessService {
     private final ArbitratorManager arbitratorManager;
     private final User user;
 
-    @Getter
     private final Map<P2PDataStorage.ByteArray, SignedWitness> signedWitnessMap = new HashMap<>();
+
+    // The getSignedWitnessSet is called very often and is a bit expensive. We cache the result in that map but we
+    // remove the cache entry if we get a matching SignedWitness added to the signedWitnessMap.
+    private final Map<P2PDataStorage.ByteArray, Set<SignedWitness>> getSignedWitnessSetCache = new ConcurrentHashMap<>();
+
     private final FilterManager filterManager;
 
+
     ///////////////////////////////////////////////////////////////////////////////////////////
     // Constructor
     ///////////////////////////////////////////////////////////////////////////////////////////
@@ -142,6 +148,10 @@ private void onBootstrapComplete() {
     // API
     ///////////////////////////////////////////////////////////////////////////////////////////
 
+    public Collection<SignedWitness> getSignedWitnessMapValues() {
+        return signedWitnessMap.values();
+    }
+
     /**
      * List of dates as long when accountAgeWitness was signed
      *
@@ -199,7 +209,7 @@ public String ownerPubKeyAsString(AccountAgeWitness accountAgeWitness) {
 
     @VisibleForTesting
     public Set<SignedWitness> getSignedWitnessSetByOwnerPubKey(byte[] ownerPubKey) {
-        return signedWitnessMap.values().stream()
+        return getSignedWitnessMapValues().stream()
                 .filter(e -> Arrays.equals(e.getWitnessOwnerPubKey(), ownerPubKey))
                 .collect(Collectors.toSet());
     }
@@ -345,29 +355,41 @@ private boolean verifySignatureWithDSAKey(SignedWitness signedWitness) {
     }
 
     private Set<SignedWitness> getSignedWitnessSet(AccountAgeWitness accountAgeWitness) {
-        return signedWitnessMap.values().stream()
-                .filter(e -> Arrays.equals(e.getAccountAgeWitnessHash(), accountAgeWitness.getHash()))
+        byte[] hash = accountAgeWitness.getHash();
+        P2PDataStorage.ByteArray key = new P2PDataStorage.ByteArray(hash);
+        // In case we get a new entry added to signedWitnessMap with our hash we remove the entry from the cache so
+        // that we use the updated signedWitnessMap to fill the cache new.
+        if (getSignedWitnessSetCache.containsKey(key)) {
+            return getSignedWitnessSetCache.get(key);
+        }
+
+        Set<SignedWitness> result = getSignedWitnessMapValues().stream()
+                .filter(e -> Arrays.equals(e.getAccountAgeWitnessHash(), hash))
                 .collect(Collectors.toSet());
+
+        getSignedWitnessSetCache.put(key, result);
+
+        return result;
     }
 
     // SignedWitness objects signed by arbitrators
     public Set<SignedWitness> getArbitratorsSignedWitnessSet(AccountAgeWitness accountAgeWitness) {
-        return signedWitnessMap.values().stream()
+        return getSignedWitnessMapValues().stream()
                 .filter(SignedWitness::isSignedByArbitrator)
                 .filter(e -> Arrays.equals(e.getAccountAgeWitnessHash(), accountAgeWitness.getHash()))
                 .collect(Collectors.toSet());
     }
 
     // SignedWitness objects signed by any other peer
     public Set<SignedWitness> getTrustedPeerSignedWitnessSet(AccountAgeWitness accountAgeWitness) {
-        return signedWitnessMap.values().stream()
+        return getSignedWitnessMapValues().stream()
                 .filter(e -> !e.isSignedByArbitrator())
                 .filter(e -> Arrays.equals(e.getAccountAgeWitnessHash(), accountAgeWitness.getHash()))
                 .collect(Collectors.toSet());
     }
 
     public Set<SignedWitness> getRootSignedWitnessSet(boolean includeSignedByArbitrator) {
-        return signedWitnessMap.values().stream()
+        return getSignedWitnessMapValues().stream()
                 .filter(witness -> getSignedWitnessSetByOwnerPubKey(witness.getSignerPubKey(), new Stack<>()).isEmpty())
                 .filter(witness -> includeSignedByArbitrator ||
                         witness.getVerificationMethod() != SignedWitness.VerificationMethod.ARBITRATOR)
@@ -388,7 +410,7 @@ public Set<SignedWitness> getUnsignedSignerPubKeys() {
     // witnessOwnerPubKey
     private Set<SignedWitness> getSignedWitnessSetByOwnerPubKey(byte[] ownerPubKey,
                                                                 Stack<P2PDataStorage.ByteArray> excluded) {
-        return signedWitnessMap.values().stream()
+        return getSignedWitnessMapValues().stream()
                 .filter(e -> Arrays.equals(e.getWitnessOwnerPubKey(), ownerPubKey))
                 .filter(e -> !excluded.contains(new P2PDataStorage.ByteArray(e.getSignerPubKey())))
                 .collect(Collectors.toSet());
@@ -487,8 +509,18 @@ private boolean verifyDate(SignedWitness signedWitness, long childSignedWitnessD
     ///////////////////////////////////////////////////////////////////////////////////////////
 
     @VisibleForTesting
-    void addToMap(SignedWitness signedWitness) {
-        signedWitnessMap.putIfAbsent(signedWitness.getHashAsByteArray(), signedWitness);
+    public void addToMap(SignedWitness signedWitness) {
+        P2PDataStorage.ByteArray hash = signedWitness.getHashAsByteArray();
+        signedWitnessMap.putIfAbsent(hash, signedWitness);
+
+        // We remove the entry with that hash in case we have cached it, so at the next getSignedWitnessSet
+        // call we use the updated signedWitnessMap to re-fill our cache.
+        getSignedWitnessSetCache.remove(new P2PDataStorage.ByteArray(signedWitness.getAccountAgeWitnessHash()));
+
+        // Not sure if that is needed as well, tests did succeed in both cases, but seems to be more safe to remove
+        // potential entries with hash as well. A removed item in getSignedWitnessSetCache carries no risk, though a
+        // remaining item would.
+        getSignedWitnessSetCache.remove(hash);
     }
 
     private void publishSignedWitness(SignedWitness signedWitness) {
@@ -501,7 +533,22 @@ private void publishSignedWitness(SignedWitness signedWitness) {
     }
 
     private void doRepublishAllSignedWitnesses() {
-        signedWitnessMap.forEach((e, signedWitness) -> p2PService.addPersistableNetworkPayload(signedWitness, true));
+        getSignedWitnessMapValues()
+                .forEach(signedWitness -> p2PService.addPersistableNetworkPayload(signedWitness, true));
+    }
+
+    @VisibleForTesting
+    public void removeSignedWitness(SignedWitness signedWitness) {
+        P2PDataStorage.ByteArray hash = signedWitness.getHashAsByteArray();
+        signedWitnessMap.remove(hash);
+
+        // Need to remove the entry matching signedWitness.getAccountAgeWitnessHash() (test would fail otherwise)
+        getSignedWitnessSetCache.remove(new P2PDataStorage.ByteArray(signedWitness.getAccountAgeWitnessHash()));
+
+        // Not sure if that is needed as well, tests did succeed in both cases, but seems to be more safe to remove
+        // potential entries with hash as well. A removed item in getSignedWitnessSetCache carries no risk, though a
+        // remaining item would.
+        getSignedWitnessSetCache.remove(hash);
     }
 
     // Remove SignedWitnesses that are signed by TRADE that also have an ARBITRATOR signature

core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java

@@ -73,6 +73,7 @@
 import java.util.Optional;
 import java.util.Random;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -138,9 +139,13 @@ public String getDisplayString() {
     @Getter
     private final AccountAgeWitnessUtils accountAgeWitnessUtils;
 
-    @Getter
     private final Map<P2PDataStorage.ByteArray, AccountAgeWitness> accountAgeWitnessMap = new HashMap<>();
 
+    // The accountAgeWitnessMap is very large (70k items) and access is a bit expensive. We usually only access less
+    // than 100 items, those who have offers online. So we use a cache for a fast lookup and only if
+    // not found there we use the accountAgeWitnessMap and put then the new item into our cache.
+    private final Map<P2PDataStorage.ByteArray, AccountAgeWitness> accountAgeWitnessCache = new ConcurrentHashMap<>();
+
 
     ///////////////////////////////////////////////////////////////////////////////////////////
     // Constructor
@@ -233,8 +238,17 @@ public void addToMap(AccountAgeWitness accountAgeWitness) {
 
     public void publishMyAccountAgeWitness(PaymentAccountPayload paymentAccountPayload) {
         AccountAgeWitness accountAgeWitness = getMyWitness(paymentAccountPayload);
-        if (!accountAgeWitnessMap.containsKey(accountAgeWitness.getHashAsByteArray()))
+        P2PDataStorage.ByteArray hash = accountAgeWitness.getHashAsByteArray();
+
+        // We use first our fast lookup cache. If its in accountAgeWitnessCache it is also in accountAgeWitnessMap
+        // and we do not publish.
+        if (accountAgeWitnessCache.containsKey(hash)) {
+            return;
+        }
+
+        if (!accountAgeWitnessMap.containsKey(hash)) {
             p2PService.addPersistableNetworkPayload(accountAgeWitness, false);
+        }
     }
 
     public byte[] getPeerAccountAgeWitnessHash(Trade trade) {
@@ -284,12 +298,21 @@ private Optional<AccountAgeWitness> findTradePeerWitness(Trade trade) {
     private Optional<AccountAgeWitness> getWitnessByHash(byte[] hash) {
         P2PDataStorage.ByteArray hashAsByteArray = new P2PDataStorage.ByteArray(hash);
 
-        final boolean containsKey = accountAgeWitnessMap.containsKey(hashAsByteArray);
-        if (!containsKey)
-            log.debug("hash not found in accountAgeWitnessMap");
+        // First we look up in our fast lookup cache
+        if (accountAgeWitnessCache.containsKey(hashAsByteArray)) {
+            return Optional.of(accountAgeWitnessCache.get(hashAsByteArray));
+        }
+
+        if (accountAgeWitnessMap.containsKey(hashAsByteArray)) {
+            AccountAgeWitness accountAgeWitness = accountAgeWitnessMap.get(hashAsByteArray);
+
+            // We add it to our fast lookup cache
+            accountAgeWitnessCache.put(hashAsByteArray, accountAgeWitness);
 
-        return accountAgeWitnessMap.containsKey(hashAsByteArray) ?
-                Optional.of(accountAgeWitnessMap.get(hashAsByteArray)) : Optional.empty();
+            return Optional.of(accountAgeWitness);
+        }
+
+        return Optional.empty();
     }
 
     private Optional<AccountAgeWitness> getWitnessByHashAsHex(String hashAsHex) {
@@ -659,7 +682,7 @@ public String arbitratorSignOrphanWitness(AccountAgeWitness accountAgeWitness,
                                               ECKey key,
                                               long time) {
         // Find AccountAgeWitness as signedwitness
-        var signedWitness = signedWitnessService.getSignedWitnessMap().values().stream()
+        var signedWitness = signedWitnessService.getSignedWitnessMapValues().stream()
                 .filter(sw -> Arrays.equals(sw.getAccountAgeWitnessHash(), accountAgeWitness.getHash()))
                 .findAny()
                 .orElse(null);

core/src/main/java/bisq/core/account/witness/AccountAgeWitnessUtils.java

@@ -30,6 +30,7 @@
 import bisq.common.util.Utilities;
 
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Optional;
 import java.util.Stack;
 
@@ -67,12 +68,11 @@ public void logSignedWitnesses() {
     }
 
     private void logChild(SignedWitness sigWit, String initString, Stack<P2PDataStorage.ByteArray> excluded) {
-        var allSig = signedWitnessService.getSignedWitnessMap();
         log.info("{}AEW: {} PKH: {} time: {}", initString,
                 Utilities.bytesAsHexString(sigWit.getAccountAgeWitnessHash()).substring(0, 7),
                 Utilities.bytesAsHexString(Hash.getRipemd160hash(sigWit.getWitnessOwnerPubKey())).substring(0, 7),
                 sigWit.getDate());
-        allSig.values().forEach(w -> {
+        signedWitnessService.getSignedWitnessMapValues().forEach(w -> {
             if (!excluded.contains(new P2PDataStorage.ByteArray(w.getWitnessOwnerPubKey())) &&
                     Arrays.equals(w.getSignerPubKey(), sigWit.getWitnessOwnerPubKey())) {
                 excluded.push(new P2PDataStorage.ByteArray(w.getWitnessOwnerPubKey()));
@@ -85,10 +85,10 @@ private void logChild(SignedWitness sigWit, String initString, Stack<P2PDataStor
     // Log signers per
     public void logSigners() {
         log.info("Signers per AEW");
-        var allSig = signedWitnessService.getSignedWitnessMap();
-        allSig.values().forEach(w -> {
+        Collection<SignedWitness> signedWitnessMapValues = signedWitnessService.getSignedWitnessMapValues();
+        signedWitnessMapValues.forEach(w -> {
                     log.info("AEW {}", Utilities.bytesAsHexString(w.getAccountAgeWitnessHash()));
-                    allSig.values().forEach(ww -> {
+                    signedWitnessMapValues.forEach(ww -> {
                         if (Arrays.equals(w.getSignerPubKey(), ww.getWitnessOwnerPubKey())) {
                             log.info("  {}", Utilities.bytesAsHexString(ww.getAccountAgeWitnessHash()));
                         }

core/src/test/java/bisq/core/account/witness/AccountAgeWitnessServiceTest.java

@@ -310,12 +310,12 @@ public void testArbitratorSignDummyWitness() throws CryptoException {
 
         // Remove SignedWitness signed by arbitrator
         @SuppressWarnings("OptionalGetWithoutIsPresent")
-        var signedWitnessArb = signedWitnessService.getSignedWitnessMap().values().stream()
+        var signedWitnessArb = signedWitnessService.getSignedWitnessMapValues().stream()
                 .filter(sw -> sw.getVerificationMethod() == SignedWitness.VerificationMethod.ARBITRATOR)
                 .findAny()
                 .get();
-        signedWitnessService.getSignedWitnessMap().remove(signedWitnessArb.getHashAsByteArray());
-        assertEquals(signedWitnessService.getSignedWitnessMap().size(), 2);
+        signedWitnessService.removeSignedWitness(signedWitnessArb);
+        assertEquals(signedWitnessService.getSignedWitnessMapValues().size(), 2);
 
         // Check that no account age witness is a signer
         assertFalse(service.accountIsSigner(aew1));
@@ -354,7 +354,7 @@ private void signAccountAgeWitness(AccountAgeWitness accountAgeWitness,
                 witnessOwnerPubKey.getEncoded(),
                 time,
                 SignedWitnessService.MINIMUM_TRADE_AMOUNT_FOR_SIGNING.value);
-        signedWitnessService.getSignedWitnessMap().putIfAbsent(signedWitness.getHashAsByteArray(), signedWitness);
+        signedWitnessService.addToMap(signedWitness);
     }
 
 }