From 33faf87a787aaa939b562a866622859b4e73aef3 Mon Sep 17 00:00:00 2001 From: Maros Marsalek Date: Fri, 9 Jan 2015 10:05:54 +0100 Subject: [PATCH] Optimize Capability.getLocation() Instead of wrapping a List in an Optional, return a Collection -- when empty, it is the same thing as Optional.absent(). We can then optimize some of the object allocations. Change-Id: I040e1e80d9012c2809370accb89daf038c7cd2e4 Signed-off-by: Robert Varga Signed-off-by: Maros Marsalek --- .../osgi/NetconfOperationServiceImpl.java | 21 +++-- .../osgi/NetconfMonitoringServiceImpl.java | 91 +++++++++---------- .../NetconfMonitoringServiceImplTest.java | 2 +- .../netconf/it/NetconfITMonitoringTest.java | 4 +- .../netconf/mapping/api/Capability.java | 4 +- .../NetconfMonitoringOperationService.java | 9 +- ...NetconfMonitoringOperationServiceTest.java | 5 +- .../test/tool/ModuleBuilderCapability.java | 5 +- 8 files changed, 72 insertions(+), 69 deletions(-) diff --git a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/osgi/NetconfOperationServiceImpl.java b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/osgi/NetconfOperationServiceImpl.java index 7008879e9f..902be44fd9 100644 --- a/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/osgi/NetconfOperationServiceImpl.java +++ b/opendaylight/netconf/config-netconf-connector/src/main/java/org/opendaylight/controller/netconf/confignetconfconnector/osgi/NetconfOperationServiceImpl.java @@ -12,8 +12,9 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.collect.Sets; +import java.util.Collection; +import java.util.Collections; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Set; import org.opendaylight.controller.config.api.LookupRegistry; @@ -36,8 +37,8 @@ public class NetconfOperationServiceImpl implements NetconfOperationService { private final Set capabilities; private final TransactionProvider transactionProvider; - public NetconfOperationServiceImpl(YangStoreService yangStoreService, ConfigRegistryJMXClient jmxClient, - String netconfSessionIdForReporting) throws YangStoreException { + public NetconfOperationServiceImpl(final YangStoreService yangStoreService, final ConfigRegistryJMXClient jmxClient, + final String netconfSessionIdForReporting) throws YangStoreException { yangStoreSnapshot = yangStoreService.getYangStoreSnapshot(); checkConsistencyBetweenYangStoreAndConfig(jmxClient, yangStoreSnapshot); @@ -50,7 +51,7 @@ public class NetconfOperationServiceImpl implements NetconfOperationService { @VisibleForTesting - static void checkConsistencyBetweenYangStoreAndConfig(LookupRegistry jmxClient, YangStoreSnapshot yangStoreSnapshot) { + static void checkConsistencyBetweenYangStoreAndConfig(final LookupRegistry jmxClient, final YangStoreSnapshot yangStoreSnapshot) { Set missingModulesFromConfig = Sets.newHashSet(); Set modulesSeenByConfig = jmxClient.getAvailableModuleFactoryQNames(); @@ -89,7 +90,7 @@ public class NetconfOperationServiceImpl implements NetconfOperationService { return operationProvider.getOperations(); } - private static Set setupCapabilities(YangStoreSnapshot yangStoreSnapshot) { + private static Set setupCapabilities(final YangStoreSnapshot yangStoreSnapshot) { Set capabilities = new HashSet<>(); // [RFC6241] 8.3. Candidate Configuration Capability capabilities.add(new BasicCapability("urn:ietf:params:netconf:capability:candidate:1.0")); @@ -110,7 +111,7 @@ public class NetconfOperationServiceImpl implements NetconfOperationService { private final String capability; - private BasicCapability(String capability) { + private BasicCapability(final String capability) { this.capability = capability; } @@ -140,8 +141,8 @@ public class NetconfOperationServiceImpl implements NetconfOperationService { } @Override - public Optional> getLocation() { - return Optional.absent(); + public Collection getLocation() { + return Collections.emptyList(); } @Override @@ -157,7 +158,7 @@ public class NetconfOperationServiceImpl implements NetconfOperationService { private final String moduleName; private final String moduleNamespace; - public YangStoreCapability(Module module, String moduleContent) { + public YangStoreCapability(final Module module, final String moduleContent) { super(toCapabilityURI(module)); this.content = moduleContent; this.moduleName = module.getName(); @@ -170,7 +171,7 @@ public class NetconfOperationServiceImpl implements NetconfOperationService { return Optional.of(content); } - private static String toCapabilityURI(Module module) { + private static String toCapabilityURI(final Module module) { return String.valueOf(module.getNamespace()) + "?module=" + module.getName() + "&revision=" + Util.writeDate(module.getRevision()); } diff --git a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfMonitoringServiceImpl.java b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfMonitoringServiceImpl.java index 3f44ff4ff8..efbe3ad68f 100644 --- a/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfMonitoringServiceImpl.java +++ b/opendaylight/netconf/netconf-impl/src/main/java/org/opendaylight/controller/netconf/impl/osgi/NetconfMonitoringServiceImpl.java @@ -10,10 +10,12 @@ package org.opendaylight.controller.netconf.impl.osgi; import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.collect.Collections2; -import com.google.common.collect.Lists; -import com.google.common.collect.Sets; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableList.Builder; import io.netty.util.internal.ConcurrentSet; -import java.util.Collections; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Set; import javax.annotation.Nonnull; @@ -37,25 +39,32 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class NetconfMonitoringServiceImpl implements NetconfMonitoringService, SessionMonitoringService { - + private static final Schema.Location NETCONF_LOCATION = new Schema.Location(Schema.Location.Enumeration.NETCONF); + private static final List NETCONF_LOCATIONS = ImmutableList.of(NETCONF_LOCATION); private static final Logger LOG = LoggerFactory.getLogger(NetconfMonitoringServiceImpl.class); + private static final Function SESSION_FUNCTION = new Function() { + @Override + public Session apply(@Nonnull final NetconfManagementSession input) { + return input.toManagementSession(); + } + }; private final Set sessions = new ConcurrentSet<>(); private final NetconfOperationProvider netconfOperationProvider; - public NetconfMonitoringServiceImpl(NetconfOperationProvider netconfOperationProvider) { + public NetconfMonitoringServiceImpl(final NetconfOperationProvider netconfOperationProvider) { this.netconfOperationProvider = netconfOperationProvider; } @Override - public void onSessionUp(NetconfManagementSession session) { + public void onSessionUp(final NetconfManagementSession session) { LOG.debug("Session {} up", session); Preconditions.checkState(!sessions.contains(session), "Session %s was already added", session); sessions.add(session); } @Override - public void onSessionDown(NetconfManagementSession session) { + public void onSessionDown(final NetconfManagementSession session) { LOG.debug("Session {} down", session); Preconditions.checkState(sessions.contains(session), "Session %s not present", session); sessions.remove(session); @@ -63,7 +72,7 @@ public class NetconfMonitoringServiceImpl implements NetconfMonitoringService, S @Override public Sessions getSessions() { - return new SessionsBuilder().setSession(transformSessions(sessions)).build(); + return new SessionsBuilder().setSession(ImmutableList.copyOf(Collections2.transform(sessions, SESSION_FUNCTION))).build(); } @Override @@ -78,65 +87,55 @@ public class NetconfMonitoringServiceImpl implements NetconfMonitoringService, S } } - private Schemas transformSchemas(Set services) { - Set caps = Sets.newHashSet(); - - List schemas = Lists.newArrayList(); - - + private static Schemas transformSchemas(final Set services) { + // FIXME: Capability implementations do not have hashcode/equals! + final Set caps = new HashSet<>(); for (NetconfOperationService netconfOperationService : services) { // TODO check for duplicates ? move capability merging to snapshot // Split capabilities from operations first and delete this duplicate code caps.addAll(netconfOperationService.getCapabilities()); } + final List schemas = new ArrayList<>(caps.size()); for (Capability cap : caps) { - SchemaBuilder builder = new SchemaBuilder(); - - if (cap.getCapabilitySchema().isPresent() == false) { - continue; - } + if (cap.getCapabilitySchema().isPresent()) { + SchemaBuilder builder = new SchemaBuilder(); + Preconditions.checkState(cap.getModuleNamespace().isPresent()); + builder.setNamespace(new Uri(cap.getModuleNamespace().get())); - Preconditions.checkState(cap.getModuleNamespace().isPresent()); - builder.setNamespace(new Uri(cap.getModuleNamespace().get())); + Preconditions.checkState(cap.getRevision().isPresent()); + String version = cap.getRevision().get(); + builder.setVersion(version); - Preconditions.checkState(cap.getRevision().isPresent()); - String version = cap.getRevision().get(); - builder.setVersion(version); + Preconditions.checkState(cap.getModuleName().isPresent()); + String identifier = cap.getModuleName().get(); + builder.setIdentifier(identifier); - Preconditions.checkState(cap.getModuleName().isPresent()); - String identifier = cap.getModuleName().get(); - builder.setIdentifier(identifier); + builder.setFormat(Yang.class); - builder.setFormat(Yang.class); + builder.setLocation(transformLocations(cap.getLocation())); - builder.setLocation(transformLocations(cap.getLocation().or(Collections.emptyList()))); + builder.setKey(new SchemaKey(Yang.class, identifier, version)); - builder.setKey(new SchemaKey(Yang.class, identifier, version)); - - schemas.add(builder.build()); + schemas.add(builder.build()); + } } return new SchemasBuilder().setSchema(schemas).build(); } - private List transformLocations(List locations) { - List monitoringLocations = Lists.newArrayList(); - monitoringLocations.add(new Schema.Location(Schema.Location.Enumeration.NETCONF)); + private static List transformLocations(final Collection locations) { + if (locations.isEmpty()) { + return NETCONF_LOCATIONS; + } + + final Builder b = ImmutableList.builder(); + b.add(NETCONF_LOCATION); for (String location : locations) { - monitoringLocations.add(new Schema.Location(new Uri(location))); + b.add(new Schema.Location(new Uri(location))); } - return monitoringLocations; - } - - private List transformSessions(Set sessions) { - return Lists.newArrayList(Collections2.transform(sessions, new Function() { - @Override - public Session apply(@Nonnull NetconfManagementSession input) { - return input.toManagementSession(); - } - })); + return b.build(); } } diff --git a/opendaylight/netconf/netconf-impl/src/test/java/org/opendaylight/controller/netconf/impl/NetconfMonitoringServiceImplTest.java b/opendaylight/netconf/netconf-impl/src/test/java/org/opendaylight/controller/netconf/impl/NetconfMonitoringServiceImplTest.java index 0d296c5f52..93caa09286 100644 --- a/opendaylight/netconf/netconf-impl/src/test/java/org/opendaylight/controller/netconf/impl/NetconfMonitoringServiceImplTest.java +++ b/opendaylight/netconf/netconf-impl/src/test/java/org/opendaylight/controller/netconf/impl/NetconfMonitoringServiceImplTest.java @@ -90,7 +90,7 @@ public class NetconfMonitoringServiceImplTest { Optional optRev = Optional.of("rev"); doReturn(optRev).when(cap).getRevision(); doReturn(Optional.of("modName")).when(cap).getModuleName(); - doReturn(Optional.of(Lists.newArrayList("loc"))).when(cap).getLocation(); + doReturn(Lists.newArrayList("loc")).when(cap).getLocation(); doNothing().when(snapshot).close(); assertNotNull(service.getSchemas()); diff --git a/opendaylight/netconf/netconf-it/src/test/java/org/opendaylight/controller/netconf/it/NetconfITMonitoringTest.java b/opendaylight/netconf/netconf-it/src/test/java/org/opendaylight/controller/netconf/it/NetconfITMonitoringTest.java index 463b5f045e..ea94fd3f8b 100644 --- a/opendaylight/netconf/netconf-it/src/test/java/org/opendaylight/controller/netconf/it/NetconfITMonitoringTest.java +++ b/opendaylight/netconf/netconf-it/src/test/java/org/opendaylight/controller/netconf/it/NetconfITMonitoringTest.java @@ -199,8 +199,8 @@ public class NetconfITMonitoringTest extends AbstractNetconfConfigTest { } @Override - public Optional> getLocation() { - return Optional.absent(); + public List getLocation() { + return Collections.emptyList(); } } } diff --git a/opendaylight/netconf/netconf-mapping-api/src/main/java/org/opendaylight/controller/netconf/mapping/api/Capability.java b/opendaylight/netconf/netconf-mapping-api/src/main/java/org/opendaylight/controller/netconf/mapping/api/Capability.java index f55a90e568..408756bf4d 100644 --- a/opendaylight/netconf/netconf-mapping-api/src/main/java/org/opendaylight/controller/netconf/mapping/api/Capability.java +++ b/opendaylight/netconf/netconf-mapping-api/src/main/java/org/opendaylight/controller/netconf/mapping/api/Capability.java @@ -9,7 +9,7 @@ package org.opendaylight.controller.netconf.mapping.api; import com.google.common.base.Optional; -import java.util.List; +import java.util.Collection; /** * Contains capability URI announced by server hello message and optionally its @@ -27,5 +27,5 @@ public interface Capability { public Optional getCapabilitySchema(); - public Optional> getLocation(); + public Collection getLocation(); } diff --git a/opendaylight/netconf/netconf-monitoring/src/main/java/org/opendaylight/controller/netconf/monitoring/osgi/NetconfMonitoringOperationService.java b/opendaylight/netconf/netconf-monitoring/src/main/java/org/opendaylight/controller/netconf/monitoring/osgi/NetconfMonitoringOperationService.java index e9e92d9202..a17e139131 100644 --- a/opendaylight/netconf/netconf-monitoring/src/main/java/org/opendaylight/controller/netconf/monitoring/osgi/NetconfMonitoringOperationService.java +++ b/opendaylight/netconf/netconf-monitoring/src/main/java/org/opendaylight/controller/netconf/monitoring/osgi/NetconfMonitoringOperationService.java @@ -9,7 +9,8 @@ package org.opendaylight.controller.netconf.monitoring.osgi; import com.google.common.base.Optional; import com.google.common.collect.Sets; -import java.util.List; +import java.util.Collection; +import java.util.Collections; import java.util.Set; import org.opendaylight.controller.netconf.api.monitoring.NetconfMonitoringService; import org.opendaylight.controller.netconf.mapping.api.Capability; @@ -48,14 +49,14 @@ public class NetconfMonitoringOperationService implements NetconfOperationServic } @Override - public Optional> getLocation() { - return Optional.absent(); + public Collection getLocation() { + return Collections.emptyList(); } }); private final NetconfMonitoringService monitor; - public NetconfMonitoringOperationService(NetconfMonitoringService monitor) { + public NetconfMonitoringOperationService(final NetconfMonitoringService monitor) { this.monitor = monitor; } diff --git a/opendaylight/netconf/netconf-monitoring/src/test/java/org/opendaylight/controller/netconf/monitoring/osgi/NetconfMonitoringOperationServiceTest.java b/opendaylight/netconf/netconf-monitoring/src/test/java/org/opendaylight/controller/netconf/monitoring/osgi/NetconfMonitoringOperationServiceTest.java index 74e6747ab7..7873183188 100644 --- a/opendaylight/netconf/netconf-monitoring/src/test/java/org/opendaylight/controller/netconf/monitoring/osgi/NetconfMonitoringOperationServiceTest.java +++ b/opendaylight/netconf/netconf-monitoring/src/test/java/org/opendaylight/controller/netconf/monitoring/osgi/NetconfMonitoringOperationServiceTest.java @@ -12,6 +12,7 @@ import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.mock; import com.google.common.base.Optional; +import java.util.Collections; import org.junit.Test; import org.opendaylight.controller.netconf.api.monitoring.NetconfMonitoringService; import org.opendaylight.controller.netconf.monitoring.MonitoringConstants; @@ -24,8 +25,8 @@ public class NetconfMonitoringOperationServiceTest { assertEquals(1, service.getNetconfOperations().size()); - assertEquals(Optional.absent(), service.getCapabilities().iterator().next().getCapabilitySchema()); - assertEquals(Optional.absent(), service.getCapabilities().iterator().next().getLocation()); + assertEquals(Optional.absent(), service.getCapabilities().iterator().next().getCapabilitySchema()); + assertEquals(Collections.emptyList(), service.getCapabilities().iterator().next().getLocation()); assertEquals(Optional.of(MonitoringConstants.MODULE_REVISION), service.getCapabilities().iterator().next().getRevision()); assertEquals(Optional.of(MonitoringConstants.MODULE_NAME), service.getCapabilities().iterator().next().getModuleName()); assertEquals(Optional.of(MonitoringConstants.NAMESPACE), service.getCapabilities().iterator().next().getModuleNamespace()); diff --git a/opendaylight/netconf/netconf-testtool/src/main/java/org/opendaylight/controller/netconf/test/tool/ModuleBuilderCapability.java b/opendaylight/netconf/netconf-testtool/src/main/java/org/opendaylight/controller/netconf/test/tool/ModuleBuilderCapability.java index 1a68f55e55..68f8796d81 100644 --- a/opendaylight/netconf/netconf-testtool/src/main/java/org/opendaylight/controller/netconf/test/tool/ModuleBuilderCapability.java +++ b/opendaylight/netconf/netconf-testtool/src/main/java/org/opendaylight/controller/netconf/test/tool/ModuleBuilderCapability.java @@ -9,6 +9,7 @@ package org.opendaylight.controller.netconf.test.tool; import com.google.common.base.Optional; +import java.util.Collections; import java.util.Date; import java.util.List; import org.opendaylight.controller.netconf.confignetconfconnector.util.Util; @@ -58,7 +59,7 @@ final class ModuleBuilderCapability implements Capability { } @Override - public Optional> getLocation() { - return Optional.absent(); + public List getLocation() { + return Collections.emptyList(); } } -- 2.36.6