From 124ebe9fdff26a7962248f177193fe0149e8df6c Mon Sep 17 00:00:00 2001
From: zzz <zzz@i2pmail.org>
Date: Mon, 30 Jan 2023 08:05:39 -0500
Subject: [PATCH] Tunnels: Refactor peer selection

by checking if a peer qualifies when adding, rather than
iterating through the whole netdb to generate an exclusion list at the start.
This was very inefficient and generated needless lookup storms via lookupBeforeDropping()
Same idea for getClosetHopExclude()
Goal is to never iterate through all the known routers, profiles, or connected peers
to select peers for a single tunnel.
Add ExcluderBase and 4 classes for various cases:
Excluder, ClosestHopExcluder, IBGWExcluder, and OBEPExcluder.

Change CSFI.getEstablished() from a Set to a List for efficiency
Improve efficiency of selectActivePeersNotFailingPeers()
by iterating through connected list rather than profiles.
Do not add not-connected peers to exclude set,
which would become huge for hidden routers.

Change getExclude() to shouldExclude()
The exclude set calls shouldExclude() in contains().
Pass the exclude set to ProfileOrganizer.

For client tunnels, do OBEP and IBGW checks at peer selection time,
not afterwards in ConnectChecker, so it doesn't fail at the end in checkTunnel().

Check closest hop when hidden.
Fail-fast for inbound when no connected peers and hidden, do not fall back to non-connected peers.
Should improve startup time for hidden routers.

Use ArraySet for matches to save space
Remove unused selectPeersLocallyUnreachable() and selectPeersRecentlyRejecting()

No peer selection policy changes here.
---
 history.txt                                   |  21 ++
 .../src/net/i2p/router/CommSystemFacade.java  |   5 +-
 .../src/net/i2p/router/RouterVersion.java     |   2 +-
 .../net/i2p/router/dummy/VMCommSystem.java    |  10 +-
 .../router/peermanager/ProfileOrganizer.java  | 140 ++++------
 .../transport/CommSystemFacadeImpl.java       |   4 +-
 .../net/i2p/router/transport/Transport.java   |   3 +-
 .../router/transport/TransportManager.java    |  18 +-
 .../router/transport/ntcp/NTCPTransport.java  |   8 +-
 .../router/transport/udp/UDPTransport.java    |   6 +-
 .../i2p/router/tunnel/pool/BuildExecutor.java |   2 +-
 .../tunnel/pool/ClientPeerSelector.java       | 183 +++++++++----
 .../i2p/router/tunnel/pool/ExcluderBase.java  |  61 +++++
 .../tunnel/pool/ExploratoryPeerSelector.java  |  87 +++---
 .../tunnel/pool/TunnelPeerSelector.java       | 258 ++++++++++++------
 15 files changed, 524 insertions(+), 284 deletions(-)
 create mode 100644 router/java/src/net/i2p/router/tunnel/pool/ExcluderBase.java

diff --git a/history.txt b/history.txt
index 38dbf09088..a9bdd48a23 100644
--- a/history.txt
+++ b/history.txt
@@ -1,3 +1,24 @@
+2023-01-30 zzz
+ * Tunnels: Refactor peer selection
+
+2023-01-27 zzz
+ * Console: Debug page cleanups
+ * Tools: Add CLI reseed test
+ * Tunnels: Reduce grace period from 120 to 90 sec.
+
+2023-01-26 zzz
+ * Console: Add revision and build date to version info
+ * i2psnark: Search fixes
+
+2023-01-25 zzz
+ * Util: New thread-unsafe version of ObjectCounter
+
+2023-01-24 zzz
+ * Router: Preliminary support for congestion caps (proposal 162)
+
+2023-01-23 zzz
+ * i2psnark standalone: Fix running from outside the directory
+
 2023-01-22 zzz
  * Build: Fix list of changed files in manifests
  * i2psnark: Add max files per torrent config
diff --git a/router/java/src/net/i2p/router/CommSystemFacade.java b/router/java/src/net/i2p/router/CommSystemFacade.java
index 0b7c70f399..64cd9b9de6 100644
--- a/router/java/src/net/i2p/router/CommSystemFacade.java
+++ b/router/java/src/net/i2p/router/CommSystemFacade.java
@@ -13,7 +13,6 @@ import java.io.Writer;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.SortedMap;
 import java.util.TreeMap;
 
