Refactor StaleBestPathRoute 12/78512/20
authorRobert Varga <robert.varga@pantheon.tech>
Fri, 14 Dec 2018 17:07:38 +0000 (18:07 +0100)
committerRobert Varga <nite@hq.sk>
Fri, 14 Dec 2018 19:40:39 +0000 (19:40 +0000)
This removes the dependency on RIBSupport by forcing users to provide
proper lists. Since there are only two users, we can create specialized
instances, co-located with them, so that we do not have to cross through
SPI/implementation boundaries.

This allows saves quite a bit of memory in non-addpath case and also
improves code AddPathAbstractRouteEntry isolation, as we no longer
leak List<Long> all over the place.

Change-Id: I48b91d15df738095812736dfdeaa71d88b30598b
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/add/AddPathAbstractRouteEntry.java
bgp/path-selection-mode/src/main/java/org/opendaylight/protocol/bgp/mode/impl/base/BaseRouteEntry.java
bgp/rib-spi/src/main/java/org/opendaylight/protocol/bgp/rib/spi/entry/StaleBestPathRoute.java

index c73d63c23580c0221acdcd1327ca2a00661e1d39..54ec51dd3dd07733ae9cee081ca0d64858406034 100644 (file)
@@ -48,9 +48,45 @@ import org.slf4j.LoggerFactory;
  */
 @NotThreadSafe
 public abstract class AddPathAbstractRouteEntry<C extends Routes & DataObject & ChoiceIn<Tables>,
-        S extends ChildOf<? super C>,
-        R extends Route & ChildOf<? super S> & Identifiable<I>, I extends Identifier<R>>
+        S extends ChildOf<? super C>, R extends Route & ChildOf<? super S> & Identifiable<I>, I extends Identifier<R>>
         implements RouteEntry<C, S, R, I> {
+    private static final class Stale<C extends Routes & DataObject & ChoiceIn<Tables>,
+            S extends ChildOf<? super C>, R extends Route & ChildOf<? super S> & Identifiable<I>,
+            I extends Identifier<R>> extends StaleBestPathRoute<C, S, R, I> {
+        private final List<I> addPathRouteKeyIdentifier;
+        private final List<I> staleRouteKeyIdentifier;
+        private final boolean isNonAddPathBestPathNew;
+
+        Stale(final RIBSupport<C, S, R, I> ribSupport, final String routeKey, final List<PathId> staleRoutesPathIds,
+            final List<PathId> withdrawalRoutePathIds, final boolean isNonAddPathBestPathNew) {
+            super(ribSupport.createRouteListKey(routeKey));
+            this.isNonAddPathBestPathNew = isNonAddPathBestPathNew;
+
+            this.staleRouteKeyIdentifier = staleRoutesPathIds.stream()
+                    .map(pathId -> ribSupport.createRouteListKey(pathId, routeKey)).collect(Collectors.toList());
+            if (withdrawalRoutePathIds != null) {
+                this.addPathRouteKeyIdentifier = withdrawalRoutePathIds.stream()
+                        .map(pathId -> ribSupport.createRouteListKey(pathId, routeKey)).collect(Collectors.toList());
+            } else {
+                this.addPathRouteKeyIdentifier = Collections.emptyList();
+            }
+        }
+
+        @Override
+        public List<I> getStaleRouteKeyIdentifiers() {
+            return this.staleRouteKeyIdentifier;
+        }
+
+        @Override
+        public List<I> getAddPathRouteKeyIdentifiers() {
+            return addPathRouteKeyIdentifier;
+        }
+
+        @Override
+        public boolean isNonAddPathBestPathNew() {
+            return isNonAddPathBestPathNew;
+        }
+    }
 
     private static final Logger LOG = LoggerFactory.getLogger(AddPathAbstractRouteEntry.class);
     private static final Long[] EMPTY_PATHS_ID = new Long[0];
@@ -129,8 +165,7 @@ public abstract class AddPathAbstractRouteEntry<C extends Routes & DataObject &
         }
 
         return stalePaths.isEmpty() && removedPaths.isEmpty() ? Optional.empty()
-                : Optional.of(new StaleBestPathRoute<>(ribSupport, routeKey, stalePaths,
-                        removedPaths, this.isNonAddPathBestPathNew));
+                : Optional.of(new Stale<>(ribSupport, routeKey, stalePaths, removedPaths, isNonAddPathBestPathNew));
     }
 
     @Override
index 3a2f531bb0b5c4bf60e0be809aa69663fc130bbf..91ed3377b1fb2e0dcfd68462522ca9e579bd701b 100644 (file)
@@ -7,8 +7,6 @@
  */
 package org.opendaylight.protocol.bgp.mode.impl.base;
 
-import static org.opendaylight.protocol.bgp.parser.spi.PathIdUtil.NON_PATH_ID;
-
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
@@ -34,9 +32,31 @@ import org.slf4j.LoggerFactory;
 
 @NotThreadSafe
 final class BaseRouteEntry<C extends Routes & DataObject & ChoiceIn<Tables>,
