modules/core/plugin-container/pom.xml | 5 modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java | 75 ++++++---- modules/core/plugin-container/src/test/java/org/rhq/core/pc/bundle/BundleManagerTest.java | 2 modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftManagerTest.java | 2 modules/enterprise/agent/ant-run.xml | 18 +- 5 files changed, 68 insertions(+), 34 deletions(-)
New commits: commit 81b37adc3805e8eb446c1f040e0a2e996740da44 Author: Heiko W. Rupp hwr@redhat.com Date: Wed Dec 4 11:40:57 2013 +0100
BZ 997670 - Do not go linearly through the list of resources to find by id, but use a map.
Using a TroveIntObjectMap saves ~1MB heap for an inventory of 40k resources opposed to a standard HashMap
diff --git a/modules/core/plugin-container/pom.xml b/modules/core/plugin-container/pom.xml index f2e6732..426e097 100644 --- a/modules/core/plugin-container/pom.xml +++ b/modules/core/plugin-container/pom.xml @@ -84,6 +84,11 @@ <artifactId>commons-io</artifactId> </dependency>
+ <dependency> + <groupId>net.sf.trove4j</groupId> + <artifactId>trove4j</artifactId> + <version>3.0.3</version> + </dependency> </dependencies>
<build> diff --git a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java index 2e4d52a..5b01a26 100644 --- a/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java +++ b/modules/core/plugin-container/src/main/java/org/rhq/core/pc/inventory/InventoryManager.java @@ -46,6 +46,9 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReentrantReadWriteLock;
+import gnu.trove.map.TIntObjectMap; +import gnu.trove.map.hash.TIntObjectHashMap; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.jetbrains.annotations.NotNull; @@ -195,7 +198,12 @@ public class InventoryManager extends AgentService implements ContainerService, /** * UUID to ResourceContainer map */ - private final Map<String, ResourceContainer> resourceContainers = new ConcurrentHashMap<String, ResourceContainer>(100); + private final Map<String, ResourceContainer> resourceContainersByUUID = new ConcurrentHashMap<String, ResourceContainer>(500); + + /** + * ResourceID to ResourceContainer map + */ + private final TIntObjectMap<ResourceContainer> resourceContainerByResourceId = new TIntObjectHashMap<ResourceContainer>(500);
/** * Collection of event listeners to inform of changes to the inventory. @@ -300,7 +308,8 @@ public class InventoryManager extends AgentService implements ContainerService, this.discoveryComponentProxyFactory.shutdown(); this.availabilityCollectors.shutdown(); this.inventoryEventListeners.clear(); - this.resourceContainers.clear(); + this.resourceContainersByUUID.clear(); + this.resourceContainerByResourceId.clear(); }
/** @@ -545,13 +554,13 @@ public class InventoryManager extends AgentService implements ContainerService,
@Nullable public ResourceContainer getResourceContainer(String uuid) { - return this.resourceContainers.get(uuid); + return this.resourceContainersByUUID.get(uuid); }
@Nullable public ResourceContainer getResourceContainer(CanonicalResourceKey canonicalId) { ResourceContainer resourceContainer = null; - for (Map.Entry<String, ResourceContainer> entry : resourceContainers.entrySet()) { + for (Map.Entry<String, ResourceContainer> entry : resourceContainersByUUID.entrySet()) { ResourceContainer container = entry.getValue(); Resource resource = container.getResource(); if (resource != null) { @@ -577,12 +586,12 @@ public class InventoryManager extends AgentService implements ContainerService, String uuid = resource.getUuid(); if (uuid == null) return null; - return this.resourceContainers.get(uuid); + return this.resourceContainersByUUID.get(uuid); }
@Nullable - public ResourceContainer getResourceContainer(Integer resourceId) { - if ((resourceId == null) || (resourceId == 0)) { + public ResourceContainer getResourceContainer(int resourceId) { + if (resourceId == 0) { // I've already found one place where passing in 0 was very bad - I want to be very noisy in the log // when this happens but not throw an exception, for fear I might break something. // I'll just return null instead; hopefully, callers are checking for null appropriately. @@ -596,10 +605,18 @@ public class InventoryManager extends AgentService implements ContainerService, return null; }
+ ResourceContainer resourceContainer = this.resourceContainerByResourceId.get(resourceId); + if (resourceContainer != null) { + return resourceContainer; + } + // We did not find the UUID in above map. + // Check on the classical way if the container is present and populate the + // resourceId -> resourceContainer map for the next call into this method. ResourceContainer retContainer = null; - for (ResourceContainer container : resourceContainers.values()) { - if (resourceId.equals(container.getResource().getId())) { + for (ResourceContainer container : resourceContainersByUUID.values()) { + if (resourceId == container.getResource().getId()) { retContainer = container; + this.resourceContainerByResourceId.put(resourceId, retContainer); break; } } @@ -1340,7 +1357,7 @@ public class InventoryManager extends AgentService implements ContainerService,
@Nullable public ResourceComponent<?> getResourceComponent(Resource resource) { - ResourceContainer resourceContainer = this.resourceContainers.get(resource.getUuid()); + ResourceContainer resourceContainer = this.resourceContainersByUUID.get(resource.getUuid());
if (resourceContainer == null) { return null; @@ -1410,12 +1427,15 @@ public class InventoryManager extends AgentService implements ContainerService, PluginContainer.getInstance().getMeasurementManager() .unscheduleCollection(Collections.singleton(resource.getId()));
- if (this.resourceContainers.remove(resource.getUuid()) == null) { + if (this.resourceContainersByUUID.remove(resource.getUuid()) == null) { if (log.isDebugEnabled()) { log.debug("Asked to remove an unknown Resource [" + resource + "] with UUID [" + resource.getUuid() + "]"); } } + else { + this.resourceContainerByResourceId.remove(resource.getId()); + }
// Notify InventoryEventListeners a Resource has been removed. fireResourcesRemoved(Collections.singleton(resource)); @@ -1509,7 +1529,7 @@ public class InventoryManager extends AgentService implements ContainerService, * @return */ public Availability updateAvailability(Resource resource, AvailabilityType availabilityType) { - ResourceContainer resourceContainer = this.resourceContainers.get(resource.getUuid()); + ResourceContainer resourceContainer = resourceContainersByUUID.get(resource.getUuid()); return resourceContainer.updateAvailability(availabilityType); }
@@ -1657,7 +1677,7 @@ public class InventoryManager extends AgentService implements ContainerService, List<Resource> activatedResources = new ArrayList<Resource>(); this.inventoryLock.readLock().lock(); try { - for (ResourceContainer container : this.resourceContainers.values()) { + for (ResourceContainer container : this.resourceContainersByUUID.values()) { if ((container != null) && (container.getResourceComponentState() == ResourceComponentState.STARTED)) { activatedResources.add(container.getResource()); } @@ -1687,7 +1707,8 @@ public class InventoryManager extends AgentService implements ContainerService, // Auto-sync if the PC is running within the embedded JBossAS console. resourceContainer.setSynchronizationState(ResourceContainer.SynchronizationState.SYNCHRONIZED); } - this.resourceContainers.put(resource.getUuid(), resourceContainer); + this.resourceContainersByUUID.put(resource.getUuid().intern(), resourceContainer); + this.resourceContainerByResourceId.put(resource.getId(), resourceContainer); } else { // container already exists, but make sure the classloader exists too if (resourceContainer.getResourceClassLoader() == null) { @@ -2144,18 +2165,22 @@ public class InventoryManager extends AgentService implements ContainerService, inventoryFile.loadInventory();
this.platform = inventoryFile.getPlatform(); - this.resourceContainers.clear(); + this.resourceContainersByUUID.clear(); + this.resourceContainerByResourceId.clear(); for (String uuid : inventoryFile.getResourceContainers().keySet()) { ResourceContainer resourceContainer = inventoryFile.getResourceContainers().get(uuid); - this.resourceContainers.put(uuid, resourceContainer); + this.resourceContainersByUUID.put(uuid, resourceContainer); + Resource resource = resourceContainer.getResource(); + this.resourceContainerByResourceId.put(resource.getId(), resourceContainer); }
- log.info("Inventory with size [" + this.resourceContainers.size() + "] loaded from data file in [" + log.info("Inventory with size [" + this.resourceContainersByUUID.size() + "] loaded from data file in [" + (System.currentTimeMillis() - start) + "ms]"); } } catch (Exception e) { this.platform = null; - this.resourceContainers.clear(); + this.resourceContainersByUUID.clear(); + this.resourceContainerByResourceId.clear(); if (file != null) { file.renameTo(new File(file.getAbsolutePath() + ".invalid")); // move it out of the way if we can, retain it for later analysis } @@ -2215,7 +2240,7 @@ public class InventoryManager extends AgentService implements ContainerService, } File file = new File(dataDir, "inventory.dat"); InventoryFile inventoryFile = new InventoryFile(file); - inventoryFile.storeInventory(this.platform, this.resourceContainers); + inventoryFile.storeInventory(this.platform, this.resourceContainersByUUID); } catch (Exception e) { log.error("Could not persist inventory data to disk", e); } @@ -3209,9 +3234,11 @@ public class InventoryManager extends AgentService implements ContainerService, + resourceFromServer + ".");
// First grab the existing Resource's container, so we can reuse it. - resourceContainer = this.resourceContainers.remove(existingResource.getUuid()); + resourceContainer = this.resourceContainersByUUID.remove(existingResource.getUuid()); if (resourceContainer != null) { - this.resourceContainers.put(resourceFromServer.getUuid(), resourceContainer); + this.resourceContainerByResourceId.remove(existingResource.getId()); + this.resourceContainersByUUID.put(resourceFromServer.getUuid().intern(), resourceContainer); + this.resourceContainerByResourceId.put(resourceFromServer.getId(), resourceContainer); } else { log.error("No ResourceContainer found for existing " + existingResource + "."); return; @@ -3308,7 +3335,7 @@ public class InventoryManager extends AgentService implements ContainerService, private void purgeObsoleteResources(Set<String> allUuids) { // Remove previously synchronized Resources that no longer exist in the Server's inventory... log.debug("Purging obsolete Resources..."); - if (this.resourceContainers == null) { + if (this.resourceContainersByUUID == null) { log.debug("No containers present, immediately returning .."); return; } @@ -3320,10 +3347,10 @@ public class InventoryManager extends AgentService implements ContainerService, * can be called later without throwing ConcurrentModificationException */ Map<String, ResourceContainer> mapForIterating = new HashMap<String, ResourceContainer>( - this.resourceContainers); + this.resourceContainersByUUID); for (String uuid : mapForIterating.keySet()) { if (!allUuids.contains(uuid)) { - ResourceContainer resourceContainer = this.resourceContainers.get(uuid); + ResourceContainer resourceContainer = this.resourceContainersByUUID.get(uuid); if (resourceContainer != null) { Resource resource = resourceContainer.getResource(); // Only purge stuff that was synchronized at some point. Other stuff may just be newly discovered. diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/bundle/BundleManagerTest.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/bundle/BundleManagerTest.java index 6cb6874..3b2b477 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/bundle/BundleManagerTest.java +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/bundle/BundleManagerTest.java @@ -455,7 +455,7 @@ public class BundleManagerTest { }
@Override - public ResourceContainer getResourceContainer(Integer resourceId) { + public ResourceContainer getResourceContainer(int resourceId) { return idResourceContainerMap.get(resourceId); }
diff --git a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftManagerTest.java b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftManagerTest.java index e1ec00d..93832bf 100644 --- a/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftManagerTest.java +++ b/modules/core/plugin-container/src/test/java/org/rhq/core/pc/drift/DriftManagerTest.java @@ -393,7 +393,7 @@ public class DriftManagerTest extends DriftTest { private Map<Integer, ResourceContainer> resourceContainers = new HashMap<Integer, ResourceContainer>();
@Override - public ResourceContainer getResourceContainer(Integer resourceId) { + public ResourceContainer getResourceContainer(int resourceId) { ResourceContainer container = resourceContainers.get(resourceId); if (container == null) { Resource resource = new Resource(); diff --git a/modules/enterprise/agent/ant-run.xml b/modules/enterprise/agent/ant-run.xml index d6ebbfb..f2e6994 100644 --- a/modules/enterprise/agent/ant-run.xml +++ b/modules/enterprise/agent/ant-run.xml @@ -16,11 +16,12 @@ Ant Script that provides ways to package the Agent. <property name="augeas.classifier" value="el5"/> <property name="augeas.zip.location" value="${settings.localRepository}/net/augeas/augeas-native/${augeas.version}/"/> <property name="augeas.zip.mask" value="*-${augeas.classifier}.zip"/> - <property name="augeas.zip.version" value="0.9.0-4"/> + <property name="augeas.zip.version" value="0.9.0-4"/> <property name="getopt.version" value="1.0.13"/> <property name="jaxb-api.version" value="2.1"/> <property name="jaxb-impl.version" value="2.1"/> <property name="log4j.version" value="1.2.16"/> + <property name="trove.version" value="3.0.3"/> <property name="so.exts" value="*.so.*"/>
<!-- @@ -63,6 +64,7 @@ Ant Script that provides ways to package the Agent. <include name="org/rhq/rhq-core-client-api/${project.version}/rhq-core-client-api-${project.version}.jar" /> <include name="org/rhq/rhq-common-drift/${project.version}/rhq-common-drift-${project.version}.jar"/> <include name="commons-io/commons-io/1.4/commons-io-1.4.jar"/> + <include name="net/sf/trove4j/trove4j/${trove.version}/trove4j-${trove.version}.jar"/> </fileset>
</path> @@ -151,18 +153,18 @@ Ant Script that provides ways to package the Agent. <include name="${so.exts}" /> </fileset> </move> - + <move todir="${agent.distro.dir}/lib/augeas/lib64" flatten="true" overwrite="false"> <fileset dir="${agent.distro.dir}/lib/augeas-native-${augeas.zip.version}/lib64"> <include name="*.so" /> <include name="${so.exts}" /> </fileset> </move> - + <delete failonerror="false" includeEmptyDirs="true" verbose="true"> <fileset dir="${agent.distro.dir}/lib" includes="augeas-native-*/"/> - </delete> - + </delete> + <move todir="${agent.distro.dir}/lib" flatten="true" overwrite="false"> <fileset dir="${agent.distro.dir}/lib"> <include name="**/lib/*" /> @@ -170,14 +172,14 @@ Ant Script that provides ways to package the Agent. </fileset> </move> <move file="${agent.distro.dir}/lib/sigar.jar" tofile="${agent.distro.dir}/lib/sigar-${sigar.version}.jar" - overwrite="false"/> + overwrite="false"/> <delete failonerror="false" includeEmptyDirs="true" verbose="true"> <fileset dir="${agent.distro.dir}/lib" includes="hyperic-sigar-*/"/> - </delete> + </delete> <chmod dir="${agent.distro.dir}/lib" perm="ug+rx" includes="*.so,${so.exts},*.sl,*.dylib"/> <chmod dir="${agent.distro.dir}/lib/augeas/lib" perm="ug+rx" includes="*.so,${so.exts}"/> <chmod dir="${agent.distro.dir}/lib/augeas/lib64" perm="ug+rx" includes="*.so,${so.exts}"/> - + <copy todir="${agent.distro.dir}/bin"> <fileset dir="${basedir}/src/etc" includes="*.bat,*.sh"/>
rhq-commits@lists.fedorahosted.org