Merge pull request #4953 from chimp1984/cache-results-in-account-witness-domain

Cache results in account witness domain

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

@@ -51,6 +51,7 @@ import java.time.temporal.ChronoUnit;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -61,7 +62,6 @@ import java.util.Set;
 import java.util.Stack;
 import java.util.stream.Collectors;
 
-import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 
 @Slf4j
@@ -74,11 +74,15 @@ public class SignedWitnessService {
     private final P2PService p2PService;
     private final ArbitratorManager arbitratorManager;
     private final User user;
-
-    @Getter
-    private final Map<P2PDataStorage.ByteArray, SignedWitness> signedWitnessMap = new HashMap<>();
     private final FilterManager filterManager;
 
+    private final Map<P2PDataStorage.ByteArray, SignedWitness> signedWitnessMap = new HashMap<>();
+
+    // This map keeps all SignedWitnesses with the same AccountAgeWitnessHash in a Set.
+    // This avoids iterations over the signedWitnessMap for getting the set of such SignedWitnesses.
+    private final Map<P2PDataStorage.ByteArray, Set<SignedWitness>> signedWitnessSetByAccountAgeWitnessHash = new HashMap<>();
+
+
     ///////////////////////////////////////////////////////////////////////////////////////////
     // Constructor
     ///////////////////////////////////////////////////////////////////////////////////////////
@@ -142,6 +146,10 @@ public class SignedWitnessService {
     // API
     ///////////////////////////////////////////////////////////////////////////////////////////
 
+    public Collection<SignedWitness> getSignedWitnessMapValues() {
+        return signedWitnessMap.values();
+    }
+
     /**
      * List of dates as long when accountAgeWitness was signed
      *
@@ -199,7 +207,7 @@ public class SignedWitnessService {
 
     @VisibleForTesting
     public Set<SignedWitness> getSignedWitnessSetByOwnerPubKey(byte[] ownerPubKey) {
-        return signedWitnessMap.values().stream()
+        return getSignedWitnessMapValues().stream()
                 .filter(e -> Arrays.equals(e.getWitnessOwnerPubKey(), ownerPubKey))
                 .collect(Collectors.toSet());
     }
@@ -344,30 +352,27 @@ public class SignedWitnessService {
         }
     }
 
-    private Set<SignedWitness> getSignedWitnessSet(AccountAgeWitness accountAgeWitness) {
+    public Set<SignedWitness> getSignedWitnessSet(AccountAgeWitness accountAgeWitness) {
-        return signedWitnessMap.values().stream()
+        P2PDataStorage.ByteArray key = new P2PDataStorage.ByteArray(accountAgeWitness.getHash());
-                .filter(e -> Arrays.equals(e.getAccountAgeWitnessHash(), accountAgeWitness.getHash()))
+        return signedWitnessSetByAccountAgeWitnessHash.getOrDefault(key, new HashSet<>());
-                .collect(Collectors.toSet());
     }
 
     // SignedWitness objects signed by arbitrators
     public Set<SignedWitness> getArbitratorsSignedWitnessSet(AccountAgeWitness accountAgeWitness) {
-        return signedWitnessMap.values().stream()
+        return getSignedWitnessSet(accountAgeWitness).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 getSignedWitnessSet(accountAgeWitness).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 +393,7 @@ public class SignedWitnessService {
     // 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 +492,12 @@ public class SignedWitnessService {
     ///////////////////////////////////////////////////////////////////////////////////////////
 
     @VisibleForTesting
-    void addToMap(SignedWitness signedWitness) {
+    public void addToMap(SignedWitness signedWitness) {
         signedWitnessMap.putIfAbsent(signedWitness.getHashAsByteArray(), signedWitness);
+
+        P2PDataStorage.ByteArray accountAgeWitnessHash = new P2PDataStorage.ByteArray(signedWitness.getAccountAgeWitnessHash());
+        signedWitnessSetByAccountAgeWitnessHash.putIfAbsent(accountAgeWitnessHash, new HashSet<>());
+        signedWitnessSetByAccountAgeWitnessHash.get(accountAgeWitnessHash).add(signedWitness);
     }
 
     private void publishSignedWitness(SignedWitness signedWitness) {
@@ -501,7 +510,22 @@ public class SignedWitnessService {
     }
 
     private void doRepublishAllSignedWitnesses() {
-        signedWitnessMap.forEach((e, signedWitness) -> p2PService.addPersistableNetworkPayload(signedWitness, true));
+        getSignedWitnessMapValues()
+                .forEach(signedWitness -> p2PService.addPersistableNetworkPayload(signedWitness, true));
+    }
+
+    @VisibleForTesting
+    public void removeSignedWitness(SignedWitness signedWitness) {
+        signedWitnessMap.remove(signedWitness.getHashAsByteArray());
+
+        P2PDataStorage.ByteArray accountAgeWitnessHash = new P2PDataStorage.ByteArray(signedWitness.getAccountAgeWitnessHash());
+        if (signedWitnessSetByAccountAgeWitnessHash.containsKey(accountAgeWitnessHash)) {
+            Set<SignedWitness> set = signedWitnessSetByAccountAgeWitnessHash.get(accountAgeWitnessHash);
+            set.remove(signedWitness);
+            if (set.isEmpty()) {
+                signedWitnessSetByAccountAgeWitnessHash.remove(accountAgeWitnessHash);
+            }
+        }
     }
 
     // Remove SignedWitnesses that are signed by TRADE that also have an ARBITRATOR signature

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

@@ -74,6 +74,7 @@ import java.util.Objects;
 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;
@@ -139,9 +140,13 @@ public class AccountAgeWitnessService {
     @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
@@ -234,8 +239,17 @@ public class AccountAgeWitnessService {
 
     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) {
@@ -285,12 +299,21 @@ public class AccountAgeWitnessService {
     private Optional<AccountAgeWitness> getWitnessByHash(byte[] hash) {
         P2PDataStorage.ByteArray hashAsByteArray = new P2PDataStorage.ByteArray(hash);
 
-        final boolean containsKey = accountAgeWitnessMap.containsKey(hashAsByteArray);
+        // First we look up in our fast lookup cache
-        if (!containsKey)
+        if (accountAgeWitnessCache.containsKey(hashAsByteArray)) {
-            log.debug("hash not found in accountAgeWitnessMap");
+            return Optional.of(accountAgeWitnessCache.get(hashAsByteArray));
+        }
 
-        return accountAgeWitnessMap.containsKey(hashAsByteArray) ?
+        if (accountAgeWitnessMap.containsKey(hashAsByteArray)) {
-                Optional.of(accountAgeWitnessMap.get(hashAsByteArray)) : Optional.empty();
+            AccountAgeWitness accountAgeWitness = accountAgeWitnessMap.get(hashAsByteArray);
+
+            // We add it to our fast lookup cache
+            accountAgeWitnessCache.put(hashAsByteArray, accountAgeWitness);
+
+            return Optional.of(accountAgeWitness);
+        }
+
+        return Optional.empty();
     }
 
     private Optional<AccountAgeWitness> getWitnessByHashAsHex(String hashAsHex) {
@@ -657,16 +680,20 @@ public class AccountAgeWitnessService {
     }
 
     public String arbitratorSignOrphanWitness(AccountAgeWitness accountAgeWitness,
-                                              ECKey key,
+                                              ECKey ecKey,
                                               long time) {
-        // Find AccountAgeWitness as signedwitness
+        // TODO Is not found signedWitness considered an error case?
-        var signedWitness = signedWitnessService.getSignedWitnessMap().values().stream()
+        //  Previous code version was throwing an exception in case no signedWitness was found...
-                .filter(sw -> Arrays.equals(sw.getAccountAgeWitnessHash(), accountAgeWitness.getHash()))
+
+        // signAndPublishAccountAgeWitness returns an empty string in success case and error otherwise
+        return signedWitnessService.getSignedWitnessSet(accountAgeWitness).stream()
                 .findAny()
-                .orElse(null);
+                .map(SignedWitness::getWitnessOwnerPubKey)
-        checkNotNull(signedWitness);
+                .map(witnessOwnerPubKey ->
-        return signedWitnessService.signAndPublishAccountAgeWitness(accountAgeWitness, key,
+                        signedWitnessService.signAndPublishAccountAgeWitness(accountAgeWitness, ecKey,
-                signedWitness.getWitnessOwnerPubKey(), time);
+                                witnessOwnerPubKey, time)
+                )
+                .orElse("No signedWitness found");
     }
 
     public String arbitratorSignOrphanPubKey(ECKey key,

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

@@ -30,6 +30,7 @@ import bisq.common.crypto.PubKeyRing;
 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 class AccountAgeWitnessUtils {
     }
 
     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 @@ public class AccountAgeWitnessUtils {
     // Log signers per
     public void logSigners() {
         log.info("Signers per AEW");
-        var allSig = signedWitnessService.getSignedWitnessMap();
+        Collection<SignedWitness> signedWitnessMapValues = signedWitnessService.getSignedWitnessMapValues();
-        allSig.values().forEach(w -> {
+        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 class AccountAgeWitnessServiceTest {
 
         // 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());
+        signedWitnessService.removeSignedWitness(signedWitnessArb);
-        assertEquals(signedWitnessService.getSignedWitnessMap().size(), 2);
+        assertEquals(signedWitnessService.getSignedWitnessMapValues().size(), 2);
 
         // Check that no account age witness is a signer
         assertFalse(service.accountIsSigner(aew1));
@@ -354,7 +354,7 @@ public class AccountAgeWitnessServiceTest {
                 witnessOwnerPubKey.getEncoded(),
                 time,
                 SignedWitnessService.MINIMUM_TRADE_AMOUNT_FOR_SIGNING.value);
-        signedWitnessService.getSignedWitnessMap().putIfAbsent(signedWitness.getHashAsByteArray(), signedWitness);
+        signedWitnessService.addToMap(signedWitness);
     }
 
 }