Fix isFirstBestPath brain damage

Using a List.indexOf() followed by equality to 0 is ... inefficient
to say the least.

Kill isFirstBestPath() and use a simple check against the first path,
if the list is non-empty.

Also optimize list allocation and remove useless copying while we're
at it.

Change-Id: Ia1a48736ff363ab15776b9f188207720f23169e2
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java
index 2f9e1f9f4d3b090cd09a132c204b88653c97ceb7..1311ca50ccc3a79d8e973b218e4d1a7b15c68f82 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java
@@ -7,6 +7,7 @@
  */
 package org.opendaylight.protocol.bgp.mode.impl.add;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.common.primitives.UnsignedInteger;
 import java.util.ArrayList;
@@ -123,10 +124,12 @@ public abstract class AddPathAbstractRouteEntry<C extends Routes & DataObject &
         if (this.newBestPathToBeAdvertised == null || this.newBestPathToBeAdvertised.isEmpty()) {
             return Collections.emptyList();
         }
-        final List<AdvertizedRoute<C, S, R, I>> advertized = new ArrayList<>();
+        final List<AdvertizedRoute<C, S, R, I>> advertized = new ArrayList<>(newBestPathToBeAdvertised.size());
+        final AddPathBestPath firstBestPath = this.bestPath.isEmpty() ? null : this.bestPath.get(0);
         for (final AddPathBestPath path : this.newBestPathToBeAdvertised) {
-            final boolean isFirstBestPath = isFirstBestPath(this.bestPath.indexOf(path));
             final R routeAddPath = createRoute(ribSupport, routeKey, path.getPathId(), path);
+            // FIXME: can we use identity check here?
+            final boolean isFirstBestPath = firstBestPath != null && firstBestPath.equals(path);
             final AdvertizedRoute<C, S, R, I> adv = new AdvertizedRoute<>(ribSupport, isFirstBestPath,
                     routeAddPath, path.getAttributes(), path.getPeerId());
             advertized.add(adv);
@@ -184,19 +187,17 @@ public abstract class AddPathAbstractRouteEntry<C extends Routes & DataObject &
         return selector.result();
     }
 
-    private static boolean isFirstBestPath(final int bestPathPosition) {
-        return bestPathPosition == 0;
-    }
-
-    protected boolean isBestPathNew(final List<AddPathBestPath> newBestPathList) {
+    protected boolean isBestPathNew(final ImmutableList<AddPathBestPath> newBestPathList) {
         this.isNonAddPathBestPathNew = !isNonAddPathBestPathTheSame(newBestPathList);
         filterRemovedPaths(newBestPathList);
         if (this.bestPathRemoved != null && !this.bestPathRemoved.isEmpty()
                 || newBestPathList != null
                 && !newBestPathList.equals(this.bestPath)) {
-            this.newBestPathToBeAdvertised = new ArrayList<>(newBestPathList);
             if (this.bestPath != null) {
+                this.newBestPathToBeAdvertised = new ArrayList<>(newBestPathList);
                 this.newBestPathToBeAdvertised.removeAll(this.bestPath);
+            } else {
+                this.newBestPathToBeAdvertised = newBestPathList;
             }
             this.bestPath = newBestPathList;
             LOG.trace("Actual Best {}, removed best {}", this.bestPath, this.bestPathRemoved);