From de384747d3e814e9157436bfd9fe87db7494efbd Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 14 Nov 2014 12:13:40 +0100 Subject: [PATCH] Use ImmutableSet/Map in ServiceReferenceRegistry Rather than wrapping collections in unmodifiable references, perform a copy to obtain a properly-constant collection. These references are usually retained for longer time, so opting for memory efficiency is better than using sparse collections. Change-Id: I708beaaa9abb13560297a7eb1369322220e87d5a Signed-off-by: Robert Varga --- .../impl/ServiceReferenceRegistryImpl.java | 81 ++++++++++--------- .../manager/impl/util/InterfacesHelper.java | 5 +- 2 files changed, 44 insertions(+), 42 deletions(-) diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ServiceReferenceRegistryImpl.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ServiceReferenceRegistryImpl.java index 0f881e95ad..0dff41402e 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ServiceReferenceRegistryImpl.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/ServiceReferenceRegistryImpl.java @@ -8,7 +8,8 @@ package org.opendaylight.controller.config.manager.impl; import static com.google.common.base.Preconditions.checkNotNull; - +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -71,22 +72,22 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } @Override - public Set lookupConfigBeans(String moduleName) { + public Set lookupConfigBeans(final String moduleName) { throw new UnsupportedOperationException(); } @Override - public Set lookupConfigBeans(String moduleName, String instanceName) { + public Set lookupConfigBeans(final String moduleName, final String instanceName) { throw new UnsupportedOperationException(); } @Override - public ObjectName lookupConfigBean(String moduleName, String instanceName) throws InstanceNotFoundException { + public ObjectName lookupConfigBean(final String moduleName, final String instanceName) throws InstanceNotFoundException { throw new UnsupportedOperationException(); } @Override - public void checkConfigBeanExists(ObjectName objectName) throws InstanceNotFoundException { + public void checkConfigBeanExists(final ObjectName objectName) throws InstanceNotFoundException { throw new InstanceNotFoundException("Cannot find " + objectName + " - Tried to use mocking registry"); } @@ -110,7 +111,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } @Override - public ServiceReferenceJMXRegistration registerMBean(ServiceReferenceMXBeanImpl object, ObjectName on) throws InstanceAlreadyExistsException { + public ServiceReferenceJMXRegistration registerMBean(final ServiceReferenceMXBeanImpl object, final ObjectName on) throws InstanceAlreadyExistsException { throw new UnsupportedOperationException(); } @@ -128,9 +129,9 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe /** * Static constructor for transaction controller. Take current state as seen by config registry, allow writing new data. */ - public static SearchableServiceReferenceWritableRegistry createSRWritableRegistry(ServiceReferenceReadableRegistry oldReadableRegistry, - ConfigTransactionLookupRegistry txLookupRegistry, - Map> currentlyRegisteredFactories) { + public static SearchableServiceReferenceWritableRegistry createSRWritableRegistry(final ServiceReferenceReadableRegistry oldReadableRegistry, + final ConfigTransactionLookupRegistry txLookupRegistry, + final Map> currentlyRegisteredFactories) { if (txLookupRegistry == null) { throw new IllegalArgumentException("txLookupRegistry is null"); @@ -148,8 +149,8 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe /** * Copy back state to config registry after commit. */ - public static CloseableServiceReferenceReadableRegistry createSRReadableRegistry(ServiceReferenceWritableRegistry oldWritableRegistry, - LookupRegistry lookupRegistry, BaseJMXRegistrator baseJMXRegistrator) { + public static CloseableServiceReferenceReadableRegistry createSRReadableRegistry(final ServiceReferenceWritableRegistry oldWritableRegistry, + final LookupRegistry lookupRegistry, final BaseJMXRegistrator baseJMXRegistrator) { ServiceReferenceRegistryImpl old = (ServiceReferenceRegistryImpl) oldWritableRegistry; // even if factories do change, nothing in the mapping can change between transactions @@ -163,7 +164,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe /** * Fill refNames and mBeans maps from old instance */ - private static void copy(ServiceReferenceRegistryImpl old, ServiceReferenceRegistryImpl newRegistry, String nullableDstTransactionName) { + private static void copy(final ServiceReferenceRegistryImpl old, final ServiceReferenceRegistryImpl newRegistry, final String nullableDstTransactionName) { for (Entry> refNameEntry : old.mBeans.entrySet()) { ObjectName currentImplementation; ObjectName currentImplementationSrc = refNameEntry.getValue().getKey().getCurrentImplementation(); @@ -182,7 +183,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } } - private static Map extractFactoriesMap(Map> currentlyRegisteredFactories) { + private static Map extractFactoriesMap(final Map> currentlyRegisteredFactories) { Map result = new HashMap<>(); for (Entry> entry : currentlyRegisteredFactories.entrySet()) { result.put(entry.getKey(), entry.getValue().getKey()); @@ -190,9 +191,9 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe return result; } - private ServiceReferenceRegistryImpl(Map factories, LookupRegistry lookupRegistry, - ServiceReferenceTransactionRegistratorFactory serviceReferenceRegistratorFactory, - boolean writable) { + private ServiceReferenceRegistryImpl(final Map factories, final LookupRegistry lookupRegistry, + final ServiceReferenceTransactionRegistratorFactory serviceReferenceRegistratorFactory, + final boolean writable) { this.factories = factories; this.writable = writable; this.lookupRegistry = lookupRegistry; @@ -213,10 +214,10 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe Set qNames = InterfacesHelper.getQNames(siAnnotations); allAnnotations.addAll(siAnnotations); allQNameSet.addAll(qNames); - modifiableFactoryNamesToQNames.put(entry.getKey(), Collections.unmodifiableSet(qNames)); + modifiableFactoryNamesToQNames.put(entry.getKey(), qNames); } - this.factoryNamesToQNames = Collections.unmodifiableMap(modifiableFactoryNamesToQNames); - this.allQNames = Collections.unmodifiableSet(allQNameSet); + this.factoryNamesToQNames = ImmutableMap.copyOf(modifiableFactoryNamesToQNames); + this.allQNames = ImmutableSet.copyOf(allQNameSet); // fill namespacesToAnnotations Map> modifiableNamespacesToAnnotations = new HashMap<>(); @@ -235,13 +236,13 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe ofNamespace.put(sia.localName(), sia); modifiableServiceQNamesToAnnotations.put(sia.value(), sia); } - this.namespacesToAnnotations = Collections.unmodifiableMap(modifiableNamespacesToAnnotations); - this.serviceQNamesToAnnotations = Collections.unmodifiableMap(modifiableServiceQNamesToAnnotations); + this.namespacesToAnnotations = ImmutableMap.copyOf(modifiableNamespacesToAnnotations); + this.serviceQNamesToAnnotations = ImmutableMap.copyOf(modifiableServiceQNamesToAnnotations); LOGGER.trace("factoryNamesToQNames:{}", this.factoryNamesToQNames); } @Override - public Map findServiceInterfaces(ModuleIdentifier moduleIdentifier) { + public Map findServiceInterfaces(final ModuleIdentifier moduleIdentifier) { Map result = modulesToServiceRef.get(moduleIdentifier); if (result == null) { return Collections.emptyMap(); @@ -250,7 +251,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } @Override - public synchronized Set lookupServiceInterfaceNames(ObjectName objectName) throws InstanceNotFoundException { + public synchronized Set lookupServiceInterfaceNames(final ObjectName objectName) throws InstanceNotFoundException { lookupRegistry.checkConfigBeanExists(objectName); String factoryName = ObjectNameUtil.getFactoryName(objectName); @@ -264,7 +265,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } @Override - public synchronized String getServiceInterfaceName(String namespace, String localName) { + public synchronized String getServiceInterfaceName(final String namespace, final String localName) { Map ofNamespace = namespacesToAnnotations.get(namespace); if (ofNamespace == null) { LOGGER.error("Cannot find namespace {} in {}", namespace, namespacesToAnnotations); @@ -295,7 +296,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe return result; } - private ObjectName getObjectName(ModuleIdentifier moduleIdentifier) { + private ObjectName getObjectName(final ModuleIdentifier moduleIdentifier) { ObjectName on; try { on = lookupRegistry.lookupConfigBean(moduleIdentifier.getFactoryName(), moduleIdentifier.getInstanceName()); @@ -307,7 +308,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } @Override - public synchronized ObjectName lookupConfigBeanByServiceInterfaceName(String serviceInterfaceQName, String refName) { + public synchronized ObjectName lookupConfigBeanByServiceInterfaceName(final String serviceInterfaceQName, final String refName) { ServiceReference serviceReference = new ServiceReference(serviceInterfaceQName, refName); ModuleIdentifier moduleIdentifier = refNames.get(serviceReference); if (moduleIdentifier == null) { @@ -318,7 +319,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } @Override - public synchronized Map lookupServiceReferencesByServiceInterfaceName(String serviceInterfaceQName) { + public synchronized Map lookupServiceReferencesByServiceInterfaceName(final String serviceInterfaceQName) { Map> serviceMapping = getServiceMapping(); Map innerMap = serviceMapping.get(serviceInterfaceQName); if (innerMap == null) { @@ -329,7 +330,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } @Override - public synchronized ObjectName getServiceReference(String serviceInterfaceQName, String refName) throws InstanceNotFoundException { + public synchronized ObjectName getServiceReference(final String serviceInterfaceQName, final String refName) throws InstanceNotFoundException { ServiceReference serviceReference = new ServiceReference(serviceInterfaceQName, refName); if (mBeans.containsKey(serviceReference) == false) { throw new InstanceNotFoundException("Cannot find " + serviceReference); @@ -338,7 +339,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } @Override - public synchronized void checkServiceReferenceExists(ObjectName objectName) throws InstanceNotFoundException { + public synchronized void checkServiceReferenceExists(final ObjectName objectName) throws InstanceNotFoundException { String actualTransactionName = ObjectNameUtil.getTransactionName(objectName); String expectedTransactionName = serviceReferenceRegistrator.getNullableTransactionName(); if (writable & actualTransactionName == null || (writable && actualTransactionName.equals(expectedTransactionName) == false)) { @@ -362,19 +363,19 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } @Override - public synchronized ObjectName saveServiceReference(String serviceInterfaceName, String refName, ObjectName moduleON) throws InstanceNotFoundException { + public synchronized ObjectName saveServiceReference(final String serviceInterfaceName, final String refName, final ObjectName moduleON) throws InstanceNotFoundException { assertWritable(); ServiceReference serviceReference = new ServiceReference(serviceInterfaceName, refName); return saveServiceReference(serviceReference, moduleON); } - private synchronized ObjectName saveServiceReference(ServiceReference serviceReference, ObjectName moduleON) + private synchronized ObjectName saveServiceReference(final ServiceReference serviceReference, final ObjectName moduleON) throws InstanceNotFoundException{ return saveServiceReference(serviceReference, moduleON, false); } - private synchronized ObjectName saveServiceReference(ServiceReference serviceReference, ObjectName moduleON, - boolean skipChecks) throws InstanceNotFoundException { + private synchronized ObjectName saveServiceReference(final ServiceReference serviceReference, final ObjectName moduleON, + final boolean skipChecks) throws InstanceNotFoundException { // make sure it is found if (skipChecks == false) { @@ -443,13 +444,13 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } @Override - public ServiceReferenceJMXRegistration setValue(ServiceReferenceJMXRegistration value) { + public ServiceReferenceJMXRegistration setValue(final ServiceReferenceJMXRegistration value) { throw new UnsupportedOperationException(); } }; } - private ObjectName getServiceON(ServiceReference serviceReference) { + private ObjectName getServiceON(final ServiceReference serviceReference) { if (writable) { return ObjectNameUtil.createTransactionServiceON(serviceReferenceRegistrator.getNullableTransactionName(), serviceReference.getServiceInterfaceQName(), serviceReference.getRefName()); @@ -459,12 +460,12 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } @Override - public synchronized void removeServiceReference(String serviceInterfaceName, String refName) throws InstanceNotFoundException{ + public synchronized void removeServiceReference(final String serviceInterfaceName, final String refName) throws InstanceNotFoundException{ ServiceReference serviceReference = new ServiceReference(serviceInterfaceName, refName); removeServiceReference(serviceReference); } - private synchronized void removeServiceReference(ServiceReference serviceReference) throws InstanceNotFoundException { + private synchronized void removeServiceReference(final ServiceReference serviceReference) throws InstanceNotFoundException { LOGGER.debug("Removing service reference {} from {}", serviceReference, this); assertWritable(); // is the qName known? @@ -496,7 +497,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } @Override - public synchronized boolean removeServiceReferences(ObjectName moduleObjectName) throws InstanceNotFoundException { + public synchronized boolean removeServiceReferences(final ObjectName moduleObjectName) throws InstanceNotFoundException { lookupRegistry.checkConfigBeanExists(moduleObjectName); String factoryName = ObjectNameUtil.getFactoryName(moduleObjectName); // check that service interface name exist @@ -505,7 +506,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe } - private boolean removeServiceReferences(ObjectName moduleObjectName, Set qNames) throws InstanceNotFoundException { + private boolean removeServiceReferences(final ObjectName moduleObjectName, final Set qNames) throws InstanceNotFoundException { ObjectNameUtil.checkType(moduleObjectName, ObjectNameUtil.TYPE_MODULE); assertWritable(); Set serviceReferencesLinkingTo = findServiceReferencesLinkingTo(moduleObjectName, qNames); @@ -515,7 +516,7 @@ public class ServiceReferenceRegistryImpl implements CloseableServiceReferenceRe return serviceReferencesLinkingTo.isEmpty() == false; } - private Set findServiceReferencesLinkingTo(ObjectName moduleObjectName, Set serviceInterfaceQNames) { + private Set findServiceReferencesLinkingTo(final ObjectName moduleObjectName, final Set serviceInterfaceQNames) { String factoryName = ObjectNameUtil.getFactoryName(moduleObjectName); if (serviceInterfaceQNames == null) { LOGGER.warn("Possible error in code: cannot find factoryName {} in {}, object name {}", factoryName, factoryNamesToQNames, moduleObjectName); diff --git a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/util/InterfacesHelper.java b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/util/InterfacesHelper.java index 5cb1513d9c..f4d732c65c 100644 --- a/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/util/InterfacesHelper.java +++ b/opendaylight/config/config-manager/src/main/java/org/opendaylight/controller/config/manager/impl/util/InterfacesHelper.java @@ -7,6 +7,7 @@ */ package org.opendaylight.controller.config.manager.impl.util; +import com.google.common.collect.ImmutableSet; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -18,7 +19,7 @@ import org.opendaylight.controller.config.api.annotations.ServiceInterfaceAnnota import org.opendaylight.controller.config.spi.Module; import org.opendaylight.controller.config.spi.ModuleFactory; -public class InterfacesHelper { +public final class InterfacesHelper { private InterfacesHelper() { } @@ -126,7 +127,7 @@ public class InterfacesHelper { for (ServiceInterfaceAnnotation sia: siAnnotations) { qNames.add(sia.value()); } - return Collections.unmodifiableSet(qNames); + return ImmutableSet.copyOf(qNames); } public static Set getServiceInterfaceAnnotations(final ModuleFactory factory) { -- 2.36.6