-        S extends ChildOf<? super C>,
-        R extends Route & ChildOf<? super S> & Identifiable<I>,
-        I extends Identifier<R>> implements RouteEntry<C,S,R,I> {
+        S extends ChildOf<? super C>, R extends Route & ChildOf<? super S> & Identifiable<I>,
+        I extends Identifier<R>> implements RouteEntry<C, S, R, I> {
+    private static final class Stale<C extends Routes & DataObject & ChoiceIn<Tables>,
+            S extends ChildOf<? super C>, R extends Route & ChildOf<? super S> & Identifiable<I>,
+            I extends Identifier<R>> extends StaleBestPathRoute<C, S, R, I> {
+        Stale(final I nonAddPathRouteKeyIdentifier) {
+            super(nonAddPathRouteKeyIdentifier);
+        }
+
+        @Override
+        public List<I> getStaleRouteKeyIdentifiers() {
+            return Collections.singletonList(getNonAddPathRouteKeyIdentifier());
+        }
+
+        @Override
+        public List<I> getAddPathRouteKeyIdentifiers() {
+            return Collections.emptyList();
+        }
+
+        @Override
+        public boolean isNonAddPathBestPathNew() {
+            return true;
+        }
+    }
+
     private static final Logger LOG = LoggerFactory.getLogger(BaseRouteEntry.class);
     private static final Route[] EMPTY_VALUES = new Route[0];
 
@@ -109,13 +129,11 @@ final class BaseRouteEntry<C extends Routes & DataObject & ChoiceIn<Tables>,
     @Override
     public Optional<StaleBestPathRoute<C, S, R, I>> removeStalePaths(final RIBSupport<C, S, R, I> ribSupport,
             final String routeKey) {
-        if (this.removedBestPath == null) {
+        if (removedBestPath == null) {
             return Optional.empty();
         }
-        final StaleBestPathRoute<C, S, R, I> stale = new StaleBestPathRoute<>(ribSupport, routeKey,
-                Collections.singletonList(NON_PATH_ID), Collections.emptyList(), true);
-        this.removedBestPath = null;
-        return Optional.of(stale);
+        removedBestPath = null;
+        return Optional.of(new Stale<C, S, R, I>(ribSupport.createRouteListKey(routeKey)));
     }
 
     @Override
index f20cd5d4018a5a237f898744e532aa463e108363..8fd2d9cb3b72a17a56d24679d2f57f1f2365abc8 100644 (file)
@@ -7,11 +7,9 @@
  */
 package org.opendaylight.protocol.bgp.rib.spi.entry;
 
-import java.util.Collections;
+import static java.util.Objects.requireNonNull;
+
 import java.util.List;
-import java.util.stream.Collectors;
-import org.opendaylight.protocol.bgp.rib.spi.RIBSupport;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev180329.PathId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev180329.Route;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev180329.rib.Tables;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev180329.rib.tables.Routes;
@@ -26,36 +24,16 @@ import org.opendaylight.yangtools.yang.binding.Identifier;
  *
  * @author Claudio D. Gasparini
  */
-public final class StaleBestPathRoute<C extends Routes & DataObject & ChoiceIn<Tables>,
-        S extends ChildOf<? super C>,
-        R extends Route & ChildOf<? super S> & Identifiable<I>,
-        I extends Identifier<R>> {
+public abstract class StaleBestPathRoute<C extends Routes & DataObject & ChoiceIn<Tables>,
+        S extends ChildOf<? super C>, R extends Route & ChildOf<? super S> & Identifiable<I>, I extends Identifier<R>> {
     private final I nonAddPathRouteKeyIdentifier;
-    private final List<I> addPathRouteKeyIdentifier;
-    private final boolean isNonAddPathBestPathNew;
-    private final List<I> staleRouteKeyIdentifier;
-
-    public StaleBestPathRoute(
-            final RIBSupport<C, S, R, I> ribSupport,
-            final String routeKey,
-            final List<PathId> staleRoutesPathIds,
-            final List<PathId> withdrawalRoutePathIds,
-            final boolean isNonAddPathBestPathNew) {
-        this.isNonAddPathBestPathNew = isNonAddPathBestPathNew;
 
-        this.staleRouteKeyIdentifier = staleRoutesPathIds.stream()
-                .map(pathId -> ribSupport.createRouteListKey(pathId, routeKey)).collect(Collectors.toList());
-        if (withdrawalRoutePathIds != null) {
-            this.addPathRouteKeyIdentifier = withdrawalRoutePathIds.stream()
-                    .map(pathId -> ribSupport.createRouteListKey(pathId, routeKey)).collect(Collectors.toList());
-        } else {
-            this.addPathRouteKeyIdentifier = Collections.emptyList();
-        }
-        this.nonAddPathRouteKeyIdentifier = ribSupport.createRouteListKey(routeKey);
+    protected StaleBestPathRoute(final I nonAddPathRouteKeyIdentifier) {
+        this.nonAddPathRouteKeyIdentifier = requireNonNull(nonAddPathRouteKeyIdentifier);
     }
 
-    public I getNonAddPathRouteKeyIdentifier() {
-        return this.nonAddPathRouteKeyIdentifier;
+    public final I getNonAddPathRouteKeyIdentifier() {
+        return nonAddPathRouteKeyIdentifier;
     }
 
     /**
@@ -63,25 +41,19 @@ public final class StaleBestPathRoute<C extends Routes & DataObject & ChoiceIn<T
      *
      * @return Route Identifier List
      */
-    public List<I> getStaleRouteKeyIdentifiers() {
-        return this.staleRouteKeyIdentifier;
-    }
+    public abstract List<I> getStaleRouteKeyIdentifiers();
 
     /**
      * Route Identifier List of withdrawn routes to advertize peers supporting additional Path.
      *
      * @return Route Identifier List
      */
-    public List<I> getAddPathRouteKeyIdentifiers() {
-        return this.addPathRouteKeyIdentifier;
-    }
+    public abstract List<I> getAddPathRouteKeyIdentifiers();
 
     /**
      * Route Identifier of withdrawn routes to advertize peers no supporting additional Path.
      *
      * @return Route Identifier
      */
-    public boolean isNonAddPathBestPathNew() {
-        return this.isNonAddPathBestPathNew;
-    }
+    public abstract boolean isNonAddPathBestPathNew();
 }