Revisit Annotations merging
- avoid extra instantiation/copying when possible
Change-Id: Ia2f74a1fe2ff1176346316565c9bc6786cd7f6e4
diff --git a/core/api/src/main/java/org/onosproject/net/DefaultAnnotations.java b/core/api/src/main/java/org/onosproject/net/DefaultAnnotations.java
index f86df5e..2dab3a2 100644
--- a/core/api/src/main/java/org/onosproject/net/DefaultAnnotations.java
+++ b/core/api/src/main/java/org/onosproject/net/DefaultAnnotations.java
@@ -141,6 +141,10 @@
return annotations;
}
+ if (annotations.keys().isEmpty()) {
+ return sparseAnnotations;
+ }
+
final HashMap<String, String> newMap;
if (annotations instanceof DefaultAnnotations) {
newMap = copy(((DefaultAnnotations) annotations).map);
diff --git a/core/net/src/main/java/org/onosproject/net/device/impl/BasicDeviceOperator.java b/core/net/src/main/java/org/onosproject/net/device/impl/BasicDeviceOperator.java
index ce3ee1a..8bd22d3 100644
--- a/core/net/src/main/java/org/onosproject/net/device/impl/BasicDeviceOperator.java
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/BasicDeviceOperator.java
@@ -86,6 +86,7 @@
*/
public static SparseAnnotations combine(BasicDeviceConfig cfg, SparseAnnotations an) {
DefaultAnnotations.Builder builder = DefaultAnnotations.builder();
+ builder.putAll(an);
if (!Objects.equals(cfg.driver(), an.value(AnnotationKeys.DRIVER))) {
builder.set(AnnotationKeys.DRIVER, cfg.driver());
}
@@ -96,7 +97,7 @@
builder.set(AnnotationKeys.MANAGEMENT_ADDRESS, cfg.managementAddress());
}
- return DefaultAnnotations.union(an, builder.build());
+ return builder.build();
}
/**
diff --git a/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java b/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java
index dd56217..87f908a 100644
--- a/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java
+++ b/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java
@@ -80,10 +80,10 @@
*/
public static SparseAnnotations combine(BasicHostConfig cfg, SparseAnnotations an) {
DefaultAnnotations.Builder builder = DefaultAnnotations.builder();
-
+ builder.putAll(an);
combineElementAnnotations(cfg, builder);
- return DefaultAnnotations.union(an, builder.build());
+ return builder.build();
}
/**
diff --git a/core/net/src/main/java/org/onosproject/net/link/impl/BasicLinkOperator.java b/core/net/src/main/java/org/onosproject/net/link/impl/BasicLinkOperator.java
index 9247b83..7cb6499 100644
--- a/core/net/src/main/java/org/onosproject/net/link/impl/BasicLinkOperator.java
+++ b/core/net/src/main/java/org/onosproject/net/link/impl/BasicLinkOperator.java
@@ -78,6 +78,7 @@
*/
public static SparseAnnotations combine(BasicLinkConfig cfg, SparseAnnotations an) {
DefaultAnnotations.Builder b = DefaultAnnotations.builder();
+ b.putAll(an);
if (cfg.metric() != DEF_METRIC) {
b.set(AnnotationKeys.METRIC, String.valueOf(cfg.metric()));
}
@@ -92,7 +93,7 @@
if (cfg.isDurable() != null) {
b.set(AnnotationKeys.DURABLE, String.valueOf(cfg.isDurable()));
}
- return DefaultAnnotations.union(an, b.build());
+ return b.build();
}
/**
diff --git a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
index 4400221..9bfdc1f 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
@@ -100,7 +100,6 @@
import static org.onlab.util.Tools.groupedThreads;
import static org.onlab.util.Tools.minPriority;
import static org.onosproject.cluster.ControllerNodeToNodeId.toNodeId;
-import static org.onosproject.net.DefaultAnnotations.merge;
import static org.onosproject.net.device.DeviceEvent.Type.DEVICE_AVAILABILITY_CHANGED;
import static org.onosproject.net.device.DeviceEvent.Type.PORT_ADDED;
import static org.onosproject.net.device.DeviceEvent.Type.PORT_REMOVED;
@@ -1123,8 +1122,8 @@
String swVersion = base.swVersion();
String serialNumber = base.serialNumber();
ChassisId chassisId = base.chassisId();
- DefaultAnnotations annotations = DefaultAnnotations.builder().build();
- annotations = merge(annotations, base.annotations());
+ DefaultAnnotations.Builder annotations = DefaultAnnotations.builder();
+ annotations.putAll(base.annotations());
for (Entry<ProviderId, DeviceDescriptions> e : providerDescs.entrySet()) {
if (e.getKey().equals(primary)) {
@@ -1136,12 +1135,12 @@
// providers
// annotation merging. not so efficient, should revisit later
- annotations = merge(annotations, e.getValue().getDeviceDesc().value().annotations());
+ annotations.putAll(e.getValue().getDeviceDesc().value().annotations());
}
return new DefaultDevice(primary, deviceId, type, manufacturer,
hwVersion, swVersion, serialNumber,
- chassisId, annotations);
+ chassisId, annotations.build());
}
private Port buildTypedPort(Device device, PortNumber number, boolean isEnabled,
@@ -1165,12 +1164,12 @@
DeviceDescriptions primDescs = descsMap.get(primary);
// if no primary, assume not enabled
boolean isEnabled = false;
- DefaultAnnotations annotations = DefaultAnnotations.builder().build();
+ DefaultAnnotations.Builder annotations = DefaultAnnotations.builder();
Timestamp newest = null;
final Timestamped<PortDescription> portDesc = primDescs.getPortDesc(number);
if (portDesc != null) {
isEnabled = portDesc.value().isEnabled();
- annotations = merge(annotations, portDesc.value().annotations());
+ annotations.putAll(portDesc.value().annotations());
newest = portDesc.timestamp();
}
Port updated = null;
@@ -1189,18 +1188,18 @@
if (newest != null && newest.isNewerThan(otherPortDesc.timestamp())) {
continue;
}
- annotations = merge(annotations, otherPortDesc.value().annotations());
+ annotations.putAll(otherPortDesc.value().annotations());
PortDescription other = otherPortDesc.value();
- updated = buildTypedPort(device, number, isEnabled, other, annotations);
+ updated = buildTypedPort(device, number, isEnabled, other, annotations.build());
newest = otherPortDesc.timestamp();
}
}
if (portDesc == null) {
- return updated == null ? new DefaultPort(device, number, false, annotations) : updated;
+ return updated == null ? new DefaultPort(device, number, false, annotations.build()) : updated;
}
PortDescription current = portDesc.value();
return updated == null
- ? buildTypedPort(device, number, isEnabled, current, annotations)
+ ? buildTypedPort(device, number, isEnabled, current, annotations.build())
: updated;
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/link/impl/ECLinkStore.java b/core/store/dist/src/main/java/org/onosproject/store/link/impl/ECLinkStore.java
index d6131da..7b92e3b 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/link/impl/ECLinkStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/link/impl/ECLinkStore.java
@@ -74,7 +74,6 @@
import com.google.common.collect.Sets;
import com.google.common.util.concurrent.Futures;
-import static org.onosproject.net.DefaultAnnotations.merge;
import static org.onosproject.net.DefaultAnnotations.union;
import static org.onosproject.net.Link.State.ACTIVE;
import static org.onosproject.net.Link.State.INACTIVE;
@@ -372,26 +371,26 @@
ConnectPoint src = base.src();
ConnectPoint dst = base.dst();
Type type = base.type();
- AtomicReference<DefaultAnnotations> annotations = new AtomicReference<>(DefaultAnnotations.builder().build());
- annotations.set(merge(annotations.get(), base.annotations()));
+ DefaultAnnotations.Builder builder = DefaultAnnotations.builder();
+ builder.putAll(base.annotations());
getAllProviders(linkKey).stream()
.map(p -> new Provided<>(linkKey, p))
.forEach(key -> {
LinkDescription linkDescription = linkDescriptions.get(key);
if (linkDescription != null) {
- annotations.set(merge(annotations.get(),
- linkDescription.annotations()));
+ builder.putAll(linkDescription.annotations());
}
});
+ DefaultAnnotations annotations = builder.build();
Link.State initialLinkState;
boolean isExpected;
if (linkDiscoveryMode == LinkDiscoveryMode.PERMISSIVE) {
initialLinkState = ACTIVE;
isExpected =
- Objects.equals(annotations.get().value(AnnotationKeys.DURABLE), "true");
+ Objects.equals(annotations.value(AnnotationKeys.DURABLE), "true");
} else {
initialLinkState = base.isExpected() ? ACTIVE : INACTIVE;
isExpected = base.isExpected();
@@ -405,7 +404,7 @@
.type(type)
.state(initialLinkState)
.isExpected(isExpected)
- .annotations(annotations.get())
+ .annotations(annotations)
.build();
}