Optimize AbstractBestPathSelector

There are inefficiencies in how we compare local preference, let's
refactor the code to have clear comparisons without unnecessary
boxing.

Also remove implicit boxing of AS numbers, as we are already getting
them as primitive longs.

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

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/api/BestPathState.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/api/BestPathState.java
index 82f473d5d403838c46c7a58cf612d95644ada893..455ced5ad9dd911bba1916fb256a9efa4cf5cc49 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/api/BestPathState.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/api/BestPathState.java
@@ -17,8 +17,7 @@ public interface BestPathState {
     @Nullable
     Long getLocalPref();
 
-    @Nullable
-    Long getMultiExitDisc();
+    long getMultiExitDisc();
 
     @Nullable
     BgpOrigin getOrigin();

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/BestPathStateImpl.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/BestPathStateImpl.java
index b61f97fbe87bd40c2a483ad0d3f155d3a0a9ad3f..8c3199c37b2301faaef9f59db977cf9b6faeccc0 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/BestPathStateImpl.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/BestPathStateImpl.java
@@ -30,7 +30,7 @@ public final class BestPathStateImpl implements BestPathState {
     private long peerAs = 0L;
     private int asPathLength = 0;
     private Long localPref;
-    private Long multiExitDisc;
+    private long multiExitDisc;
     private BgpOrigin origin;
     private boolean depreferenced;
     private boolean resolved;
@@ -67,7 +67,12 @@ public final class BestPathStateImpl implements BestPathState {
         localPref = attrLocalPref != null ? attrLocalPref.getPref() : null;
 
         final MultiExitDisc attrMed = attributes.getMultiExitDisc();
-        multiExitDisc = attrMed != null ? attrMed.getMed() : null;
+        if (attrMed != null) {
+            final Long med = attrMed.getMed();
+            multiExitDisc = med == null ? 0L : med;
+        } else {
+            multiExitDisc = 0L;
+        }
 
         final Origin attrOrigin = attributes.getOrigin();
         origin = attrOrigin != null ? attrOrigin.getValue() : null;
@@ -94,7 +99,7 @@ public final class BestPathStateImpl implements BestPathState {
     }
 
     @Override
-    public Long getMultiExitDisc() {
+    public long getMultiExitDisc() {
         resolveValues();
         return this.multiExitDisc;
     }
@@ -149,7 +154,7 @@ public final class BestPathStateImpl implements BestPathState {
         int result = 1;
         result = prime * result + this.attributes.hashCode();
         result = prime * result + (this.localPref == null ? 0 : this.localPref.hashCode());
-        result = prime * result + (this.multiExitDisc == null ? 0 : this.multiExitDisc.hashCode());
+        result = prime * result + Long.hashCode(multiExitDisc);
         result = prime * result + (this.origin == null ? 0 : this.origin.hashCode());
         result = prime * result + Boolean.hashCode(depreferenced);
         return result;
@@ -174,11 +179,7 @@ public final class BestPathStateImpl implements BestPathState {
         } else if (!this.localPref.equals(other.localPref)) {
             return false;
         }
-        if (this.multiExitDisc == null) {
-            if (other.multiExitDisc != null) {
-                return false;
-            }
-        } else if (!this.multiExitDisc.equals(other.multiExitDisc)) {
+        if (this.multiExitDisc != other.multiExitDisc) {
             return false;
         }
         if (this.origin != other.origin) {

diff --git a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractBestPathSelector.java b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractBestPathSelector.java
index 45d2636a01f5189ba6d8d9db72de7a993ebc7c53..0e3b7a10e475532192fc60482de86dc92f13f346 100644 (file)
--- a/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractBestPathSelector.java
+++ b/bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/spi/AbstractBestPathSelector.java
@@ -61,18 +61,21 @@ public class AbstractBestPathSelector {
          * FIXME: for eBGP cases (when the LOCAL_PREF is missing), we should assign a policy-based preference
          *        before we ever get here.
          */
-        if (this.bestState.getLocalPref() == null && state.getLocalPref() != null) {
-            return true;
-        }
-        if (this.bestState.getLocalPref() != null && state.getLocalPref() == null) {
-            return false;
-        }
-        if (state.getLocalPref() != null && state.getLocalPref() > this.bestState.getLocalPref()) {
+        final Long bestLocal = this.bestState.getLocalPref();
+        final Long stateLocal = state.getLocalPref();
+        if (stateLocal != null) {
+            if (bestLocal == null) {
+                return true;
+            }
+
+            final Boolean bool = firstLower(stateLocal, bestLocal);
+            if (bool != null) {
+                return bool;
+            }
+        } else if (bestLocal != null) {
             return false;
         }
-        if (state.getLocalPref() != null && state.getLocalPref() < this.bestState.getLocalPref()) {
-            return true;
-        }
+
         // 3. prefer learned path
         // - we assume that all paths are learned
 
@@ -91,8 +94,8 @@ public class AbstractBestPathSelector {
             return no.ordinal() > bo.ordinal();
         }
         // FIXME: we should be able to cache the best AS
-        final Long bestAs = this.bestState.getPeerAs();
-        final Long newAs = state.getPeerAs();
+        final long bestAs = this.bestState.getPeerAs();
+        final long newAs = state.getPeerAs();
 
         /*
          * Checks 6 and 7 are mutually-exclusive, as MEDs are comparable
@@ -101,12 +104,11 @@ public class AbstractBestPathSelector {
          * relationship.
          *
          */
-        if (bestAs.equals(newAs)) {
+        if (bestAs == newAs) {
             // 6. prefer the path with the lowest multi-exit discriminator (MED)
-            if (this.bestState.getMultiExitDisc() != null || state.getMultiExitDisc() != null) {
-                final Long bmed = this.bestState.getMultiExitDisc();
-                final Long nmed = state.getMultiExitDisc();
-                return nmed >= bmed;
+            final Boolean cmp = firstLower(this.bestState.getMultiExitDisc(), state.getMultiExitDisc());
+            if (cmp != null) {
+                return cmp;
             }
         } else {
             /*
@@ -152,4 +154,9 @@ public class AbstractBestPathSelector {
          */
         return true;
     }
+
+    private static Boolean firstLower(final long first, final long second) {
+        return first < second ? Boolean.TRUE : first == second ? null : Boolean.FALSE;
+
+    }
 }