@@ -171,7 +170,7 @@ public abstract class CommSystemFacade implements Service {
      *  @return the hashes of all the routers we are connected to, non-null
      *  @since 0.9.34
      */
-    public abstract Set<Hash> getEstablished();
+    public abstract List<Hash> getEstablished();
     
     /** @since 0.8.13 */
     public boolean isDummy() { return true; }
@@ -395,9 +394,11 @@ public abstract class CommSystemFacade implements Service {
         IPV4_UNKNOWN_IPV6_OK(STATUS_IPV4_UNKNOWN_IPV6_OK, _x("IPv4: Testing; IPv6: OK")),
         IPV4_FIREWALLED_IPV6_OK(STATUS_IPV4_FIREWALLED_IPV6_OK, _x("IPv4: Firewalled; IPv6: OK")),
         IPV4_DISABLED_IPV6_OK(STATUS_IPV4_DISABLED_IPV6_OK, _x("IPv4: Disabled; IPv6: OK")),
+        /** IPv4 symmetric NAT (not source NAT) */
         IPV4_SNAT_IPV6_OK(STATUS_IPV4_SNAT_IPV6_OK, _x("IPv4: Symmetric NAT; IPv6: OK")),
         /** IPv4 symmetric NAT, IPv6 firewalled or disabled or no address */
         DIFFERENT(STATUS_DIFFERENT, _x("Symmetric NAT")),
+        /** IPv4 symmetric NAT (not source NAT) */
         IPV4_SNAT_IPV6_UNKNOWN(STATUS_IPV4_SNAT_IPV6_UNKNOWN, _x("IPv4: Symmetric NAT; IPv6: Testing")),
         IPV4_FIREWALLED_IPV6_UNKNOWN(STATUS_IPV4_FIREWALLED_IPV6_UNKNOWN, _x("IPv4: Firewalled; IPv6: Testing")),
         /** IPv4 firewalled, IPv6 firewalled or disabled or no address */
diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java
index 35ab2b80b1..9bc4388a0c 100644
--- a/router/java/src/net/i2p/router/RouterVersion.java
+++ b/router/java/src/net/i2p/router/RouterVersion.java
@@ -18,7 +18,7 @@ public class RouterVersion {
     /** deprecated */
     public final static String ID = "Git";
     public final static String VERSION = CoreVersion.VERSION;
-    public final static long BUILD = 4;
+    public final static long BUILD = 5;
 
     /** for example "-test" */
     public final static String EXTRA = "";
diff --git a/router/java/src/net/i2p/router/dummy/VMCommSystem.java b/router/java/src/net/i2p/router/dummy/VMCommSystem.java
index 6f2df37924..ead455361c 100644
--- a/router/java/src/net/i2p/router/dummy/VMCommSystem.java
+++ b/router/java/src/net/i2p/router/dummy/VMCommSystem.java
@@ -3,10 +3,10 @@ package net.i2p.router.dummy;
 import java.io.IOException;
 import java.io.Writer;
 import java.util.Collections;
+import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import net.i2p.data.Hash;
 import net.i2p.data.i2np.I2NPMessage;
@@ -67,10 +67,10 @@ public class VMCommSystem extends CommSystemFacade {
 
     public boolean isEstablished(Hash peer) { return _commSystemFacades.containsKey(peer); }
 
-    public Set<Hash> getEstablished() {
-        Set<Hash> rv;
+    public List<Hash> getEstablished() {
+        List<Hash> rv;
         synchronized (_commSystemFacades) {
-            rv = new HashSet<Hash>(_commSystemFacades.keySet());
+            rv = new ArrayList<Hash>(_commSystemFacades.keySet());
         }
         Hash us = _context.routerHash();
         if (us != null)
diff --git a/router/java/src/net/i2p/router/peermanager/ProfileOrganizer.java b/router/java/src/net/i2p/router/peermanager/ProfileOrganizer.java
index 568dc8e9d9..ce23263de3 100644
--- a/router/java/src/net/i2p/router/peermanager/ProfileOrganizer.java
+++ b/router/java/src/net/i2p/router/peermanager/ProfileOrganizer.java
@@ -604,7 +604,7 @@ public class ProfileOrganizer {
      * Caution, this does NOT cascade further to non-connected peers, so it should only
      * be used when there is a good number of connected peers.
      *
-     * @param exclude non-null, WARNING - side effect, all not-connected peers are added
+     * @param exclude non-null, not-connected peers will NOT be added, as of 0.9.58
      */
     public void selectActiveNotFailingPeers(int howMany, Set<Hash> exclude, Set<Hash> matches) {
         selectActiveNotFailingPeers(howMany, exclude, matches, 0, null);
@@ -621,7 +621,7 @@ public class ProfileOrganizer {
      * Caution, this does NOT cascade further to non-connected peers, so it should only
      * be used when there is a good number of connected peers.
      *
-     * @param exclude non-null, WARNING - side effect, all not-connected peers are added
+     * @param exclude non-null, not-connected peers will NOT be added, as of 0.9.58
      * @param mask 0-4 Number of bytes to match to determine if peers in the same IP range should
      *             not be in the same tunnel. 0 = disable check; 1 = /8; 2 = /16; 3 = /24; 4 = exact IP match
      * @param ipSet may be null only if mask is 0
@@ -629,14 +629,12 @@ public class ProfileOrganizer {
      */
     public void selectActiveNotFailingPeers(int howMany, Set<Hash> exclude, Set<Hash> matches, int mask, MaskedIPSet ipSet) {
         if (matches.size() < howMany) {
-            Set<Hash> connected = _context.commSystem().getEstablished();
+            List<Hash> connected = _context.commSystem().getEstablished();
+            if (connected.isEmpty())
+                return;
             getReadLock();
             try {
-                for (Hash peer : _notFailingPeers.keySet()) {
-                    if (!connected.contains(peer))
-                        exclude.add(peer);
-                }
-                locked_selectPeers(_notFailingPeers, howMany, exclude, matches, mask, ipSet);
+                locked_selectActive(connected, howMany, exclude, matches, mask, ipSet);
             } finally { releaseReadLock(); }
         }
     }
@@ -651,6 +649,7 @@ public class ProfileOrganizer {
      *
      * This DOES cascade further to non-connected peers.
      *
+     * @param exclude non-null, not-connected peers will NOT be added, as of 0.9.58
      * @param mask 0-4 Number of bytes to match to determine if peers in the same IP range should
      *             not be in the same tunnel. 0 = disable check; 1 = /8; 2 = /16; 3 = /24; 4 = exact IP match
      * @param ipSet in/out param, use for multiple calls, may be null only if mask is 0
@@ -658,17 +657,13 @@ public class ProfileOrganizer {
      */
     private void selectActiveNotFailingPeers2(int howMany, Set<Hash> exclude, Set<Hash> matches, int mask, MaskedIPSet ipSet) {
         if (matches.size() < howMany) {
-            Set<Hash> connected = _context.commSystem().getEstablished();
-            Map<Hash, PeerProfile> activePeers = new HashMap<Hash, PeerProfile>(connected.size());
-            getReadLock();
-            try {
-                for (Hash peer : connected) {
-                    PeerProfile prof = _notFailingPeers.get(peer);
-                    if (prof != null)
-                        activePeers.put(peer, prof);
-                }
-                locked_selectPeers(activePeers, howMany, exclude, matches, mask, ipSet);
-            } finally { releaseReadLock(); }
+            List<Hash> connected = _context.commSystem().getEstablished();
+            if (!connected.isEmpty()) {
+                getReadLock();
+                try {
+                    locked_selectActive(connected, howMany, exclude, matches, mask, ipSet);
+                } finally { releaseReadLock(); }
+            }
         }
         if (matches.size() < howMany) {
             if (_log.shouldLog(Log.INFO))
@@ -745,69 +740,6 @@ public class ProfileOrganizer {
         return;        
     }                  
 
-    /**                
-     * Get the peers the transport layer thinks are unreachable,
-     * and peers requiring introducers.
-     *                 
-     */                
-    public List<Hash> selectPeersLocallyUnreachable() { 
-        List<Hash> n;
-        int count;
-        getReadLock();
-        try {
-            count = _notFailingPeers.size();
-            n = new ArrayList<Hash>(_notFailingPeers.keySet());
-        } finally { releaseReadLock(); }
-        List<Hash> l = new ArrayList<Hash>(count / 4);
-        for (Hash peer : n) {
-            if (_context.commSystem().wasUnreachable(peer)) {
-                l.add(peer);
-            } else {
-                // Blacklist all peers requiring SSU introducers, because either
-                //  a) it's slow; or
-                //  b) it doesn't work very often; or
-                //  c) in the event they are advertising NTCP, it probably won't work because
-                //     they probably don't have a TCP hole punched in their firewall either.
-                RouterInfo info = _context.netDb().lookupRouterInfoLocally(peer);
-                if (info != null) {
-                        RouterAddress ra = info.getTargetAddress("SSU");
-                        // peers with no SSU address at all are fine.
-                        // as long as they have NTCP
-                        if (ra == null) {
-                            if (info.getTargetAddresses("NTCP", "NTCP2").isEmpty())
-                                l.add(peer);
-                            continue;
-                        }
-                        // This is the quick way of doing UDPAddress.getIntroducerCount() > 0
-                        if (ra.getOption("itag0") != null)
-                            l.add(peer);
-                }
-            }
-        }
-        if (_log.shouldLog(Log.DEBUG))
-            _log.debug("Unreachable: " + l);
-        return l;
-    }
-
-    /**
-     * Get the peers that have recently rejected us for bandwidth
-     * recent == last 20s
-     *
-     */
-    public List<Hash> selectPeersRecentlyRejecting() { 
-        getReadLock();
-        try {
-            long cutoff = _context.clock().now() - (20*1000);
-            int count = _notFailingPeers.size();
-            List<Hash> l = new ArrayList<Hash>(count / 128);
-            for (PeerProfile prof : _notFailingPeers.values()) {
-                if (prof.getTunnelHistory().getLastRejectedBandwidth() > cutoff)
-                    l.add(prof.getPeer());
-            }
-            return l;
-        } finally { releaseReadLock(); }
-    }
-
     /**
      * Find the hashes for all peers we are actively profiling
      *
@@ -1315,6 +1247,9 @@ public class ProfileOrganizer {
                 ok = mask <= 0 || notRestricted(peer, ipSet, mask);
                 if ((!ok) && _log.shouldWarn())
                     _log.warn("IP restriction prevents " + peer + " from joining " + matches);
+            } else {
+                if (toExclude != null)
+                    toExclude.add(peer);
             }
             if (ok)
                 matches.add(peer);
@@ -1322,6 +1257,44 @@ public class ProfileOrganizer {
                 matches.remove(peer);
         }
     }
+
+    /**
+     *
+     * For efficiency. Rather than iterating through _notFailingPeers looking for connected peers,
+     * iterate through the connected peers and then check if failing.
+     *
+     * @param mask 0-4 Number of bytes to match to determine if peers in the same IP range should
+     *             not be in the same tunnel. 0 = disable check; 1 = /8; 2 = /16; 3 = /24; 4 = exact IP match
+     * @param ipSet may be null only if mask is 0
+     * @since 0.9.58
+     */
+    private void locked_selectActive(List<Hash> connected, int howMany, Set<Hash> toExclude, Set<Hash> matches,
+                                     int mask, MaskedIPSet ipSet) {
+        // use RandomIterator to avoid shuffling the whole thing
+        for (Iterator<Hash> iter = new RandomIterator<Hash>(connected); (matches.size() < howMany) && iter.hasNext(); ) {
+            Hash peer = iter.next();
+            if (toExclude != null && toExclude.contains(peer))
+                continue;
+            if (matches.contains(peer))
+                continue;
+            if (_us.equals(peer))
+                continue;
+            if (_failingPeers.containsKey(peer))
+                continue;
+            // we assume if connected, it's fine, don't look in _notFailingPeers
+            boolean ok = isSelectable(peer);
+            if (ok) {
+                ok = mask <= 0 || notRestricted(peer, ipSet, mask);
+                if ((!ok) && _log.shouldWarn())
+                    _log.warn("IP restriction prevents " + peer + " from joining " + matches);
+            } else {
+                if (toExclude != null)
+                    toExclude.add(peer);
+            }
+            if (ok)
+                matches.add(peer);
+        }
+    }
     
     /**
      * Does the peer's IP address NOT match the IP address of any peer already in the set,
@@ -1383,6 +1356,9 @@ public class ProfileOrganizer {
                 ok = mask <= 0 || notRestricted(peer, ipSet, mask);
                 if ((!ok) && _log.shouldWarn())
                     _log.warn("IP restriction prevents " + peer + " from joining " + matches);
+            } else {
+                if (toExclude != null)
+                    toExclude.add(peer);
             }
             if (ok)
                 matches.add(peer);
diff --git a/router/java/src/net/i2p/router/transport/CommSystemFacadeImpl.java b/router/java/src/net/i2p/router/transport/CommSystemFacadeImpl.java
index 48356b0cd8..9ad13754b8 100644
--- a/router/java/src/net/i2p/router/transport/CommSystemFacadeImpl.java
+++ b/router/java/src/net/i2p/router/transport/CommSystemFacadeImpl.java
@@ -183,10 +183,10 @@ public class CommSystemFacadeImpl extends CommSystemFacade {
     }
 
     /**
-     *  @return a new set, may be modified
+     *  @return a new list, may be modified
      *  @since 0.9.34
      */    
-    public Set<Hash> getEstablished() {
+    public List<Hash> getEstablished() {
         return _manager.getEstablished();
     }
 
diff --git a/router/java/src/net/i2p/router/transport/Transport.java b/router/java/src/net/i2p/router/transport/Transport.java
index 552a249d22..c22b383695 100644
--- a/router/java/src/net/i2p/router/transport/Transport.java
+++ b/router/java/src/net/i2p/router/transport/Transport.java
@@ -11,7 +11,6 @@ package net.i2p.router.transport;
 import java.io.IOException;
 import java.io.Writer;
 import java.util.List;
-import java.util.Set;
 
 import net.i2p.data.Hash;
 import net.i2p.data.router.RouterAddress;
@@ -168,7 +167,7 @@ public interface Transport {
      * @return may or may not be modifiable; check implementation
      * @since 0.9.34
      */
-    public Set<Hash> getEstablished();    
+    public List<Hash> getEstablished();
 
     public int countPeers();    
     public int countActivePeers();    
diff --git a/router/java/src/net/i2p/router/transport/TransportManager.java b/router/java/src/net/i2p/router/transport/TransportManager.java
index d765caefcd..68c5558461 100644
--- a/router/java/src/net/i2p/router/transport/TransportManager.java
+++ b/router/java/src/net/i2p/router/transport/TransportManager.java
@@ -667,20 +667,22 @@ public class TransportManager implements TransportEventListener {
     }    
     
     /**
-     *  @return a new set, may be modified
+     *  @return a new list, may be modified
      *  @since 0.9.34
      */
-    public Set<Hash> getEstablished() {
-        // for efficiency. NTCP is modifiable, SSU isn't
+    public List<Hash> getEstablished() {
+        // for efficiency
         Transport t = _transports.get("NTCP");
-        Set<Hash> rv;
+        List<Hash> rv = null;
         if (t != null)
             rv = t.getEstablished();
-        else
-            rv = new HashSet<Hash>(256);
         t = _transports.get("SSU");
-        if (t != null)
-            rv.addAll(t.getEstablished());
+        if (t != null) {
+            if (rv != null)
+                rv.addAll(t.getEstablished());
+            else
+                rv = t.getEstablished();
+        }
         return rv;
     }
     
diff --git a/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java b/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java
index 141bd41144..e57fcbba97 100644
--- a/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java
+++ b/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java
@@ -760,12 +760,12 @@ public class NTCPTransport extends TransportImpl {
      * @return a copy, modifiable
      * @since 0.9.34
      */
-    public Set<Hash> getEstablished() {
-        Set<Hash> rv = new HashSet<Hash>(_conByIdent.keySet());
+    public List<Hash> getEstablished() {
+        List<Hash> rv = new ArrayList<Hash>(_conByIdent.size());
         for (Map.Entry<Hash, NTCPConnection> e : _conByIdent.entrySet()) {
             NTCPConnection con = e.getValue();
-            if (!con.isEstablished() || con.isClosed())
-                rv.remove(e.getKey());
+            if (con.isEstablished() && !con.isClosed())
+                rv.add(e.getKey());
         }
         return rv;
     }
diff --git a/router/java/src/net/i2p/router/transport/udp/UDPTransport.java b/router/java/src/net/i2p/router/transport/udp/UDPTransport.java
index cba2007dbc..d4915c54cb 100644
--- a/router/java/src/net/i2p/router/transport/udp/UDPTransport.java
+++ b/router/java/src/net/i2p/router/transport/udp/UDPTransport.java
@@ -1803,11 +1803,11 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority
     /** 
      * Connected peers.
      *
-     * @return not a copy, do not modify
+     * @return a copy, modifiable
      * @since 0.9.34
      */
-    public Set<Hash> getEstablished() {
-        return _peersByIdent.keySet();
+    public List<Hash> getEstablished() {
+        return new ArrayList<Hash>(_peersByIdent.keySet());
     }
 
     /**
diff --git a/router/java/src/net/i2p/router/tunnel/pool/BuildExecutor.java b/router/java/src/net/i2p/router/tunnel/pool/BuildExecutor.java
index 395d93a130..31167e7363 100644
--- a/router/java/src/net/i2p/router/tunnel/pool/BuildExecutor.java
+++ b/router/java/src/net/i2p/router/tunnel/pool/BuildExecutor.java
@@ -121,7 +121,7 @@ class BuildExecutor implements Runnable {
         CommSystemFacade csf = _context.commSystem();
         if (csf.getStatus() == Status.DISCONNECTED)
             return 0;
-        if (csf.isDummy() && csf.getEstablished().size() <= 0)
+        if (csf.isDummy() && csf.countActivePeers() <= 0)
             return 0;
         int maxKBps = _context.bandwidthLimiter().getOutboundKBytesPerSecond();
         int allowed = maxKBps / 6; // Max. 1 concurrent build per 6 KB/s outbound
diff --git a/router/java/src/net/i2p/router/tunnel/pool/ClientPeerSelector.java b/router/java/src/net/i2p/router/tunnel/pool/ClientPeerSelector.java
index f1d726d198..1134646fa1 100644
--- a/router/java/src/net/i2p/router/tunnel/pool/ClientPeerSelector.java
+++ b/router/java/src/net/i2p/router/tunnel/pool/ClientPeerSelector.java
@@ -1,7 +1,6 @@
 package net.i2p.router.tunnel.pool;
 
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -14,6 +13,7 @@ import net.i2p.router.TunnelManagerFacade;
 import net.i2p.router.TunnelPoolSettings;
 import static net.i2p.router.peermanager.ProfileOrganizer.Slice.*;
 import net.i2p.router.util.MaskedIPSet;
+import net.i2p.util.ArraySet;
 
 /**
  * Pick peers randomly out of the fast pool, and put them into tunnels
@@ -52,6 +52,8 @@ class ClientPeerSelector extends TunnelPeerSelector {
             boolean v6Only = isIPv6Only();
             boolean ntcpDisabled = isNTCPDisabled();
             boolean ssuDisabled = isSSUDisabled();
+            // for these cases, check the closest hop up front,
+            // otherwise, will be done in checkTunnel() at the end
             boolean checkClosestHop = v6Only || ntcpDisabled || ssuDisabled;
             boolean hidden = ctx.router().isHidden() ||
                              ctx.router().getRouterInfo().getAddressCount() <= 0 ||
@@ -65,21 +67,27 @@ class ClientPeerSelector extends TunnelPeerSelector {
                 return selectExplicit(settings, length);
 
             Set<Hash> exclude = getExclude(isInbound, false);
-            Set<Hash> matches = new HashSet<Hash>(length);
+            Set<Hash> matches = new ArraySet<Hash>(length);
             if (length == 1) {
                 // closest-hop restrictions
-                if (checkClosestHop) {
-                    Set<Hash> moreExclude = getClosestHopExclude(isInbound);
-                    if (moreExclude != null)
-                        exclude.addAll(moreExclude);
-                }
+                if (checkClosestHop)
+                    exclude = getClosestHopExclude(isInbound, exclude);
+                if (isInbound)
+                    exclude = new IBGWExcluder(exclude);
+                else
+                    exclude = new OBEPExcluder(exclude);
                 // 1-hop, IP restrictions not required here
                 if (hiddenInbound) {
-                    // SANFP adds all not-connected to exclude, so make a copy
-                    Set<Hash> SANFPExclude = new HashSet<Hash>(exclude);
-                    ctx.profileOrganizer().selectActiveNotFailingPeers(1, SANFPExclude, matches);
+                    // TODO this doesn't pick from fast
+                    ctx.profileOrganizer().selectActiveNotFailingPeers(1, exclude, matches);
                 }
                 if (matches.isEmpty()) {
+                    if (hiddenInbound) {
+                        // No connected peers found, give up now
+                        if (log.shouldWarn())
+                            log.warn("CPS SANFP hidden closest IB no active peers found, returning null");
+                        return null;
+                    }
                     // ANFP does not fall back to non-connected
                     ctx.profileOrganizer().selectFastPeers(length, exclude, matches);
                 }
@@ -95,34 +103,30 @@ class ClientPeerSelector extends TunnelPeerSelector {
                 // group 0 or 1 if two hops, otherwise group 0
                 Set<Hash> lastHopExclude;
                 if (isInbound) {
-                    // exclude existing OBEPs to get some diversity ?
-                    // closest-hop restrictions
-                    if (checkClosestHop) {
-                        Set<Hash> moreExclude = getClosestHopExclude(false);
-                        if (moreExclude != null) {
-                            moreExclude.addAll(exclude);
-                            lastHopExclude = moreExclude;
-                        } else {
-                            lastHopExclude = exclude;
-                        }
+                    if (checkClosestHop && !hidden) {
+                        // exclude existing OBEPs to get some diversity ?
+                        // closest-hop restrictions
+                        lastHopExclude = getClosestHopExclude(true, exclude);
                     } else {
-                         lastHopExclude = exclude;
+                        lastHopExclude = exclude;
                     }
+                    if (log.shouldInfo())
+                        log.info("CPS SFP closest IB " + lastHopExclude);
                 } else {
-                    lastHopExclude = exclude;
+                    lastHopExclude = new OBEPExcluder(exclude);
+                    if (log.shouldInfo())
+                        log.info("CPS SFP OBEP " + lastHopExclude);
                 }
                 if (hiddenInbound) {
                     // IB closest hop
                     if (log.shouldInfo())
-                        log.info("CPS SANFP closest IB exclude " + lastHopExclude.size());
-                    // SANFP adds all not-connected to exclude, so make a copy
-                    Set<Hash> SANFPExclude = new HashSet<Hash>(lastHopExclude);
-                    ctx.profileOrganizer().selectActiveNotFailingPeers(1, SANFPExclude, matches, ipRestriction, ipSet);
+                        log.info("CPS SANFP hidden closest IB " + lastHopExclude);
+                    ctx.profileOrganizer().selectActiveNotFailingPeers(1, lastHopExclude, matches, ipRestriction, ipSet);
                     if (matches.isEmpty()) {
-                        if (log.shouldInfo())
-                            log.info("CPS SFP closest IB exclude " + lastHopExclude.size());
-                        // ANFP does not fall back to non-connected
-                        ctx.profileOrganizer().selectFastPeers(1, lastHopExclude, matches, randomKey, length == 2 ? SLICE_0_1 : SLICE_0, ipRestriction, ipSet);
+                        // No connected peers found, give up now
+                        if (log.shouldWarn())
+                            log.warn("CPS SANFP hidden closest IB no active peers found, returning null");
+                        return null;
                     }
                 } else if (hiddenOutbound) {
                     // OBEP
@@ -178,15 +182,13 @@ class ClientPeerSelector extends TunnelPeerSelector {
                     }
                     if (pickFurthest) {
                         if (log.shouldInfo())
-                            log.info("CPS SANFP OBEP exclude " + lastHopExclude.size());
-                        // SANFP adds all not-connected to exclude, so make a copy
-                        Set<Hash> SANFPExclude = new HashSet<Hash>(lastHopExclude);
-                        ctx.profileOrganizer().selectActiveNotFailingPeers(1, SANFPExclude, matches, ipRestriction, ipSet);
+                            log.info("CPS SANFP OBEP " + lastHopExclude);
+                        ctx.profileOrganizer().selectActiveNotFailingPeers(1, lastHopExclude, matches, ipRestriction, ipSet);
                         if (matches.isEmpty()) {
-                            // ANFP does not fall back to non-connected
-                            if (log.shouldInfo())
-                                log.info("CPS SFP OBEP exclude " + lastHopExclude.size());
-                            ctx.profileOrganizer().selectFastPeers(1, lastHopExclude, matches, randomKey, length == 2 ? SLICE_0_1 : SLICE_0, ipRestriction, ipSet);
+                            // No connected peers found, give up now
+                            if (log.shouldWarn())
+                                log.warn("CPS SANFP hidden OBEP no active peers found, returning null");
+                            return null;
                         }
                     } else {
                         ctx.profileOrganizer().selectFastPeers(1, lastHopExclude, matches, randomKey, length == 2 ? SLICE_0_1 : SLICE_0, ipRestriction, ipSet);
@@ -203,6 +205,8 @@ class ClientPeerSelector extends TunnelPeerSelector {
                 if (length > 2) {
                     // middle hop(s)
                     // group 2 or 3
+                    if (log.shouldInfo())
+                        log.info("CPS SFP middle " + exclude);
                     ctx.profileOrganizer().selectFastPeers(length - 2, exclude, matches, randomKey, SLICE_2_3, ipRestriction, ipSet);
                     matches.remove(ctx.routerHash());
                     if (matches.size() > 1) {
@@ -219,20 +223,25 @@ class ClientPeerSelector extends TunnelPeerSelector {
 
                 // IBGW or OB first hop
                 // group 2 or 3 if two hops, otherwise group 1
-                if (!isInbound) {
+                if (isInbound) {
+                    exclude = new IBGWExcluder(exclude);
+                    if (log.shouldInfo())
+                        log.info("CPS SFP IBGW " + exclude);
+                } else {
                     // exclude existing IBGWs to get some diversity ?
-                    // closest-hop restrictions
-                    if (checkClosestHop) {
-                        Set<Hash> moreExclude = getClosestHopExclude(true);
-                        if (moreExclude != null)
-                            exclude.addAll(moreExclude);
-                    }
+                    // OB closest-hop restrictions
+                    if (checkClosestHop)
+                        exclude = getClosestHopExclude(false, exclude);
+                    if (log.shouldInfo())
+                        log.info("CPS SFP closest OB " + exclude);
                 }
                 // TODO exclude IPv6-only at IBGW? Caught in checkTunnel() below
                 ctx.profileOrganizer().selectFastPeers(1, exclude, matches, randomKey, length == 2 ? SLICE_2_3 : SLICE_1, ipRestriction, ipSet);
                 matches.remove(ctx.routerHash());
                 rv.addAll(matches);
             }
+            if (log.shouldInfo())
+                log.info("CPS " + length + (isInbound ? " IB " : " OB ") + "final: " + exclude);
             if (rv.size() < length) {
                 // not enough peers to build the requested size
                 // client tunnels do not use overrides
@@ -255,9 +264,91 @@ class ClientPeerSelector extends TunnelPeerSelector {
         else
             rv.add(ctx.routerHash());
         if (rv.size() > 1) {
-            if (!checkTunnel(isInbound, rv))
+            if (!checkTunnel(isInbound, false, rv))
                 rv = null;
         }
         return rv;
     }
+
+    /**
+     *  A Set of Hashes that automatically adds to the
+     *  Set in the contains() check.
+     *
+     *  So we don't need to generate the exclude set up front.
+     *
+     *  @since 0.9.58
+     */
+    private class IBGWExcluder extends ExcluderBase {
+
+        /**
+         *  Automatically check if peer is connected
+         *  and add the Hash to the set if not.
+         *
+         *  @param set not copied, contents will be modified by all methods
+         */
+        public IBGWExcluder(Set<Hash> set) {
+            super(set);
+        }
+
+        /**
+         *  Automatically check if peer is connected
+         *  and add the Hash to the set if not.
+         *
+         *  @param o a Hash
+         *  @return true if peer should be excluded
+         */
+        public boolean contains(Object o) {
+            if (s.contains(o))
+                return true;
+            Hash h = (Hash) o;
+            boolean rv = !allowAsIBGW(h);
+            if (rv) {
+                s.add(h);
+                if (log.shouldDebug())
+                    log.debug("CPS IBGW exclude " + h.toBase64());
+            }
+            return rv;
+        }
+    }
+
+    /**
+     *  A Set of Hashes that automatically adds to the
+     *  Set in the contains() check.
+     *
+     *  So we don't need to generate the exclude set up front.
+     *
+     *  @since 0.9.58
+     */
+    private class OBEPExcluder extends ExcluderBase {
+
+        /**
+         *  Automatically check if peer is connected
+         *  and add the Hash to the set if not.
+         *
+         *  @param set not copied, contents will be modified by all methods
+         */
+        public OBEPExcluder(Set<Hash> set) {
+            super(set);
+        }
+
+        /**
+         *  Automatically check if peer is connected
+         *  and add the Hash to the set if not.
+         *
+         *  @param o a Hash
+         *  @return true if peer should be excluded
+         */
+        public boolean contains(Object o) {
+            if (s.contains(o))
+                return true;
+            Hash h = (Hash) o;
+            boolean rv = !allowAsOBEP(h);
+            if (rv) {
+                s.add(h);
+                if (log.shouldDebug())
+                    log.debug("CPS OBEP exclude " + h.toBase64());
+            }
+            return rv;
+        }
+    }
 }
diff --git a/router/java/src/net/i2p/router/tunnel/pool/ExcluderBase.java b/router/java/src/net/i2p/router/tunnel/pool/ExcluderBase.java
new file mode 100644
index 0000000000..ff0fdf05ad
--- /dev/null
+++ b/router/java/src/net/i2p/router/tunnel/pool/ExcluderBase.java
@@ -0,0 +1,61 @@
+package net.i2p.router.tunnel.pool;
+
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.Set;
+
+import net.i2p.data.Hash;
+
+/**
+ *  A Set of Hashes that automatically adds to the
+ *  Set in the contains() check.
+ *
+ *  So we don't need to generate the exclude set up front.
+ *  Less object churn and copying.
+ *
+ *  @since 0.9.58
+ */
+abstract class ExcluderBase implements Set<Hash> {
+    protected final Set<Hash> s;
+
+    /**
+     *  Automatically check if peer is connected
+     *  and add the Hash to the set if not.
+     *
+     *  @param set not copied, contents will be modified by all methods
+     */
+    protected ExcluderBase(Set<Hash> set) {
+        s = set;
+    }
+
+    /**
+     *  Automatically check if peer is allowed
+     *  and add the Hash to the set if not.
+     *
+     *  @param o a Hash
+     *  @return true if peer should be excluded
+     */
+    public abstract boolean contains(Object o);
+
+    public boolean add(Hash h) { return s.add(h); }
+    public boolean addAll(Collection<? extends Hash> c) { return s.addAll(c); }
+    public void clear() { s.clear(); }
+    public boolean containsAll(Collection<?> c) { return s.containsAll(c); }
+    public boolean equals(Object o) { return s.equals(o); }
+    public int hashCode() { return s.hashCode(); }
+    public boolean isEmpty() { return s.isEmpty(); }
+    public Iterator<Hash> iterator() { return s.iterator(); }
+    public boolean remove(Object o) { return s.remove(o); }
+    public boolean removeAll(Collection<?> c) { return s.removeAll(c); }
+    public boolean retainAll(Collection<?> c) { return s.retainAll(c); }
+    public int size() { return s.size(); }
+    public Object[] toArray() { return s.toArray(); }
+    public <Hash> Hash[] toArray(Hash[] a) { return s.toArray(a); }
+
+    @Override
+    public String toString() {
+         return getClass().getSimpleName() +
+                " (" + s.size() + ") " +
+                (s.size() <= 10 ? s.toString() : "");
+    }
+}
diff --git a/router/java/src/net/i2p/router/tunnel/pool/ExploratoryPeerSelector.java b/router/java/src/net/i2p/router/tunnel/pool/ExploratoryPeerSelector.java
index fbff109229..38c40dd119 100644
--- a/router/java/src/net/i2p/router/tunnel/pool/ExploratoryPeerSelector.java
+++ b/router/java/src/net/i2p/router/tunnel/pool/ExploratoryPeerSelector.java
@@ -1,7 +1,6 @@
 package net.i2p.router.tunnel.pool;
 
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -14,6 +13,7 @@ import net.i2p.router.TunnelPoolSettings;
 import net.i2p.router.util.MaskedIPSet;
 import net.i2p.stat.Rate;
 import net.i2p.stat.RateStat;
+import net.i2p.util.ArraySet;
 import net.i2p.util.Log;
 import net.i2p.util.SystemVersion;
 
@@ -64,6 +64,8 @@ class ExploratoryPeerSelector extends TunnelPeerSelector {
         boolean v6Only = nonzero && isIPv6Only();
         boolean ntcpDisabled = nonzero && isNTCPDisabled();
         boolean ssuDisabled = nonzero && isSSUDisabled();
+        // for these cases, check the closest hop up front,
+        // otherwise, will be done in checkTunnel() at the end
         boolean checkClosestHop = v6Only || ntcpDisabled || ssuDisabled;
         boolean hidden = nonzero && (ctx.router().isHidden() ||
                                      ctx.router().getRouterInfo().getAddressCount() <= 0 ||
@@ -83,43 +85,41 @@ class ExploratoryPeerSelector extends TunnelPeerSelector {
         if (v6Only || hiddenInbound || lowOutbound) {
             Set<Hash> closestExclude;
             if (checkClosestHop) {
-                closestExclude = getClosestHopExclude(isInbound);
-                if (closestExclude != null)
-                    closestExclude.addAll(exclude);
-                else
-                    closestExclude = exclude;
+                closestExclude = getClosestHopExclude(isInbound, exclude);
             } else {
                 closestExclude = exclude;
             }
 
-            Set<Hash> closest = new HashSet<Hash>(1);
+            ArraySet<Hash> closest = new ArraySet<Hash>(1);
             if (hiddenInbound || lowOutbound) {
-                // If hidden and inbound, use fast peers - that we probably have recently
-                // connected to and so they have our real RI - to maximize the chance
+                // If hidden and inbound, use connected peers to guarantee
                 // that the adjacent hop can connect to us.
-                // use only connected peers so we don't make more connections
-                if (log.shouldLog(Log.INFO))
-                    log.info("EPS SANFP closest " + (isInbound ? "IB" : "OB") + " exclude " + closestExclude.size());
-                // SANFP adds all not-connected to exclude, so make a copy
-                Set<Hash> SANFPExclude = new HashSet<Hash>(closestExclude);
-                ctx.profileOrganizer().selectActiveNotFailingPeers(1, SANFPExclude, closest, ipRestriction, ipSet);
+                if (log.shouldInfo())
+                    log.info("EPS SANFP closest " + (isInbound ? "IB " : "OB ") + closestExclude);
+                ctx.profileOrganizer().selectActiveNotFailingPeers(1, closestExclude, closest, ipRestriction, ipSet);
                 if (closest.isEmpty()) {
+                    if (hiddenInbound) {
+                        // No connected peers found, give up now
+                        if (log.shouldWarn())
+                            log.warn("EPS SANFP hidden closest IB no active peers found, returning null");
+                        return null;
+                    }
                     // ANFP does not fall back to non-connected
-                    if (log.shouldLog(Log.INFO))
-                        log.info("EPS SFP closest " + (isInbound ? "IB" : "OB") + " exclude " + closestExclude.size());
+                    if (log.shouldInfo())
+                        log.info("EPS SFP closest " + (isInbound ? "IB " : "OB ") + closestExclude);
                     ctx.profileOrganizer().selectFastPeers(1, closestExclude, closest, ipRestriction, ipSet);
                 }
             } else if (exploreHighCap) {
-                if (log.shouldLog(Log.INFO))
-                    log.info("EPS SHCP closest " + (isInbound ? "IB" : "OB") + " exclude " + closestExclude.size());
+                if (log.shouldInfo())
+                    log.info("EPS SHCP closest " + (isInbound ? "IB " : "OB ") + closestExclude);
                 ctx.profileOrganizer().selectHighCapacityPeers(1, closestExclude, closest, ipRestriction, ipSet);
             } else {
-                if (log.shouldLog(Log.INFO))
-                    log.info("EPS SNFP closest " + (isInbound ? "IB" : "OB") + " exclude " + closestExclude.size());
+                if (log.shouldInfo())
+                    log.info("EPS SNFP closest " + (isInbound ? "IB " : "OB ") + closestExclude);
                 ctx.profileOrganizer().selectNotFailingPeers(1, closestExclude, closest, false, ipRestriction, ipSet);
             }
             if (!closest.isEmpty()) {
-                closestHop = closest.iterator().next();
+                closestHop = closest.get(0);
                 exclude.add(closestHop);
                 length--;
             }
@@ -153,38 +153,30 @@ class ExploratoryPeerSelector extends TunnelPeerSelector {
                 }
             }
             if (pickFurthest) {
-                Set<Hash> furthest = new HashSet<Hash>(1);
-                if (log.shouldLog(Log.INFO))
-                    log.info("EPS SANFP furthest OB exclude " + exclude.size());
-                // ANFP adds all not-connected to exclude, so make a copy
-                Set<Hash> SANFPExclude = new HashSet<Hash>(exclude);
-                ctx.profileOrganizer().selectActiveNotFailingPeers(1, SANFPExclude, furthest, ipRestriction, ipSet);
+                ArraySet<Hash> furthest = new ArraySet<Hash>(1);
+                if (log.shouldInfo())
+                    log.info("EPS SANFP OBEP exclude " + exclude);
+                ctx.profileOrganizer().selectActiveNotFailingPeers(1, exclude, furthest, ipRestriction, ipSet);
                 if (furthest.isEmpty()) {
                     // ANFP does not fall back to non-connected
-                    if (log.shouldLog(Log.INFO))
-                        log.info("EPS SFP furthest OB exclude " + exclude.size());
+                    if (log.shouldInfo())
+                        log.info("EPS SFP OBEP exclude " + exclude);
                     ctx.profileOrganizer().selectFastPeers(1, exclude, furthest, ipRestriction, ipSet);
                 }
                 if (!furthest.isEmpty()) {
-                    furthestHop = furthest.iterator().next();
+                    furthestHop = furthest.get(0);
                     exclude.add(furthestHop);
                     length--;
                 }
             }
         }
 
-
-        // Don't use ff peers for exploratory tunnels to lessen exposure to netDb searches and stores
-        // Hmm if they don't get explored they don't get a speed/capacity rating
-        // so they don't get used for client tunnels either.
-        // FloodfillNetworkDatabaseFacade fac = (FloodfillNetworkDatabaseFacade)ctx.netDb();
-        // exclude.addAll(fac.getFloodfillPeers());
-        HashSet<Hash> matches = new HashSet<Hash>(length);
-
+        ArrayList<Hash> rv = new ArrayList<Hash>(length + 3);
         if (length > 0) {
+            Set<Hash> matches = new ArraySet<Hash>(length);
             if (exploreHighCap) {
-                if (log.shouldLog(Log.INFO))
-                    log.info("EPS SHCP " + length + (isInbound ? " IB" : " OB") + " exclude " + exclude.size());
+                if (log.shouldInfo())
+                    log.info("EPS SHCP " + length + (isInbound ? " IB " : " OB ") + exclude);
                 ctx.profileOrganizer().selectHighCapacityPeers(length, exclude, matches, ipRestriction, ipSet);
             } else {
                 // As of 0.9.23, we include a max of 2 not failing peers,
@@ -192,14 +184,17 @@ class ExploratoryPeerSelector extends TunnelPeerSelector {
                 // Peer org credits existing items in matches
                 if (length > 2)
                     ctx.profileOrganizer().selectHighCapacityPeers(length - 2, exclude, matches);
-                if (log.shouldLog(Log.INFO))
-                    log.info("EPS SNFP " + length + (isInbound ? " IB" : " OB") + " exclude " + exclude.size());
+                // select will check both matches and exclude, no need to add matches to exclude here
+                if (log.shouldInfo())
+                    log.info("EPS SNFP " + length + (isInbound ? " IB " : " OB ") + exclude);
                 ctx.profileOrganizer().selectNotFailingPeers(length, exclude, matches, false, ipRestriction, ipSet);
             }
             matches.remove(ctx.routerHash());
+            rv.addAll(matches);
         }
+        if (log.shouldInfo())
+            log.info("EPS " + length + (isInbound ? " IB " : " OB ") + "final: " + exclude);
 
-        ArrayList<Hash> rv = new ArrayList<Hash>(matches);
         if (rv.size() > 1)
             orderPeers(rv, settings.getRandomKey());
         if (closestHop != null) {
@@ -226,7 +221,7 @@ class ExploratoryPeerSelector extends TunnelPeerSelector {
         else
             rv.add(ctx.routerHash());
         if (rv.size() > 1) {
-            if (!checkTunnel(isInbound, rv))
+            if (!checkTunnel(isInbound, true, rv))
                 rv = null;
         }
         return rv;
@@ -277,7 +272,7 @@ class ExploratoryPeerSelector extends TunnelPeerSelector {
         // high capacity peers, at least for a little bit.
         int failPct;
         // getEvents() will be 0 for first 10 minutes
-        if (ctx.router().getUptime() <= 11*60*1000) {
+        if (uptime <= 11*60*1000) {
             failPct = 100 - MIN_NONFAILING_PCT;
         } else {
             failPct = getExploratoryFailPercentage();
diff --git a/router/java/src/net/i2p/router/tunnel/pool/TunnelPeerSelector.java b/router/java/src/net/i2p/router/tunnel/pool/TunnelPeerSelector.java
index 06f7dc7ac7..8491270f42 100644
--- a/router/java/src/net/i2p/router/tunnel/pool/TunnelPeerSelector.java
+++ b/router/java/src/net/i2p/router/tunnel/pool/TunnelPeerSelector.java
@@ -28,7 +28,9 @@ import net.i2p.router.Router;
 import net.i2p.router.RouterContext;
 import net.i2p.router.TunnelPoolSettings;
 import net.i2p.router.networkdb.kademlia.FloodfillNetworkDatabaseFacade;
+import net.i2p.router.peermanager.PeerProfile;
 import net.i2p.router.transport.TransportUtil;
+import net.i2p.util.ArraySet;
 import net.i2p.util.Log;
 import net.i2p.util.SystemVersion;
 import net.i2p.util.VersionComparator;
@@ -151,7 +153,7 @@ public abstract class TunnelPeerSelector extends ConnectChecker {
             int more = length - rv.size();
             Set<Hash> exclude = getExclude(settings.isInbound(), settings.isExploratory());
             exclude.addAll(rv);
-            Set<Hash> matches = new HashSet<Hash>(more);
+            Set<Hash> matches = new ArraySet<Hash>(more);
             // don't bother with IP restrictions here
             ctx.profileOrganizer().selectFastPeers(more, exclude, matches);
             rv.addAll(matches);
@@ -184,9 +186,19 @@ public abstract class TunnelPeerSelector extends ConnectChecker {
     }
 
     /**
-     * Pick peers that we want to avoid
+     *  As of 0.9.58, this returns a set populated only by TunnelManager.selectPeersInTooManyTunnels(),
+     *  for passing to ProfileOrganizer.
+     *  The set will be populated via the contains() calls.
      */
-    public Set<Hash> getExclude(boolean isInbound, boolean isExploratory) {
+    protected Set<Hash> getExclude(boolean isInbound, boolean isExploratory) {
+        return new Excluder(isInbound, isExploratory);
+    }
+
+
+    /**
+     *  @since 0.9.58, previously getExclude()
+     */
+    private boolean shouldExclude(Hash h, boolean isInbound, boolean isExploratory) {
         // we may want to update this to skip 'hidden' or 'unreachable' peers, but that
         // isn't safe, since they may publish one set of routerInfo to us and another to
         // other peers.  the defaults for filterUnreachable has always been to return false,
@@ -204,46 +216,35 @@ public abstract class TunnelPeerSelector extends ConnectChecker {
         // (and even worse for anonymity?).
         //
 
-        Set<Hash> peers = new HashSet<Hash>(8);
-        peers.addAll(ctx.profileOrganizer().selectPeersRecentlyRejecting());
-        if (!ctx.getBooleanProperty("i2np.allowLocal"))
-            peers.addAll(ctx.tunnelManager().selectPeersInTooManyTunnels());
+        PeerProfile prof = ctx.profileOrganizer().getProfileNonblocking(h);
+        if (prof != null) {
+            long cutoff = ctx.clock().now() - (20*1000);
+            if (prof.getTunnelHistory().getLastRejectedBandwidth() > cutoff)
+                return true;
+        }
+
+        // the transport layer thinks is unreachable
+        if (ctx.commSystem().wasUnreachable(h))
+            return true;
+
+        RouterInfo info = ctx.netDb().lookupRouterInfoLocally(h);
+        if (info == null)
+            return true;
+
         if (filterUnreachable(isInbound, isExploratory)) {
-            // This is the only use for getPeersByCapability? And the whole set of datastructures in PeerManager?
-            Collection<Hash> caps = ctx.peerManager().getPeersByCapability(Router.CAPABILITY_UNREACHABLE);
-            if (caps != null)
-                peers.addAll(caps);
+            String caps = info.getCapabilities();
+            if (caps.indexOf(Router.CAPABILITY_UNREACHABLE) >= 0)
+                return true;
         }
-        Collection<Hash> local = ctx.profileOrganizer().selectPeersLocallyUnreachable();
-        if (local != null)
-            peers.addAll(local);
+
         if (filterSlow(isInbound, isExploratory)) {
             // NOTE: filterSlow always returns true
             String excl = getExcludeCaps(ctx);
-
-                FloodfillNetworkDatabaseFacade fac = (FloodfillNetworkDatabaseFacade)ctx.netDb();
-                List<RouterInfo> known = fac.getKnownRouterData();
-                if (known != null) {
-                    for (int i = 0; i < known.size(); i++) {
-                        RouterInfo peer = known.get(i);
-                        boolean shouldExclude = shouldExclude(peer, excl);
-                        if (shouldExclude) {
-                            peers.add(peer.getIdentity().calculateHash());
-                            continue;
-                        }
-                    }
-                }
-                /*
-                for (int i = 0; i < excludeCaps.length(); i++) {
-                    List matches = ctx.peerManager().getPeersByCapability(excludeCaps.charAt(i));
-                    if (log.shouldLog(Log.INFO))
-                        log.info("Filtering out " + matches.size() + " peers with capability " + excludeCaps.charAt(i));
-                    peers.addAll(matches);
-                }
-                 */
-
+            if (shouldExclude(info, excl))
+                return true;
         }
-        return peers;
+
+        return false;
     }
 
     /**
@@ -263,9 +264,9 @@ public abstract class TunnelPeerSelector extends ConnectChecker {
      *  the RI. Will not force RI lookups.
      *  Default true.
      *
-     *  @since 0.9.34
+     *  @since 0.9.34, protected since 0.9.58 for ClientPeerSelector
      */
-    private boolean allowAsOBEP(Hash h) {
+    protected boolean allowAsOBEP(Hash h) {
         RouterInfo ri = ctx.netDb().lookupRouterInfoLocally(h);
         if (ri == null)
             return true;
@@ -280,9 +281,9 @@ public abstract class TunnelPeerSelector extends ConnectChecker {
      *  the RI. Will not force RI lookups.
      *  Default true.
      *
-     *  @since 0.9.34
+     *  @since 0.9.34, protected since 0.9.58 for ClientPeerSelector
      */
-    private boolean allowAsIBGW(Hash h) {
+    protected boolean allowAsIBGW(Hash h) {
         RouterInfo ri = ctx.netDb().lookupRouterInfoLocally(h);
         if (ri == null)
             return true;
@@ -312,51 +313,22 @@ public abstract class TunnelPeerSelector extends ConnectChecker {
      *      Make sure that ClientPeerSelector and TunnelPeerSelector selectPeers() call this when needed.
      *  </ol>
      *
-     *  Don't call this unless you need to.
-     *  See ClientPeerSelector and TunnelPeerSelector selectPeers().
+     *  As of 0.9.58, this a set with only toAdd, for use in ProfileOrganizer.
+     *  The set will be populated via the contains() calls.
      *
      *  @param isInbound
-     *  @return null if none
+     *  @return non-null
      *  @since 0.9.17
      */
-    protected Set<Hash> getClosestHopExclude(boolean isInbound) {
-        RouterInfo ri = ctx.router().getRouterInfo();
-        if (ri == null)
-            return null;
-
-        // we can skip this check now, uncomment if we have some new sigtype
-        //SigType type = ri.getIdentity().getSigType();
-        //if (type == SigType.DSA_SHA1)
-        //    return null;
-
-        int ourMask = isInbound ? getInboundMask(ri) : getOutboundMask(ri);
-        Set<Hash> connected = ctx.commSystem().getEstablished();
-        Set<Hash> rv = new HashSet<Hash>(256);
-        FloodfillNetworkDatabaseFacade fac = (FloodfillNetworkDatabaseFacade)ctx.netDb();
-        List<RouterInfo> known = fac.getKnownRouterData();
-        if (known != null) {
-            for (int i = 0; i < known.size(); i++) {
-                RouterInfo peer = known.get(i);
-                Hash h = peer.getIdentity().calculateHash();
-
-                // Uncomment if stricter than in shouldExclude() below
-                //String v = peer.getVersion();
-                //if (VersionComparator.comp(v, "0.9.16") < 0) {
-                //    rv.add(h);
-                //    continue;
-                //}
-
-                if (connected.contains(h))
-                    continue;
-                boolean canConnect = isInbound ? canConnect(peer, ourMask) : canConnect(ourMask, peer);
-                if (!canConnect)
-                    rv.add(h);
-            }
-        }
-        return rv;
+    protected Set<Hash> getClosestHopExclude(boolean isInbound, Set<Hash> toAdd) {
+        return new ClosestHopExcluder(isInbound, toAdd);
     }
 
-    /** warning, this is also called by ProfileOrganizer.isSelectable() */
+    /**
+     *  Should the peer be excluded based on its published caps?
+     *
+     *  Warning, this is also called by ProfileOrganizer.isSelectable()
+     */
     public static boolean shouldExclude(RouterContext ctx, RouterInfo peer) {
         return shouldExclude(peer, getExcludeCaps(ctx));
     }
@@ -371,7 +343,11 @@ public abstract class TunnelPeerSelector extends ConnectChecker {
     /** NTCP2 */
     private static final String MIN_VERSION = "0.9.36";
 
-    /** warning, this is also called by ProfileOrganizer.isSelectable() */
+    /**
+     *  Should the peer be excluded based on its published caps?
+     *
+     *  Warning, this is also called by ProfileOrganizer.isSelectable()
+     */
     private static boolean shouldExclude(RouterInfo peer, String excl) {
         String cap = peer.getCapabilities();
         for (int j = 0; j < excl.length(); j++) {
@@ -538,9 +514,12 @@ public abstract class TunnelPeerSelector extends ConnectChecker {
      *  @return ok
      *  @since 0.9.34
      */
-    protected boolean checkTunnel(boolean isInbound, List<Hash> tunnel) {
+    protected boolean checkTunnel(boolean isInbound, boolean isExploratory, List<Hash> tunnel) {
         if (!checkTunnel(tunnel))
             return false;
+        // client OBEP/IBGW checks now in CPS
+        if (!isExploratory)
+            return true;
         if (isInbound) {
             Hash h = tunnel.get(tunnel.size() - 1);
             if (!allowAsIBGW(h)) {
@@ -597,4 +576,119 @@ public abstract class TunnelPeerSelector extends ConnectChecker {
         }
         return rv;
     }
+
+    /**
+     *  A Set of Hashes that automatically adds to the
+     *  Set in the contains() check.
+     *
+     *  So we don't need to generate the exclude set up front.
+     *
+     *  @since 0.9.58
+     */
+    protected class Excluder extends ExcluderBase {
+        private final boolean _isIn, _isExpl;
+
+        /**
+         *  Automatically adds selectPeersInTooManyTunnels(), unless i2np.allowLocal.
+         */
+        public Excluder(boolean isInbound, boolean isExploratory) {
+            super(ctx.getBooleanProperty("i2np.allowLocal") ? new HashSet<Hash>()
+                                                            : ctx.tunnelManager().selectPeersInTooManyTunnels());
+            _isIn = isInbound;
+            _isExpl = isExploratory;
+        }
+
+        /**
+         *  Does not add selectPeersInTooManyTunnels().
+         *  Makes a copy of toAdd
+         *
+         *  @param toAdd initial contents, copied
+         */
+        public Excluder(boolean isInbound, boolean isExploratory, Set<Hash> toAdd) {
+            super(new HashSet<Hash>(toAdd));
+            _isIn = isInbound;
+            _isExpl = isExploratory;
+        }
+
+        /**
+         *  Overridden to automatically check our exclusion criteria
+         *  and add the Hash to the set if the criteria are met.
+         *
+         *  @param o a Hash
+         *  @return true if peer should be excluded
+         */
+        @Override
+        public boolean contains(Object o) {
+            if (s.contains(o))
+                return true;
+            Hash h = (Hash) o;
+            if (shouldExclude(h, _isIn, _isExpl)) {
+                s.add(h);
+                //if (log.shouldDebug())
+                //    log.debug("TPS exclude " + h.toBase64());
+                return true;
+            }
+            return false;
+        }
+    }
+
+    /**
+     *  Only for hidden mode and other tough situations.
+     *  See checkClosestHop boolean.
+     *  Not for hidden inbound; use SANFP instead.
+     *
+     *  @since 0.9.58
+     */
+    private class ClosestHopExcluder extends ExcluderBase {
+        private final boolean isIn;
+        private final int ourMask;
+
+        /**
+         *  Automatically check if peer can connect to us (for inbound)
+         *  or we can connect to it (for outbound)
+         *  and add the Hash to the set if not.
+         *
+         *  @param set not copied, contents will be modified by all methods
+         */
+        public ClosestHopExcluder(boolean isInbound, Set<Hash> set) {
+            super(set);
+            isIn = isInbound;
+            RouterInfo ri = ctx.router().getRouterInfo();
+            if (ri != null)
+                ourMask = isInbound ? getInboundMask(ri) : getOutboundMask(ri);
+            else
+                ourMask = 0xff;
+        }
+
+        /**
+         *  Automatically check if peer can connect to us (for inbound)
+         *  or we can connect to it (for outbound)
+         *  and add the Hash to the set if not.
+         *
+         *  @param o a Hash
+         *  @return true if peer should be excluded
+         */
+        public boolean contains(Object o) {
+            if (s.contains(o))
+                return true;
+            Hash h = (Hash) o;
+            if (ctx.commSystem().isEstablished(h))
+                return false;
+            boolean canConnect;
+            RouterInfo peer = ctx.netDb().lookupRouterInfoLocally(h);
+            if (peer == null) {
+                canConnect = false;
+            } else if (isIn) {
+                canConnect = canConnect(peer, ourMask);
+            } else {
+                canConnect = canConnect(ourMask, peer);
+            }
+            if (!canConnect) {
+                s.add(h);
+                //if (log.shouldDebug())
+                //    log.debug("TPS closest exclude "  h.toBase64());
+            }
+            return !canConnect;
+        }
+    }
 }
-- 
GitLab