Measuring topology performance. Fixing a defect that caused us to run w/o accumu…

…lator when config file was not present.

Change-Id: I5ad538b8a441cd6ff2aefea49a0def10b8e0f4d5

@@ -22,17 +22,23 @@ import org.onlab.onos.net.Provided;
  */
  */
 public interface Topology extends Provided {
 public interface Topology extends Provided {
 
 
-    // FIXME: Following is not true right now. It is actually System.nanoTime(),
-    // which has no relation to epoch time, wall clock, etc.
     /**
     /**
-     * Returns the time, specified in milliseconds since start of epoch,
-     * when the topology became active and made available.
+     * Returns the time, specified in system nanos of when the topology
+     * became available.
      *
      *
-     * @return time in milliseconds since start of epoch
+     * @return time in system nanos
      */
      */
     long time();
     long time();
 
 
     /**
     /**
+     * Returns the time, specified in system nanos of how long the topology
+     * took to compute.
+     *
+     * @return elapsed time in system nanos
+     */
+    long computeCost();
+
+    /**
      * Returns the number of SCCs (strongly connected components) in the
      * Returns the number of SCCs (strongly connected components) in the
      * topology.
      * topology.
      *
      *

@@ -66,9 +66,9 @@ public class DefaultTopologyProvider extends AbstractProvider
         implements TopologyProvider {
         implements TopologyProvider {
 
 
     private static final int MAX_THREADS = 8;
     private static final int MAX_THREADS = 8;
-    private static final int DEFAULT_MAX_EVENTS = 200;
-    private static final int DEFAULT_MAX_BATCH_MS = 60;
-    private static final int DEFAULT_MAX_IDLE_MS = 30;
+    private static final int DEFAULT_MAX_EVENTS = 1000;
+    private static final int DEFAULT_MAX_IDLE_MS = 10;
+    private static final int DEFAULT_MAX_BATCH_MS = 50;
 
 
     // FIXME: Replace with a system-wide timer instance;
     // FIXME: Replace with a system-wide timer instance;
     // TODO: Convert to use HashedWheelTimer or produce a variant of that; then decide which we want to adopt
     // TODO: Convert to use HashedWheelTimer or produce a variant of that; then decide which we want to adopt
@@ -116,15 +116,15 @@ public class DefaultTopologyProvider extends AbstractProvider
     @Activate
     @Activate
     public synchronized void activate(ComponentContext context) {
     public synchronized void activate(ComponentContext context) {
         executor = newFixedThreadPool(MAX_THREADS, namedThreads("topo-build-%d"));
         executor = newFixedThreadPool(MAX_THREADS, namedThreads("topo-build-%d"));
+        accumulator = new TopologyChangeAccumulator();
+        logConfig("Configured");
+
         modified(context);
         modified(context);
 
 
         providerService = providerRegistry.register(this);
         providerService = providerRegistry.register(this);
         deviceService.addListener(deviceListener);
         deviceService.addListener(deviceListener);
         linkService.addListener(linkListener);
         linkService.addListener(linkListener);
 
 
-        log.info("Configured with maxEvents = {}; maxBatchMs = {}; maxIdleMs = {}",
-                 maxEvents, maxBatchMs, maxIdleMs);
-
         isStarted = true;
         isStarted = true;
         triggerRecompute();
         triggerRecompute();
         log.info("Started");
         log.info("Started");
@@ -149,6 +149,7 @@ public class DefaultTopologyProvider extends AbstractProvider
     public void modified(ComponentContext context) {
     public void modified(ComponentContext context) {
         if (context == null) {
         if (context == null) {
             accumulator = new TopologyChangeAccumulator();
             accumulator = new TopologyChangeAccumulator();
+            logConfig("Reconfigured");
             return;
             return;
         }
         }
 
 
@@ -163,6 +164,7 @@ public class DefaultTopologyProvider extends AbstractProvider
 
 
             s = (String) properties.get("maxIdleMs");
             s = (String) properties.get("maxIdleMs");
             newMaxIdleMs = isNullOrEmpty(s) ? maxIdleMs : Integer.parseInt(s);
             newMaxIdleMs = isNullOrEmpty(s) ? maxIdleMs : Integer.parseInt(s);
+
         } catch (Exception e) {
         } catch (Exception e) {
             newMaxEvents = DEFAULT_MAX_EVENTS;
             newMaxEvents = DEFAULT_MAX_EVENTS;
             newMaxBatchMs = DEFAULT_MAX_BATCH_MS;
             newMaxBatchMs = DEFAULT_MAX_BATCH_MS;
@@ -174,9 +176,13 @@ public class DefaultTopologyProvider extends AbstractProvider
             maxBatchMs = newMaxBatchMs;
             maxBatchMs = newMaxBatchMs;
             maxIdleMs = newMaxIdleMs;
             maxIdleMs = newMaxIdleMs;
             accumulator = maxEvents > 1 ? new TopologyChangeAccumulator() : null;
             accumulator = maxEvents > 1 ? new TopologyChangeAccumulator() : null;
-            log.info("Reconfigured with maxEvents = {}; maxBatchMs = {}; maxIdleMs = {}",
-                     maxEvents, maxBatchMs, maxIdleMs);
+            logConfig("Reconfigured");
+        }
     }
     }
+
+    private void logConfig(String prefix) {
+        log.info("{} with maxEvents = {}; maxBatchMs = {}; maxIdleMs = {}; accumulator={}",
+                 prefix, maxEvents, maxBatchMs, maxIdleMs, accumulator != null);
     }
     }
 
 
 
 

@@ -65,6 +65,7 @@ public class DefaultTopology extends AbstractModel implements Topology {
             new TarjanGraphSearch<>();
             new TarjanGraphSearch<>();
 
 
     private final long time;
     private final long time;
+    private final long computeCost;
     private final TopologyGraph graph;
     private final TopologyGraph graph;
 
 
     private final SCCResult<TopologyVertex, TopologyEdge> clusterResults;
     private final SCCResult<TopologyVertex, TopologyEdge> clusterResults;
@@ -104,6 +105,7 @@ public class DefaultTopology extends AbstractModel implements Topology {
 
 
         this.broadcastSets = buildBroadcastSets();
         this.broadcastSets = buildBroadcastSets();
         this.infrastructurePoints = findInfrastructurePoints();
         this.infrastructurePoints = findInfrastructurePoints();
+        this.computeCost = Math.max(0, System.nanoTime() - time);
     }
     }
 
 
     @Override
     @Override
@@ -112,6 +114,11 @@ public class DefaultTopology extends AbstractModel implements Topology {
     }
     }
 
 
     @Override
     @Override
+    public long computeCost() {
+        return computeCost;
+    }
+
+    @Override
     public int clusterCount() {
     public int clusterCount() {
         return clusters.size();
         return clusters.size();
     }
     }
@@ -453,6 +460,7 @@ public class DefaultTopology extends AbstractModel implements Topology {
     public String toString() {
     public String toString() {
         return toStringHelper(this)
         return toStringHelper(this)
                 .add("time", time)
                 .add("time", time)
+                .add("computeCost", computeCost)
                 .add("clusters", clusterCount())
                 .add("clusters", clusterCount())
                 .add("devices", deviceCount())
                 .add("devices", deviceCount())
                 .add("links", linkCount())
                 .add("links", linkCount())

@@ -65,6 +65,7 @@ public class DefaultTopology extends AbstractModel implements Topology {
             new TarjanGraphSearch<>();
             new TarjanGraphSearch<>();
 
 
     private final long time;
     private final long time;
+    private final long computeCost;
     private final TopologyGraph graph;
     private final TopologyGraph graph;
 
 
     private final SCCResult<TopologyVertex, TopologyEdge> clusterResults;
     private final SCCResult<TopologyVertex, TopologyEdge> clusterResults;
@@ -104,6 +105,7 @@ public class DefaultTopology extends AbstractModel implements Topology {
 
 
         this.broadcastSets = buildBroadcastSets();
         this.broadcastSets = buildBroadcastSets();
         this.infrastructurePoints = findInfrastructurePoints();
         this.infrastructurePoints = findInfrastructurePoints();
+        this.computeCost = Math.max(0, System.nanoTime() - time);
     }
     }
 
 
     @Override
     @Override
@@ -112,6 +114,11 @@ public class DefaultTopology extends AbstractModel implements Topology {
     }
     }
 
 
     @Override
     @Override
+    public long computeCost() {
+        return computeCost;
+    }
+
+    @Override
     public int clusterCount() {
     public int clusterCount() {
         return clusters.size();
         return clusters.size();
     }
     }
@@ -453,6 +460,7 @@ public class DefaultTopology extends AbstractModel implements Topology {
     public String toString() {
     public String toString() {
         return toStringHelper(this)
         return toStringHelper(this)
                 .add("time", time)
                 .add("time", time)
+                .add("computeCost", computeCost)
                 .add("clusters", clusterCount())
                 .add("clusters", clusterCount())
                 .add("devices", deviceCount())
                 .add("devices", deviceCount())
                 .add("links", linkCount())
                 .add("links", linkCount())