commit 4127d394605f6b0bdbba4968277d0a9e1a90671f
author Jordan Halterman <jordan@opennetworking.org> Tue Jun 12 11:40:21 2018 -0700
committer Ray Milkey <ray@opennetworking.org> Fri Jun 29 17:36:14 2018 +0000
tree 64e050f05bb72bcf6d20f8fc7ca3a5ceb36012b7
parent 8f90d6d707f67cd48bcd893c9dcf006f002fc16b

Use external null checks to avoid computeIfAbsent calls in FlowRuleStore

Change-Id: Ib5cc1bb77dfcd8f38b52cb175f21e1f1055447a5

core/store/dist/src/main/java/org/onosproject/store/flow/impl/ECFlowRuleStore.java

diff --git a/core/store/dist/src/main/java/org/onosproject/store/flow/impl/ECFlowRuleStore.java b/core/store/dist/src/main/java/org/onosproject/store/flow/impl/ECFlowRuleStore.java
index 204f98f..b89bd58 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/flow/impl/ECFlowRuleStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/flow/impl/ECFlowRuleStore.java
@@ -783,43 +783,37 @@
          * @return Map representing Flow Table of given device.
          */
         private Map<FlowId, Map<StoredFlowEntry, StoredFlowEntry>> getFlowTable(DeviceId deviceId) {
+            // Use an external get/null check to avoid locks.
+            // https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8161372
             if (persistenceEnabled) {
-                return flowEntries.computeIfAbsent(deviceId, id -> persistenceService
-                        .<FlowId, Map<StoredFlowEntry, StoredFlowEntry>>persistentMapBuilder()
+                Map<FlowId, Map<StoredFlowEntry, StoredFlowEntry>> flowTable = flowEntries.get(deviceId);
+                return flowTable != null ? flowTable
+                    : flowEntries.computeIfAbsent(deviceId, id ->
+                    persistenceService.<FlowId, Map<StoredFlowEntry, StoredFlowEntry>>persistentMapBuilder()
                         .withName("FlowTable:" + deviceId.toString())
                         .withSerializer(serializer)
                         .build());
             } else {
-                return flowEntries.computeIfAbsent(deviceId, id -> Maps.newConcurrentMap());
+                Map<FlowId, Map<StoredFlowEntry, StoredFlowEntry>> flowTable = flowEntries.get(deviceId);
+                return flowTable != null ? flowTable
+                    : flowEntries.computeIfAbsent(deviceId, id -> Maps.newConcurrentMap());
             }
         }
 
         private FlowBucket getFlowBucket(BucketId bucketId) {
-            if (persistenceEnabled) {
-                return new FlowBucket(bucketId, flowEntries.computeIfAbsent(bucketId.deviceId(), id ->
-                    persistenceService.<FlowId, Map<StoredFlowEntry, StoredFlowEntry>>persistentMapBuilder()
-                        .withName("FlowTable:" + bucketId.deviceId().toString())
-                        .withSerializer(serializer)
-                        .build())
-                    .entrySet()
-                    .stream()
-                    .filter(entry -> isInBucket(entry.getKey(), bucketId.bucket()))
-                    .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)));
-            } else {
-                Map<FlowId, Map<StoredFlowEntry, StoredFlowEntry>> copy = Maps.newHashMap();
-                flowEntries.computeIfAbsent(bucketId.deviceId(), id -> Maps.newConcurrentMap())
-                    .entrySet()
-                    .stream()
-                    .filter(entry -> isInBucket(entry.getKey(), bucketId.bucket()))
-                    .forEach(entry -> {
-                        copy.put(entry.getKey(), Maps.newHashMap(entry.getValue()));
-                    });
-                return new FlowBucket(bucketId, copy);
-            }
+            return new FlowBucket(bucketId, getFlowTable(bucketId.deviceId())
+                .entrySet()
+                .stream()
+                .filter(entry -> isInBucket(entry.getKey(), bucketId.bucket()))
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)));
         }
 
         private Map<StoredFlowEntry, StoredFlowEntry> getFlowEntriesInternal(DeviceId deviceId, FlowId flowId) {
-            return getFlowTable(deviceId).computeIfAbsent(flowId, id -> Maps.newConcurrentMap());
+            // Use an external get/null check to avoid locks.
+            // https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8161372
+            Map<FlowId, Map<StoredFlowEntry, StoredFlowEntry>> flowTable = getFlowTable(deviceId);
+            Map<StoredFlowEntry, StoredFlowEntry> flowEntries = flowTable.get(flowId);
+            return flowEntries != null ? flowEntries : flowTable.computeIfAbsent(flowId, id -> Maps.newConcurrentMap());
         }
 
         private StoredFlowEntry getFlowEntryInternal(FlowRule rule) {