Repository: brooklyn-server Updated Branches: refs/heads/master 2fea6d1ba -> 664ee0ea7
BROOKLYN-528: same proxy class for all entities of type Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/a635dc03 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/a635dc03 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/a635dc03 Branch: refs/heads/master Commit: a635dc039123af948d9d7e95b71bf9db0b4678a1 Parents: 2fea6d1 Author: Aled Sage <[email protected]> Authored: Thu Aug 31 11:00:46 2017 +0100 Committer: Aled Sage <[email protected]> Committed: Thu Aug 31 11:00:46 2017 +0100 ---------------------------------------------------------------------- .../core/objs/proxy/ClassLoaderCache.java | 121 +++++++++++++++++++ .../core/objs/proxy/InternalEntityFactory.java | 31 +---- .../core/objs/proxy/ClassLoaderCacheTest.java | 82 +++++++++++++ .../qa/performance/EntityPerformanceTest.java | 18 +++ 4 files changed, 224 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a635dc03/core/src/main/java/org/apache/brooklyn/core/objs/proxy/ClassLoaderCache.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/proxy/ClassLoaderCache.java b/core/src/main/java/org/apache/brooklyn/core/objs/proxy/ClassLoaderCache.java new file mode 100644 index 0000000..efdc811 --- /dev/null +++ b/core/src/main/java/org/apache/brooklyn/core/objs/proxy/ClassLoaderCache.java @@ -0,0 +1,121 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.core.objs.proxy; + +import static com.google.common.base.Preconditions.checkNotNull; + +import java.util.Set; +import java.util.concurrent.ConcurrentMap; + +import org.apache.brooklyn.util.javalang.AggregateClassLoader; + +import com.google.common.base.Objects; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; + +/** + * For getting/creating {@link AggregateClassLoader} for an entity proxy. The returned class loader + * can be used when calling {@link java.lang.reflect.Proxy#newProxyInstance(ClassLoader, Class[], java.lang.reflect.InvocationHandler)}. + * + * The classloader is created with references to ClassLoaders for the given class and interfaces + * (including all super-classes and super-interfaces of those). Also see comment in + * {@link InternalEntityFactory#createEntityProxy(Iterable, org.apache.brooklyn.api.entity.Entity)}. + * + * If a classloader has already been created for the given class+interfaces, then it guarantees + * to return that same classloader instance. This is very important when using {@code newProxyInstance} + * because if a different ClassLoader instance is used then it will reflectively create a new Proxy class. + * See https://issues.apache.org/jira/browse/BROOKLYN-528. + */ +class ClassLoaderCache { + + // TODO Should TypesKey use WeakReferences for the classes (e.g. so that if bundle is unloaded, + // karaf can eventually get rid of it)? But not as simple as that - the map's value + // (the AggregateClassLoader) will also reference the class. + + private final ConcurrentMap<TypesKey, AggregateClassLoader> cache = Maps.newConcurrentMap(); + + public AggregateClassLoader getClassLoaderForProxy(Class<?> clazz, Set<Class<?>> interfaces) { + TypesKey typesKey = new TypesKey(clazz, interfaces); + AggregateClassLoader result = cache.get(typesKey); + if (result == null) { + result = newClassLoader(clazz, interfaces); + AggregateClassLoader oldVal = cache.putIfAbsent(typesKey, result); + if (oldVal != null) result = oldVal; + } + return result; + } + + private AggregateClassLoader newClassLoader(Class<?> clazz, Set<Class<?>> interfaces) { + Set<ClassLoader> loaders = Sets.newLinkedHashSet(); + addClassLoaders(clazz, loaders); + for (Class<?> iface : interfaces) { + loaders.add(iface.getClassLoader()); + } + + AggregateClassLoader aggregateClassLoader = AggregateClassLoader.newInstanceWithNoLoaders(); + for (ClassLoader cl : loaders) { + aggregateClassLoader.addLast(cl); + } + + return aggregateClassLoader; + } + + private void addClassLoaders(Class<?> type, Set<ClassLoader> loaders) { + ClassLoader cl = type.getClassLoader(); + + //java.lang.Object.getClassLoader() == null + if (cl != null) { + loaders.add(cl); + } + + Class<?> superType = type.getSuperclass(); + if (superType != null) { + addClassLoaders(superType, loaders); + } + for (Class<?> iface : type.getInterfaces()) { + addClassLoaders(iface, loaders); + } + } + + /** + * Uses weak references for the class/interface references. + */ + private static class TypesKey { + final Class<?> type; + final Set<Class<?>> interfaces; + + public TypesKey(Class<?> type, Set<Class<?>> interfaces) { + this.type = checkNotNull(type, "type"); + this.interfaces = ImmutableSet.copyOf(checkNotNull(interfaces, "interfaces")); + } + + @Override + public int hashCode() { + return Objects.hashCode(type, interfaces); + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof TypesKey)) return false; + TypesKey o = (TypesKey) obj; + return Objects.equal(type, o.type) && Objects.equal(interfaces, o.interfaces); + } + } +} http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a635dc03/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java b/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java index e3ca216..a14225b 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/proxy/InternalEntityFactory.java @@ -22,7 +22,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import java.lang.reflect.InvocationTargetException; -import java.util.Collection; import java.util.Map; import java.util.Queue; import java.util.Set; @@ -90,11 +89,13 @@ public class InternalEntityFactory extends InternalFactory { private final EntityTypeRegistry entityTypeRegistry; private final InternalPolicyFactory policyFactory; + private final ClassLoaderCache classLoaderCache; public InternalEntityFactory(ManagementContextInternal managementContext, EntityTypeRegistry entityTypeRegistry, InternalPolicyFactory policyFactory) { super(managementContext); this.entityTypeRegistry = checkNotNull(entityTypeRegistry, "entityTypeRegistry"); this.policyFactory = checkNotNull(policyFactory, "policyFactory"); + this.classLoaderCache = new ClassLoaderCache(); } @VisibleForTesting @@ -132,16 +133,7 @@ public class InternalEntityFactory extends InternalFactory { // referenced from the entity and its interfaces with the single passed loader // while a normal class loading would nest the class loaders (loading interfaces' // references with their own class loaders which in our case are different). - Collection<ClassLoader> loaders = Sets.newLinkedHashSet(); - addClassLoaders(entity.getClass(), loaders); - for (Class<?> iface : allInterfaces) { - loaders.add(iface.getClassLoader()); - } - - AggregateClassLoader aggregateClassLoader = AggregateClassLoader.newInstanceWithNoLoaders(); - for (ClassLoader cl : loaders) { - aggregateClassLoader.addLast(cl); - } + AggregateClassLoader aggregateClassLoader = classLoaderCache.getClassLoaderForProxy(entity.getClass(), allInterfaces); return (T) java.lang.reflect.Proxy.newProxyInstance( aggregateClassLoader, @@ -149,23 +141,6 @@ public class InternalEntityFactory extends InternalFactory { new EntityProxyImpl(entity)); } - private void addClassLoaders(Class<?> type, Collection<ClassLoader> loaders) { - ClassLoader cl = type.getClassLoader(); - - //java.lang.Object.getClassLoader() = null - if (cl != null) { - loaders.add(cl); - } - - Class<?> superType = type.getSuperclass(); - if (superType != null) { - addClassLoaders(superType, loaders); - } - for (Class<?> iface : type.getInterfaces()) { - addClassLoaders(iface, loaders); - } - } - /** creates a new entity instance from a spec, with all children, policies, etc, * fully initialized ({@link AbstractEntity#init()} invoked) and ready for * management -- commonly the caller will next call http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a635dc03/core/src/test/java/org/apache/brooklyn/core/objs/proxy/ClassLoaderCacheTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/objs/proxy/ClassLoaderCacheTest.java b/core/src/test/java/org/apache/brooklyn/core/objs/proxy/ClassLoaderCacheTest.java new file mode 100644 index 0000000..5f2a4a4 --- /dev/null +++ b/core/src/test/java/org/apache/brooklyn/core/objs/proxy/ClassLoaderCacheTest.java @@ -0,0 +1,82 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.core.objs.proxy; + +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertNotSame; +import static org.testng.Assert.assertSame; + +import java.util.Set; + +import org.apache.brooklyn.api.entity.Entity; +import org.apache.brooklyn.core.entity.EntityInternal; +import org.apache.brooklyn.core.test.entity.TestEntity; +import org.apache.brooklyn.core.test.entity.TestEntityImpl; +import org.apache.brooklyn.entity.stock.BasicEntity; +import org.apache.brooklyn.entity.stock.BasicEntityImpl; +import org.apache.brooklyn.util.javalang.AggregateClassLoader; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; + +public class ClassLoaderCacheTest { + + private ClassLoaderCache cache; + + @BeforeMethod(alwaysRun=true) + public void setUp() throws Exception { + cache = new ClassLoaderCache(); + } + + @Test + public void testSameLoader() throws Exception { + Class<BasicEntityImpl> clazz = BasicEntityImpl.class; + ImmutableSet<Class<? extends Object>> interfaces = ImmutableSet.of(EntityProxy.class, Entity.class, EntityInternal.class, BasicEntity.class); + + AggregateClassLoader loader = cache.getClassLoaderForProxy(clazz, interfaces); + assertLoader(loader, Iterables.concat(ImmutableList.of(clazz), interfaces)); + + AggregateClassLoader loader2 = cache.getClassLoaderForProxy(clazz, interfaces); + assertSame(loader, loader2); + } + + @Test + public void testDifferentLoader() throws Exception { + Class<BasicEntityImpl> clazz = BasicEntityImpl.class; + Set<Class<?>> interfaces = ImmutableSet.of(EntityProxy.class, Entity.class, EntityInternal.class, BasicEntity.class); + AggregateClassLoader loader = cache.getClassLoaderForProxy(clazz, interfaces); + assertLoader(loader, Iterables.concat(ImmutableList.of(clazz), interfaces)); + + Class<TestEntityImpl> clazz2 = TestEntityImpl.class; + Set<Class<?>> interfaces2 = ImmutableSet.of(EntityProxy.class, Entity.class, EntityInternal.class, TestEntity.class); + AggregateClassLoader loader2 = cache.getClassLoaderForProxy(clazz2, interfaces2); + assertLoader(loader2, Iterables.concat(ImmutableList.of(clazz2), interfaces2)); + assertNotSame(loader, loader2); + } + + private void assertLoader(ClassLoader loader, Iterable<? extends Class<?>> clazzes) throws Exception { + assertNotNull(loader); + for (Class<?> clazz : clazzes) { + loader.loadClass(clazz.getName()); + } + } +} http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/a635dc03/core/src/test/java/org/apache/brooklyn/core/test/qa/performance/EntityPerformanceTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/test/qa/performance/EntityPerformanceTest.java b/core/src/test/java/org/apache/brooklyn/core/test/qa/performance/EntityPerformanceTest.java index 115d835..3fb8c4f 100644 --- a/core/src/test/java/org/apache/brooklyn/core/test/qa/performance/EntityPerformanceTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/test/qa/performance/EntityPerformanceTest.java @@ -29,6 +29,7 @@ import org.apache.brooklyn.api.sensor.SensorEvent; import org.apache.brooklyn.api.sensor.SensorEventListener; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.test.entity.TestEntity; +import org.apache.brooklyn.entity.stock.BasicApplication; import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.test.performance.PerformanceTestDescriptor; import org.apache.brooklyn.util.collections.MutableMap; @@ -67,6 +68,23 @@ public class EntityPerformanceTest extends AbstractPerformanceTest { return 1000; } + @Test(groups={"Integration", "Acceptance"}) + public void testCreateAndDeleteApp() { + int numIterations = numIterations(); + double minRatePerSec = 100 * PERFORMANCE_EXPECTATION; + + measure(PerformanceTestDescriptor.create() + .summary("EntityPerformanceTest.testUpdateAttributeWhenNoListeners") + .iterations(numIterations) + .minAcceptablePerSecond(minRatePerSec) + .job(new Runnable() { + @Override + public void run() { + BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class)); + app.stop(); + }})); + } + @Test(groups={"Integration", "Acceptance"}) public void testUpdateAttributeWhenNoListeners() { int numIterations = numIterations();
