* LeaseSet:

- Sort the leases by expiration date in TunnelPool.locked_buildNewLeaseSet()
        to make later LeaseSet comparisons reliable. This cleans up the code too.
      - Fix broken old vs. new LeaseSet comparison
        in ClientConnectionRunner.requestLeaseSet(),
        so that we only sign and publish a new LeaseSet when it's really new.
        Should reduce outbound overhead both in LeaseSet publishing and LeaseSet bundling,
        and floodfill router load, since locked_buildNewLeaseSet() generates
        the same LeaseSet as before quite frequently, often just seconds apart.

history.txt

@@ -1,3 +1,14 @@
+2008-06-06 zzz
+    * LeaseSet:
+      - Sort the leases by expiration date in TunnelPool.locked_buildNewLeaseSet()
+        to make later LeaseSet comparisons reliable. This cleans up the code too.
+      - Fix broken old vs. new LeaseSet comparison
+        in ClientConnectionRunner.requestLeaseSet(),
+        so that we only sign and publish a new LeaseSet when it's really new.
+        Should reduce outbound overhead both in LeaseSet publishing and LeaseSet bundling,
+        and floodfill router load, since locked_buildNewLeaseSet() generates
+        the same LeaseSet as before quite frequently, often just seconds apart.
+
 2008-06-05 zzz
     * LeaseSet - code cleanup:
       - Add exception to enforce max # of leases = 6, should be plenty

router/java/src/net/i2p/router/RouterVersion.java

@@ -17,7 +17,7 @@ import net.i2p.CoreVersion;
 public class RouterVersion {
     public final static String ID = "$Revision: 1.548 $ $Date: 2008-02-10 15:00:00 $";
     public final static String VERSION = "0.6.1.33";
-    public final static long BUILD = 2002;
+    public final static long BUILD = 2003;
     public static void main(String args[]) {
         System.out.println("I2P Router version: " + VERSION + "-" + BUILD);
         System.out.println("Router ID: " + RouterVersion.ID);

router/java/src/net/i2p/router/client/ClientConnectionRunner.java

@@ -375,13 +375,32 @@ public class ClientConnectionRunner {
                 _context.jobQueue().addJob(onFailedJob);
             return;
         }
-        if ( (_currentLeaseSet != null) && (_currentLeaseSet.equals(set)) ) {
-            if (_log.shouldLog(Log.DEBUG))
-                _log.debug("Requested leaseSet hasn't changed");
+        // We can't use LeaseSet.equals() here because the dest, keys, and sig on
+        // the new LeaseSet are null. So we compare leases one by one.
+        // In addition, the client rewrites the expiration time of all the leases to
+        // the earliest one, so we can't use Lease.equals() or Lease.getEndDate().
+        // So compare by tunnel ID, and then by gateway.
+        // (on the remote possibility that two gateways are using the same ID).
+        // TunnelPool.locked_buildNewLeaseSet() ensures that leases are sorted,
+        //  so the comparison will always work.
+        int leases = set.getLeaseCount();
+        if (_currentLeaseSet != null && _currentLeaseSet.getLeaseCount() == leases) {
+            for (int i = 0; i < leases; i++) {
+                if (! _currentLeaseSet.getLease(i).getTunnelId().equals(set.getLease(i).getTunnelId()))
+                    break;
+                if (! _currentLeaseSet.getLease(i).getGateway().equals(set.getLease(i).getGateway()))
+                    break;
+                if (i == leases - 1) {
+                    if (_log.shouldLog(Log.INFO))
+                        _log.info("Requested leaseSet hasn't changed");
                     if (onCreateJob != null)
                         _context.jobQueue().addJob(onCreateJob);
                     return; // no change
                 }
+            }
+        }
+        if (_log.shouldLog(Log.INFO))
+            _log.info("Current leaseSet " + _currentLeaseSet + "\nNew leaseSet " + set);
         LeaseRequestState state = null;
         synchronized (this) {
             state = _leaseRequest;

router/java/src/net/i2p/router/tunnel/pool/TunnelPool.java

@@ -3,9 +3,12 @@ package net.i2p.router.tunnel.pool;
 import java.util.Arrays;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.Date;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Properties;
+import java.util.TreeSet;
 
 import net.i2p.data.Hash;
 import net.i2p.data.Lease;
@@ -429,6 +432,19 @@ public class TunnelPool {
         return false;
     }
     
+    /**
+     * Always build a LeaseSet with Leases in sorted order,
+     * so that LeaseSet.equals() and lease-by-lease equals() always work.
+     * The sort method is arbitrary, as far as the equals() tests are concerned,
+     * but we use latest expiration first, since we need to sort them by that anyway.
+     *
+     */
+    class LeaseComparator implements Comparator {
+         public int compare(Object l, Object r) {
+             return ((Lease)r).getEndDate().compareTo(((Lease)l).getEndDate());
+        }
+    }
+
     /**
      * Build a leaseSet with the required tunnels that aren't about to expire
      *
@@ -436,9 +452,17 @@ public class TunnelPool {
     private LeaseSet locked_buildNewLeaseSet() {
         if (!_alive)
             return null;
+
+        int wanted = _settings.getQuantity();
+        if (_tunnels.size() < wanted) {
+            if (_log.shouldLog(Log.WARN))
+                _log.warn(toString() + ": Not enough tunnels (" + _tunnels.size() + ", wanted " + wanted + ")");
+            return null;
+        }
+
         long expireAfter = _context.clock().now(); // + _settings.getRebuildPeriod();
         
-        List leases = new ArrayList(_tunnels.size());
+        TreeSet leases = new TreeSet(new LeaseComparator());
         for (int i = 0; i < _tunnels.size(); i++) {
             TunnelInfo tunnel = (TunnelInfo)_tunnels.get(i);
             if (tunnel.getExpiration() <= expireAfter)
@@ -457,31 +481,16 @@ public class TunnelPool {
             leases.add(lease);
         }
         
-        int wanted = _settings.getQuantity();
-        
         if (leases.size() < wanted) {
             if (_log.shouldLog(Log.WARN))
                 _log.warn(toString() + ": Not enough leases (" + leases.size() + ", wanted " + wanted + ")");
             return null;
-        } else {
-            // linear search to trim down the leaseSet, removing the ones that 
-            // will expire the earliest.  cheaper than a tree for this size
-            while (leases.size() > wanted) {
-                int earliestIndex = -1;
-                long earliestExpiration = -1;
-                for (int i = 0; i < leases.size(); i++) {
-                    Lease cur = (Lease) leases.get(i);
-                    if ( (earliestExpiration < 0) || (cur.getEndDate().getTime() < earliestExpiration) ) {
-                        earliestIndex = i;
-                        earliestExpiration = cur.getEndDate().getTime();
-                    }
-                }
-                leases.remove(earliestIndex);
-            }
         }
+
         LeaseSet ls = new LeaseSet();
-        for (int i = 0; i < leases.size(); i++)
-             ls.addLease((Lease) leases.get(i));
+        Iterator iter = leases.iterator();
+        for (int i = 0; i < wanted; i++)
+             ls.addLease((Lease) iter.next());
         if (_log.shouldLog(Log.INFO))
             _log.info(toString() + ": built new leaseSet: " + ls);
         return ls;