Speed up PeerSpecificParserConstraintImpl 73/78373/6
authorRobert Varga <robert.varga@pantheon.tech>
Mon, 3 Dec 2018 10:34:19 +0000 (11:34 +0100)
committerClaudio David Gasparini <claudio.gasparini@pantheon.tech>
Fri, 7 Dec 2018 15:16:58 +0000 (15:16 +0000)
This improves design by taking advantage of ClassToInstanceMap,
which provides a type-safe getInstance() method. Furthermore we
recognize that the this class is used mostly for lookups of a few
classes, we use an ImmutableClassToInstanceMap updated concurrently
via a compare-and-set.

This yields significantly faster lookups for the common cases, where
the map is empty or has a single entry. In other cases the speed up
is still there, although not as pronounced.

Change-Id: I4da36e592e88cf22d5ad1654f126eec81660c98c
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/pojo/PeerSpecificParserConstraintImpl.java

index 1455dbbf25ff5b9764852aafb16d1d5d33b665d1..5fbbce52c28b63a046e8bd97c8612a77d0a8de70 100644 (file)
@@ -10,29 +10,44 @@ package org.opendaylight.protocol.bgp.parser.spi.pojo;
 
 import static java.util.Objects.requireNonNull;
 
-import java.util.HashMap;
-import java.util.Map;
+import com.google.common.collect.ImmutableClassToInstanceMap;
+import com.google.common.collect.ImmutableClassToInstanceMap.Builder;
 import java.util.Optional;
-import javax.annotation.concurrent.GuardedBy;
+import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 import org.opendaylight.protocol.bgp.parser.spi.PeerConstraint;
 import org.opendaylight.protocol.bgp.parser.spi.PeerSpecificParserConstraintProvider;
 
 public class PeerSpecificParserConstraintImpl implements PeerSpecificParserConstraintProvider {
+    @SuppressWarnings("rawtypes")
+    private static final AtomicReferenceFieldUpdater<PeerSpecificParserConstraintImpl, ImmutableClassToInstanceMap>
+            CONSTRAINTS_UPDATER = AtomicReferenceFieldUpdater.newUpdater(PeerSpecificParserConstraintImpl.class,
+                ImmutableClassToInstanceMap.class, "constraints");
 
-    @GuardedBy("this")
-    private final Map<Class<? extends PeerConstraint>, PeerConstraint> constraints = new HashMap<>();
+    private volatile ImmutableClassToInstanceMap<PeerConstraint> constraints = ImmutableClassToInstanceMap.of();
 
     @Override
-    public synchronized <T extends PeerConstraint> Optional<T> getPeerConstraint(final Class<T> peerConstraintType) {
-        return (Optional<T>) Optional.ofNullable(this.constraints.get(peerConstraintType));
+    public <T extends PeerConstraint> Optional<T> getPeerConstraint(final Class<T> peerConstraintType) {
+        return Optional.ofNullable(constraints.getInstance(peerConstraintType));
     }
 
     @Override
-    public synchronized <T extends PeerConstraint> boolean addPeerConstraint(final Class<T> classType, final T peerConstraint) {
+    public <T extends PeerConstraint> boolean addPeerConstraint(final Class<T> classType, final T peerConstraint) {
         requireNonNull(classType);
         requireNonNull(peerConstraint);
-        final PeerConstraint previous = this.constraints.putIfAbsent(classType, peerConstraint);
-        return previous == null;
-    }
 
+        ImmutableClassToInstanceMap<PeerConstraint> local = constraints;
+        while (!local.containsKey(classType)) {
+            final Builder<PeerConstraint> builder = ImmutableClassToInstanceMap.builder();
+            builder.putAll(local);
+            builder.put(classType, peerConstraint);
+            if (CONSTRAINTS_UPDATER.compareAndSet(this, local, builder.build())) {
+                // Successfully updated, finished
+                return true;
+            }
+
+            // Raced with another update, retry
+            local = constraints;
+        }
+        return false;
+    }
 }