You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@isis.apache.org by Martin Grigorov <mg...@apache.org> on 2015/04/24 14:00:45 UTC
Re: [1/4] isis git commit: ISIS-1137: Now cache a ProxyFactory
instance by class (allowing reuse of the enhanced class for all instances of
that type) rather than creating a newly enhanced class as currently.
On Fri, Apr 24, 2015 at 1:15 PM, <da...@apache.org> wrote:
> Repository: isis
> Updated Branches:
> refs/heads/master 69d559486 -> ee834d9a3
>
>
> ISIS-1137: Now cache a ProxyFactory instance by class (allowing reuse of
> the enhanced class for all instances of that type) rather than creating a
> newly enhanced class as currently.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/isis/repo
> Commit: http://git-wip-us.apache.org/repos/asf/isis/commit/a4f924b1
> Tree: http://git-wip-us.apache.org/repos/asf/isis/tree/a4f924b1
> Diff: http://git-wip-us.apache.org/repos/asf/isis/diff/a4f924b1
>
> Branch: refs/heads/master
> Commit: a4f924b11cf614535e2faf2fb4f7aacdb4c7e1e0
> Parents: 69d5594
> Author: Dan Haywood <da...@haywood-associates.co.uk>
> Authored: Fri Apr 24 10:29:16 2015 +0100
> Committer: Dan Haywood <da...@haywood-associates.co.uk>
> Committed: Fri Apr 24 10:29:16 2015 +0100
>
> ----------------------------------------------------------------------
> .../isis/core/commons/lang/ArrayExtensions.java | 4 +-
> .../core/wrapper/WrapperFactoryAbstract.java | 6 +-
> .../proxy/ProxyInstantiatorForJavassist.java | 71 +++++++++++++++-----
> 3 files changed, 61 insertions(+), 20 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/isis/blob/a4f924b1/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> ----------------------------------------------------------------------
> diff --git
> a/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> b/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> index 25a6f88..df7238b 100644
> ---
> a/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> +++
> b/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> @@ -27,6 +27,8 @@ import java.util.Arrays;
> import java.util.Collection;
> import java.util.List;
>
> +import com.google.common.collect.Lists;
> +
> import org.apache.isis.core.commons.exceptions.IsisException;
>
> public final class ArrayExtensions {
> @@ -71,7 +73,7 @@ public final class ArrayExtensions {
> }
>
> public static <T> T[] combine(final T[]... arrays) {
> - final List<T> combinedList = new ArrayList<T>();
> + final List<T> combinedList = Lists.newArrayList();
> for (final T[] array : arrays) {
> for (final T t : array) {
> combinedList.add(t);
>
>
> http://git-wip-us.apache.org/repos/asf/isis/blob/a4f924b1/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> ----------------------------------------------------------------------
> diff --git
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> index ceea4a5..2d251ed 100644
> ---
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> +++
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> @@ -51,11 +51,11 @@ public abstract class WrapperFactoryAbstract
> implements WrapperFactory, Authenti
> private AdapterManager adapterManager;
> private ObjectPersistor objectPersistor;
>
> - private final ProxyContextHandler proxy;
> + private final ProxyContextHandler proxyContextHandler;
>
> public WrapperFactoryAbstract(final ProxyInstantiator
> proxyInstantiator) {
>
> - proxy = new ProxyContextHandler(proxyInstantiator);
> + proxyContextHandler = new ProxyContextHandler(proxyInstantiator);
>
> dispatchersByEventClass.put(ObjectTitleEvent.class, new
> InteractionEventDispatcherTypeSafe<ObjectTitleEvent>() {
> @Override
> @@ -221,7 +221,7 @@ public abstract class WrapperFactoryAbstract
> implements WrapperFactory, Authenti
> }
>
> protected <T> T createProxy(final T domainObject, final ExecutionMode
> mode) {
> - return proxy.proxy(domainObject, this, mode,
> getAuthenticationSessionProvider(), getSpecificationLookup(),
> getAdapterManager(), getObjectPersistor());
> + return proxyContextHandler.proxy(domainObject, this, mode,
> getAuthenticationSessionProvider(), getSpecificationLookup(),
> getAdapterManager(), getObjectPersistor());
> }
>
> @Override
>
>
> http://git-wip-us.apache.org/repos/asf/isis/blob/a4f924b1/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> ----------------------------------------------------------------------
> diff --git
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> index 3d2b838..cc0ee19 100644
> ---
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> +++
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> @@ -19,21 +19,38 @@
>
> package org.apache.isis.core.wrapper.proxy;
>
> -import java.lang.reflect.InvocationHandler;
> import java.lang.reflect.Method;
> +import java.util.Map;
>
> -import javassist.util.proxy.MethodFilter;
> -import javassist.util.proxy.MethodHandler;
> -import javassist.util.proxy.ProxyFactory;
> -import javassist.util.proxy.ProxyObject;
> +import com.google.common.collect.MapMaker;
>
> import org.apache.isis.applib.services.wrapper.WrapperObject;
> +import org.apache.isis.applib.services.wrapper.WrappingObject;
> import org.apache.isis.core.commons.lang.ArrayExtensions;
> +import
> org.apache.isis.core.metamodel.specloader.classsubstitutor.JavassistEnhanced;
> import org.apache.isis.core.wrapper.handlers.DelegatingInvocationHandler;
> import org.apache.isis.core.wrapper.internal.util.Util;
>
> +import javassist.util.proxy.MethodFilter;
> +import javassist.util.proxy.MethodHandler;
> +import javassist.util.proxy.Proxy;
> +import javassist.util.proxy.ProxyFactory;
> +
> public class ProxyInstantiatorForJavassist implements ProxyInstantiator {
>
> + /**
> + * Lazily constructed cache.
> + */
> + private final Map<Class, ProxyFactory> proxyFactoryByClass;
> +
>
> + public ProxyInstantiatorForJavassist() {
> + this(new MapMaker().concurrencyLevel(10).<Class,
> ProxyFactory>makeMap());
>
I think this should use weak keys to prevent classloader leaks and allow
class unloading.
> + }
> +
> + public ProxyInstantiatorForJavassist(Map<Class, ProxyFactory>
> proxyFactoryByClass) {
> + this.proxyFactoryByClass = proxyFactoryByClass;
> + }
> +
> @SuppressWarnings("unchecked")
> public <T> T instantiateProxy(final DelegatingInvocationHandler<T>
> handler) {
>
> @@ -44,24 +61,40 @@ public class ProxyInstantiatorForJavassist implements
> ProxyInstantiator {
> if (clazz.isInterface()) {
> return Util.createInstance(clazz, handler,
> WrapperObject.class);
> } else {
> - final Class<T> enhancedClass = createEnhancedClass(clazz,
> handler, WrapperObject.class);
> - ProxyObject proxyObject = (ProxyObject)
> Util.createInstance(enhancedClass);
> - proxyObject.setHandler(new MethodHandler() {
> + final ProxyFactory proxyFactory = proxyFactoryFor(clazz);
> +
> + final Class<T> enhancedClass = proxyFactory.createClass();
> + final Proxy proxy = (Proxy)
> Util.createInstance(enhancedClass);
> +
> + proxy.setHandler(new MethodHandler() {
> @Override
> public Object invoke(Object self, Method thisMethod,
> Method proceed, Object[] args) throws Throwable {
> return handler.invoke(self, thisMethod, args);
> }
> });
> - return (T) proxyObject;
> +
> + return (T) proxy;
> }
> }
> -
> - @SuppressWarnings("unchecked")
> - private static <T> Class<T> createEnhancedClass(final Class<T>
> toProxyClass, final InvocationHandler handler, final Class<?>...
> auxiliaryTypes) {
> -
> +
> + private <T> ProxyFactory proxyFactoryFor(final Class<T> toProxyClass)
> {
> + ProxyFactory proxyFactory = proxyFactoryByClass.get(toProxyClass);
> + if(proxyFactory == null) {
> + proxyFactory = createProxyFactoryFor(toProxyClass);
> + proxyFactoryByClass.put(toProxyClass, proxyFactory);
> + }
> + return proxyFactory;
> + }
> +
> + private <T> ProxyFactory createProxyFactoryFor(final Class<T>
> toProxyClass) {
> +
> final ProxyFactory proxyFactory = new ProxyFactory();
> +
> proxyFactory.setSuperclass(toProxyClass);
> -
> proxyFactory.setInterfaces(ArrayExtensions.combine(toProxyClass.getInterfaces(),
> new
> Class<?>[]{org.apache.isis.core.metamodel.specloader.classsubstitutor.JavassistEnhanced.class},
> auxiliaryTypes));
> + final Class[] types = ArrayExtensions.combine(
> + toProxyClass.getInterfaces(),
> + new Class<?>[] { JavassistEnhanced.class,
> WrappingObject.class });
> + proxyFactory.setInterfaces(types);
>
> proxyFactory.setFilter(new MethodFilter() {
> @Override
> @@ -70,8 +103,14 @@ public class ProxyInstantiatorForJavassist implements
> ProxyInstantiator {
> return !m.getName().equals("finalize") || m.isBridge();
> }
> });
> -
> - return proxyFactory.createClass();
> +
> + // this is the default, I believe
> + // calling it here only because I know that it will throw an
> exception if the code were
> + // in the future changed such that caching is invalid
> + // (ie fail fast if future change could introduce a bug)
> + proxyFactory.setUseCache(true);
> +
> + return proxyFactory;
> }
>
> }
>
>
Re: [1/4] isis git commit: ISIS-1137: Now cache a ProxyFactory
instance by class (allowing reuse of the enhanced class for all instances of
that type) rather than creating a newly enhanced class as currently.
Posted by Dan Haywood <da...@haywood-associates.co.uk>.
OK, cool. As you saw, I've committed that change to use weak keys.
And thanks for these hints on the JVM args; I'll add them to our new
documentation.
Cheers
Dan
On 24 April 2015 at 13:42, Martin Grigorov <mg...@apache.org> wrote:
> Hi Dan,
>
> On Fri, Apr 24, 2015 at 3:13 PM, Dan Haywood <dan@haywood-associates.co.uk
> >
> wrote:
>
> > Hi Martin,
> >
> > thx for the input. I did consider this, but realized that I don't know
> > about the interaction of class(un)loading vs GC.
> >
> > My understanding (correct me if I'm wrong) is that GC daemon will quite
> > eagerly clean up any objects if there are only weak references to that
> > object. So I was worried about our ProxyFactory instances - that we want
> > to hold on to for as long as there are any instances of the class being
> > enhanced - might get GC'd away.
> >
> > But thinking a bit deeper, I suspect that the above sentence is
> nonsense...
> > if we have weak keys for the Class', then of course the container itself
> > will have strong references to those classes. And, as you say, only if
> the
> >
>
> Right.
> The ClassLoader that loaded the Class has a reference to it, so the Class
> will be eligible for GC only if the ClassLoader is unloaded/GC-ed.
> The ClassLoader could not be unloaded if its Class-es are strong referenced
> by something else. This is what I meant by "Class Loader leak".
> By using weak key you say "I'm fine to remove this entry if the key is not
> referenced anywhere else.
>
> Additionally by using "-XX:+CMSClassUnloadingEnabled" (with
> -XX:+UseConcMarkSweepGC) you can hint the JVM to unload dynamically created
> classes which ClassLoaders are still around.
>
>
> > container tries to unload the Class then the map won't be "strong enough"
> > to hold on.
> >
> > Correct me if I'm wrong; but if that's what you were thinking, then I
> > agree.
> >
> > I'll make the change.
> >
> > Thx again
> > Dan
> >
> >
> >
> > On 24 April 2015 at 13:00, Martin Grigorov <mg...@apache.org> wrote:
> >
> > > On Fri, Apr 24, 2015 at 1:15 PM, <da...@apache.org> wrote:
> > >
> > > > Repository: isis
> > > > Updated Branches:
> > > > refs/heads/master 69d559486 -> ee834d9a3
> > > >
> > > >
> > > > ISIS-1137: Now cache a ProxyFactory instance by class (allowing reuse
> > of
> > > > the enhanced class for all instances of that type) rather than
> > creating a
> > > > newly enhanced class as currently.
> > > >
> > > >
> > > > Project: http://git-wip-us.apache.org/repos/asf/isis/repo
> > > > Commit: http://git-wip-us.apache.org/repos/asf/isis/commit/a4f924b1
> > > > Tree: http://git-wip-us.apache.org/repos/asf/isis/tree/a4f924b1
> > > > Diff: http://git-wip-us.apache.org/repos/asf/isis/diff/a4f924b1
> > > >
> > > > Branch: refs/heads/master
> > > > Commit: a4f924b11cf614535e2faf2fb4f7aacdb4c7e1e0
> > > > Parents: 69d5594
> > > > Author: Dan Haywood <da...@haywood-associates.co.uk>
> > > > Authored: Fri Apr 24 10:29:16 2015 +0100
> > > > Committer: Dan Haywood <da...@haywood-associates.co.uk>
> > > > Committed: Fri Apr 24 10:29:16 2015 +0100
> > > >
> > > >
> ----------------------------------------------------------------------
> > > > .../isis/core/commons/lang/ArrayExtensions.java | 4 +-
> > > > .../core/wrapper/WrapperFactoryAbstract.java | 6 +-
> > > > .../proxy/ProxyInstantiatorForJavassist.java | 71
> > > +++++++++++++++-----
> > > > 3 files changed, 61 insertions(+), 20 deletions(-)
> > > >
> ----------------------------------------------------------------------
> > > >
> > > >
> > > >
> > > >
> > >
> >
> http://git-wip-us.apache.org/repos/asf/isis/blob/a4f924b1/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > > >
> ----------------------------------------------------------------------
> > > > diff --git
> > > >
> > >
> >
> a/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > > >
> > >
> >
> b/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > > > index 25a6f88..df7238b 100644
> > > > ---
> > > >
> > >
> >
> a/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > > > +++
> > > >
> > >
> >
> b/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > > > @@ -27,6 +27,8 @@ import java.util.Arrays;
> > > > import java.util.Collection;
> > > > import java.util.List;
> > > >
> > > > +import com.google.common.collect.Lists;
> > > > +
> > > > import org.apache.isis.core.commons.exceptions.IsisException;
> > > >
> > > > public final class ArrayExtensions {
> > > > @@ -71,7 +73,7 @@ public final class ArrayExtensions {
> > > > }
> > > >
> > > > public static <T> T[] combine(final T[]... arrays) {
> > > > - final List<T> combinedList = new ArrayList<T>();
> > > > + final List<T> combinedList = Lists.newArrayList();
> > > > for (final T[] array : arrays) {
> > > > for (final T t : array) {
> > > > combinedList.add(t);
> > > >
> > > >
> > > >
> > >
> >
> http://git-wip-us.apache.org/repos/asf/isis/blob/a4f924b1/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > > >
> ----------------------------------------------------------------------
> > > > diff --git
> > > >
> > >
> >
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > > >
> > >
> >
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > > > index ceea4a5..2d251ed 100644
> > > > ---
> > > >
> > >
> >
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > > > +++
> > > >
> > >
> >
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > > > @@ -51,11 +51,11 @@ public abstract class WrapperFactoryAbstract
> > > > implements WrapperFactory, Authenti
> > > > private AdapterManager adapterManager;
> > > > private ObjectPersistor objectPersistor;
> > > >
> > > > - private final ProxyContextHandler proxy;
> > > > + private final ProxyContextHandler proxyContextHandler;
> > > >
> > > > public WrapperFactoryAbstract(final ProxyInstantiator
> > > > proxyInstantiator) {
> > > >
> > > > - proxy = new ProxyContextHandler(proxyInstantiator);
> > > > + proxyContextHandler = new
> > > ProxyContextHandler(proxyInstantiator);
> > > >
> > > > dispatchersByEventClass.put(ObjectTitleEvent.class, new
> > > > InteractionEventDispatcherTypeSafe<ObjectTitleEvent>() {
> > > > @Override
> > > > @@ -221,7 +221,7 @@ public abstract class WrapperFactoryAbstract
> > > > implements WrapperFactory, Authenti
> > > > }
> > > >
> > > > protected <T> T createProxy(final T domainObject, final
> > > ExecutionMode
> > > > mode) {
> > > > - return proxy.proxy(domainObject, this, mode,
> > > > getAuthenticationSessionProvider(), getSpecificationLookup(),
> > > > getAdapterManager(), getObjectPersistor());
> > > > + return proxyContextHandler.proxy(domainObject, this, mode,
> > > > getAuthenticationSessionProvider(), getSpecificationLookup(),
> > > > getAdapterManager(), getObjectPersistor());
> > > > }
> > > >
> > > > @Override
> > > >
> > > >
> > > >
> > >
> >
> http://git-wip-us.apache.org/repos/asf/isis/blob/a4f924b1/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > > >
> ----------------------------------------------------------------------
> > > > diff --git
> > > >
> > >
> >
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > > >
> > >
> >
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > > > index 3d2b838..cc0ee19 100644
> > > > ---
> > > >
> > >
> >
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > > > +++
> > > >
> > >
> >
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > > > @@ -19,21 +19,38 @@
> > > >
> > > > package org.apache.isis.core.wrapper.proxy;
> > > >
> > > > -import java.lang.reflect.InvocationHandler;
> > > > import java.lang.reflect.Method;
> > > > +import java.util.Map;
> > > >
> > > > -import javassist.util.proxy.MethodFilter;
> > > > -import javassist.util.proxy.MethodHandler;
> > > > -import javassist.util.proxy.ProxyFactory;
> > > > -import javassist.util.proxy.ProxyObject;
> > > > +import com.google.common.collect.MapMaker;
> > > >
> > > > import org.apache.isis.applib.services.wrapper.WrapperObject;
> > > > +import org.apache.isis.applib.services.wrapper.WrappingObject;
> > > > import org.apache.isis.core.commons.lang.ArrayExtensions;
> > > > +import
> > > >
> > >
> >
> org.apache.isis.core.metamodel.specloader.classsubstitutor.JavassistEnhanced;
> > > > import
> > > org.apache.isis.core.wrapper.handlers.DelegatingInvocationHandler;
> > > > import org.apache.isis.core.wrapper.internal.util.Util;
> > > >
> > > > +import javassist.util.proxy.MethodFilter;
> > > > +import javassist.util.proxy.MethodHandler;
> > > > +import javassist.util.proxy.Proxy;
> > > > +import javassist.util.proxy.ProxyFactory;
> > > > +
> > > > public class ProxyInstantiatorForJavassist implements
> > ProxyInstantiator
> > > {
> > > >
> > > > + /**
> > > > + * Lazily constructed cache.
> > > > + */
> > > > + private final Map<Class, ProxyFactory> proxyFactoryByClass;
> > > > +
> > > >
> > >
> > >
> > >
> > > > + public ProxyInstantiatorForJavassist() {
> > > > + this(new MapMaker().concurrencyLevel(10).<Class,
> > > > ProxyFactory>makeMap());
> > > >
> > >
> > > I think this should use weak keys to prevent classloader leaks and
> allow
> > > class unloading.
> > >
> > >
> > > > + }
> > > > +
> > > > + public ProxyInstantiatorForJavassist(Map<Class, ProxyFactory>
> > > > proxyFactoryByClass) {
> > > > + this.proxyFactoryByClass = proxyFactoryByClass;
> > > > + }
> > > > +
> > > > @SuppressWarnings("unchecked")
> > > > public <T> T instantiateProxy(final
> DelegatingInvocationHandler<T>
> > > > handler) {
> > > >
> > > > @@ -44,24 +61,40 @@ public class ProxyInstantiatorForJavassist
> > implements
> > > > ProxyInstantiator {
> > > > if (clazz.isInterface()) {
> > > > return Util.createInstance(clazz, handler,
> > > > WrapperObject.class);
> > > > } else {
> > > > - final Class<T> enhancedClass =
> createEnhancedClass(clazz,
> > > > handler, WrapperObject.class);
> > > > - ProxyObject proxyObject = (ProxyObject)
> > > > Util.createInstance(enhancedClass);
> > > > - proxyObject.setHandler(new MethodHandler() {
> > > > + final ProxyFactory proxyFactory =
> proxyFactoryFor(clazz);
> > > > +
> > > > + final Class<T> enhancedClass =
> proxyFactory.createClass();
> > > > + final Proxy proxy = (Proxy)
> > > > Util.createInstance(enhancedClass);
> > > > +
> > > > + proxy.setHandler(new MethodHandler() {
> > > > @Override
> > > > public Object invoke(Object self, Method thisMethod,
> > > > Method proceed, Object[] args) throws Throwable {
> > > > return handler.invoke(self, thisMethod, args);
> > > > }
> > > > });
> > > > - return (T) proxyObject;
> > > > +
> > > > + return (T) proxy;
> > > > }
> > > > }
> > > > -
> > > > - @SuppressWarnings("unchecked")
> > > > - private static <T> Class<T> createEnhancedClass(final Class<T>
> > > > toProxyClass, final InvocationHandler handler, final Class<?>...
> > > > auxiliaryTypes) {
> > > > -
> > > > +
> > > > + private <T> ProxyFactory proxyFactoryFor(final Class<T>
> > > toProxyClass)
> > > > {
> > > > + ProxyFactory proxyFactory =
> > > proxyFactoryByClass.get(toProxyClass);
> > > > + if(proxyFactory == null) {
> > > > + proxyFactory = createProxyFactoryFor(toProxyClass);
> > > > + proxyFactoryByClass.put(toProxyClass, proxyFactory);
> > > > + }
> > > > + return proxyFactory;
> > > > + }
> > > > +
> > > > + private <T> ProxyFactory createProxyFactoryFor(final Class<T>
> > > > toProxyClass) {
> > > > +
> > > > final ProxyFactory proxyFactory = new ProxyFactory();
> > > > +
> > > > proxyFactory.setSuperclass(toProxyClass);
> > > > -
> > > >
> > >
> >
> proxyFactory.setInterfaces(ArrayExtensions.combine(toProxyClass.getInterfaces(),
> > > > new
> > > >
> > >
> >
> Class<?>[]{org.apache.isis.core.metamodel.specloader.classsubstitutor.JavassistEnhanced.class},
> > > > auxiliaryTypes));
> > > > + final Class[] types = ArrayExtensions.combine(
> > > > + toProxyClass.getInterfaces(),
> > > > + new Class<?>[] { JavassistEnhanced.class,
> > > > WrappingObject.class });
> > > > + proxyFactory.setInterfaces(types);
> > > >
> > > > proxyFactory.setFilter(new MethodFilter() {
> > > > @Override
> > > > @@ -70,8 +103,14 @@ public class ProxyInstantiatorForJavassist
> > implements
> > > > ProxyInstantiator {
> > > > return !m.getName().equals("finalize") ||
> > m.isBridge();
> > > > }
> > > > });
> > > > -
> > > > - return proxyFactory.createClass();
> > > > +
> > > > + // this is the default, I believe
> > > > + // calling it here only because I know that it will throw an
> > > > exception if the code were
> > > > + // in the future changed such that caching is invalid
> > > > + // (ie fail fast if future change could introduce a bug)
> > > > + proxyFactory.setUseCache(true);
> > > > +
> > > > + return proxyFactory;
> > > > }
> > > >
> > > > }
> > > >
> > > >
> > >
> >
>
Re: [1/4] isis git commit: ISIS-1137: Now cache a ProxyFactory
instance by class (allowing reuse of the enhanced class for all instances of
that type) rather than creating a newly enhanced class as currently.
Posted by Martin Grigorov <mg...@apache.org>.
Hi Dan,
On Fri, Apr 24, 2015 at 3:13 PM, Dan Haywood <da...@haywood-associates.co.uk>
wrote:
> Hi Martin,
>
> thx for the input. I did consider this, but realized that I don't know
> about the interaction of class(un)loading vs GC.
>
> My understanding (correct me if I'm wrong) is that GC daemon will quite
> eagerly clean up any objects if there are only weak references to that
> object. So I was worried about our ProxyFactory instances - that we want
> to hold on to for as long as there are any instances of the class being
> enhanced - might get GC'd away.
>
> But thinking a bit deeper, I suspect that the above sentence is nonsense...
> if we have weak keys for the Class', then of course the container itself
> will have strong references to those classes. And, as you say, only if the
>
Right.
The ClassLoader that loaded the Class has a reference to it, so the Class
will be eligible for GC only if the ClassLoader is unloaded/GC-ed.
The ClassLoader could not be unloaded if its Class-es are strong referenced
by something else. This is what I meant by "Class Loader leak".
By using weak key you say "I'm fine to remove this entry if the key is not
referenced anywhere else.
Additionally by using "-XX:+CMSClassUnloadingEnabled" (with
-XX:+UseConcMarkSweepGC) you can hint the JVM to unload dynamically created
classes which ClassLoaders are still around.
> container tries to unload the Class then the map won't be "strong enough"
> to hold on.
>
> Correct me if I'm wrong; but if that's what you were thinking, then I
> agree.
>
> I'll make the change.
>
> Thx again
> Dan
>
>
>
> On 24 April 2015 at 13:00, Martin Grigorov <mg...@apache.org> wrote:
>
> > On Fri, Apr 24, 2015 at 1:15 PM, <da...@apache.org> wrote:
> >
> > > Repository: isis
> > > Updated Branches:
> > > refs/heads/master 69d559486 -> ee834d9a3
> > >
> > >
> > > ISIS-1137: Now cache a ProxyFactory instance by class (allowing reuse
> of
> > > the enhanced class for all instances of that type) rather than
> creating a
> > > newly enhanced class as currently.
> > >
> > >
> > > Project: http://git-wip-us.apache.org/repos/asf/isis/repo
> > > Commit: http://git-wip-us.apache.org/repos/asf/isis/commit/a4f924b1
> > > Tree: http://git-wip-us.apache.org/repos/asf/isis/tree/a4f924b1
> > > Diff: http://git-wip-us.apache.org/repos/asf/isis/diff/a4f924b1
> > >
> > > Branch: refs/heads/master
> > > Commit: a4f924b11cf614535e2faf2fb4f7aacdb4c7e1e0
> > > Parents: 69d5594
> > > Author: Dan Haywood <da...@haywood-associates.co.uk>
> > > Authored: Fri Apr 24 10:29:16 2015 +0100
> > > Committer: Dan Haywood <da...@haywood-associates.co.uk>
> > > Committed: Fri Apr 24 10:29:16 2015 +0100
> > >
> > > ----------------------------------------------------------------------
> > > .../isis/core/commons/lang/ArrayExtensions.java | 4 +-
> > > .../core/wrapper/WrapperFactoryAbstract.java | 6 +-
> > > .../proxy/ProxyInstantiatorForJavassist.java | 71
> > +++++++++++++++-----
> > > 3 files changed, 61 insertions(+), 20 deletions(-)
> > > ----------------------------------------------------------------------
> > >
> > >
> > >
> > >
> >
> http://git-wip-us.apache.org/repos/asf/isis/blob/a4f924b1/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > > ----------------------------------------------------------------------
> > > diff --git
> > >
> >
> a/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > >
> >
> b/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > > index 25a6f88..df7238b 100644
> > > ---
> > >
> >
> a/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > > +++
> > >
> >
> b/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > > @@ -27,6 +27,8 @@ import java.util.Arrays;
> > > import java.util.Collection;
> > > import java.util.List;
> > >
> > > +import com.google.common.collect.Lists;
> > > +
> > > import org.apache.isis.core.commons.exceptions.IsisException;
> > >
> > > public final class ArrayExtensions {
> > > @@ -71,7 +73,7 @@ public final class ArrayExtensions {
> > > }
> > >
> > > public static <T> T[] combine(final T[]... arrays) {
> > > - final List<T> combinedList = new ArrayList<T>();
> > > + final List<T> combinedList = Lists.newArrayList();
> > > for (final T[] array : arrays) {
> > > for (final T t : array) {
> > > combinedList.add(t);
> > >
> > >
> > >
> >
> http://git-wip-us.apache.org/repos/asf/isis/blob/a4f924b1/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > > ----------------------------------------------------------------------
> > > diff --git
> > >
> >
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > >
> >
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > > index ceea4a5..2d251ed 100644
> > > ---
> > >
> >
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > > +++
> > >
> >
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > > @@ -51,11 +51,11 @@ public abstract class WrapperFactoryAbstract
> > > implements WrapperFactory, Authenti
> > > private AdapterManager adapterManager;
> > > private ObjectPersistor objectPersistor;
> > >
> > > - private final ProxyContextHandler proxy;
> > > + private final ProxyContextHandler proxyContextHandler;
> > >
> > > public WrapperFactoryAbstract(final ProxyInstantiator
> > > proxyInstantiator) {
> > >
> > > - proxy = new ProxyContextHandler(proxyInstantiator);
> > > + proxyContextHandler = new
> > ProxyContextHandler(proxyInstantiator);
> > >
> > > dispatchersByEventClass.put(ObjectTitleEvent.class, new
> > > InteractionEventDispatcherTypeSafe<ObjectTitleEvent>() {
> > > @Override
> > > @@ -221,7 +221,7 @@ public abstract class WrapperFactoryAbstract
> > > implements WrapperFactory, Authenti
> > > }
> > >
> > > protected <T> T createProxy(final T domainObject, final
> > ExecutionMode
> > > mode) {
> > > - return proxy.proxy(domainObject, this, mode,
> > > getAuthenticationSessionProvider(), getSpecificationLookup(),
> > > getAdapterManager(), getObjectPersistor());
> > > + return proxyContextHandler.proxy(domainObject, this, mode,
> > > getAuthenticationSessionProvider(), getSpecificationLookup(),
> > > getAdapterManager(), getObjectPersistor());
> > > }
> > >
> > > @Override
> > >
> > >
> > >
> >
> http://git-wip-us.apache.org/repos/asf/isis/blob/a4f924b1/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > > ----------------------------------------------------------------------
> > > diff --git
> > >
> >
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > >
> >
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > > index 3d2b838..cc0ee19 100644
> > > ---
> > >
> >
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > > +++
> > >
> >
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > > @@ -19,21 +19,38 @@
> > >
> > > package org.apache.isis.core.wrapper.proxy;
> > >
> > > -import java.lang.reflect.InvocationHandler;
> > > import java.lang.reflect.Method;
> > > +import java.util.Map;
> > >
> > > -import javassist.util.proxy.MethodFilter;
> > > -import javassist.util.proxy.MethodHandler;
> > > -import javassist.util.proxy.ProxyFactory;
> > > -import javassist.util.proxy.ProxyObject;
> > > +import com.google.common.collect.MapMaker;
> > >
> > > import org.apache.isis.applib.services.wrapper.WrapperObject;
> > > +import org.apache.isis.applib.services.wrapper.WrappingObject;
> > > import org.apache.isis.core.commons.lang.ArrayExtensions;
> > > +import
> > >
> >
> org.apache.isis.core.metamodel.specloader.classsubstitutor.JavassistEnhanced;
> > > import
> > org.apache.isis.core.wrapper.handlers.DelegatingInvocationHandler;
> > > import org.apache.isis.core.wrapper.internal.util.Util;
> > >
> > > +import javassist.util.proxy.MethodFilter;
> > > +import javassist.util.proxy.MethodHandler;
> > > +import javassist.util.proxy.Proxy;
> > > +import javassist.util.proxy.ProxyFactory;
> > > +
> > > public class ProxyInstantiatorForJavassist implements
> ProxyInstantiator
> > {
> > >
> > > + /**
> > > + * Lazily constructed cache.
> > > + */
> > > + private final Map<Class, ProxyFactory> proxyFactoryByClass;
> > > +
> > >
> >
> >
> >
> > > + public ProxyInstantiatorForJavassist() {
> > > + this(new MapMaker().concurrencyLevel(10).<Class,
> > > ProxyFactory>makeMap());
> > >
> >
> > I think this should use weak keys to prevent classloader leaks and allow
> > class unloading.
> >
> >
> > > + }
> > > +
> > > + public ProxyInstantiatorForJavassist(Map<Class, ProxyFactory>
> > > proxyFactoryByClass) {
> > > + this.proxyFactoryByClass = proxyFactoryByClass;
> > > + }
> > > +
> > > @SuppressWarnings("unchecked")
> > > public <T> T instantiateProxy(final DelegatingInvocationHandler<T>
> > > handler) {
> > >
> > > @@ -44,24 +61,40 @@ public class ProxyInstantiatorForJavassist
> implements
> > > ProxyInstantiator {
> > > if (clazz.isInterface()) {
> > > return Util.createInstance(clazz, handler,
> > > WrapperObject.class);
> > > } else {
> > > - final Class<T> enhancedClass = createEnhancedClass(clazz,
> > > handler, WrapperObject.class);
> > > - ProxyObject proxyObject = (ProxyObject)
> > > Util.createInstance(enhancedClass);
> > > - proxyObject.setHandler(new MethodHandler() {
> > > + final ProxyFactory proxyFactory = proxyFactoryFor(clazz);
> > > +
> > > + final Class<T> enhancedClass = proxyFactory.createClass();
> > > + final Proxy proxy = (Proxy)
> > > Util.createInstance(enhancedClass);
> > > +
> > > + proxy.setHandler(new MethodHandler() {
> > > @Override
> > > public Object invoke(Object self, Method thisMethod,
> > > Method proceed, Object[] args) throws Throwable {
> > > return handler.invoke(self, thisMethod, args);
> > > }
> > > });
> > > - return (T) proxyObject;
> > > +
> > > + return (T) proxy;
> > > }
> > > }
> > > -
> > > - @SuppressWarnings("unchecked")
> > > - private static <T> Class<T> createEnhancedClass(final Class<T>
> > > toProxyClass, final InvocationHandler handler, final Class<?>...
> > > auxiliaryTypes) {
> > > -
> > > +
> > > + private <T> ProxyFactory proxyFactoryFor(final Class<T>
> > toProxyClass)
> > > {
> > > + ProxyFactory proxyFactory =
> > proxyFactoryByClass.get(toProxyClass);
> > > + if(proxyFactory == null) {
> > > + proxyFactory = createProxyFactoryFor(toProxyClass);
> > > + proxyFactoryByClass.put(toProxyClass, proxyFactory);
> > > + }
> > > + return proxyFactory;
> > > + }
> > > +
> > > + private <T> ProxyFactory createProxyFactoryFor(final Class<T>
> > > toProxyClass) {
> > > +
> > > final ProxyFactory proxyFactory = new ProxyFactory();
> > > +
> > > proxyFactory.setSuperclass(toProxyClass);
> > > -
> > >
> >
> proxyFactory.setInterfaces(ArrayExtensions.combine(toProxyClass.getInterfaces(),
> > > new
> > >
> >
> Class<?>[]{org.apache.isis.core.metamodel.specloader.classsubstitutor.JavassistEnhanced.class},
> > > auxiliaryTypes));
> > > + final Class[] types = ArrayExtensions.combine(
> > > + toProxyClass.getInterfaces(),
> > > + new Class<?>[] { JavassistEnhanced.class,
> > > WrappingObject.class });
> > > + proxyFactory.setInterfaces(types);
> > >
> > > proxyFactory.setFilter(new MethodFilter() {
> > > @Override
> > > @@ -70,8 +103,14 @@ public class ProxyInstantiatorForJavassist
> implements
> > > ProxyInstantiator {
> > > return !m.getName().equals("finalize") ||
> m.isBridge();
> > > }
> > > });
> > > -
> > > - return proxyFactory.createClass();
> > > +
> > > + // this is the default, I believe
> > > + // calling it here only because I know that it will throw an
> > > exception if the code were
> > > + // in the future changed such that caching is invalid
> > > + // (ie fail fast if future change could introduce a bug)
> > > + proxyFactory.setUseCache(true);
> > > +
> > > + return proxyFactory;
> > > }
> > >
> > > }
> > >
> > >
> >
>
Re: [1/4] isis git commit: ISIS-1137: Now cache a ProxyFactory
instance by class (allowing reuse of the enhanced class for all instances of
that type) rather than creating a newly enhanced class as currently.
Posted by Dan Haywood <da...@haywood-associates.co.uk>.
Hi Martin,
thx for the input. I did consider this, but realized that I don't know
about the interaction of class(un)loading vs GC.
My understanding (correct me if I'm wrong) is that GC daemon will quite
eagerly clean up any objects if there are only weak references to that
object. So I was worried about our ProxyFactory instances - that we want
to hold on to for as long as there are any instances of the class being
enhanced - might get GC'd away.
But thinking a bit deeper, I suspect that the above sentence is nonsense...
if we have weak keys for the Class', then of course the container itself
will have strong references to those classes. And, as you say, only if the
container tries to unload the Class then the map won't be "strong enough"
to hold on.
Correct me if I'm wrong; but if that's what you were thinking, then I agree.
I'll make the change.
Thx again
Dan
On 24 April 2015 at 13:00, Martin Grigorov <mg...@apache.org> wrote:
> On Fri, Apr 24, 2015 at 1:15 PM, <da...@apache.org> wrote:
>
> > Repository: isis
> > Updated Branches:
> > refs/heads/master 69d559486 -> ee834d9a3
> >
> >
> > ISIS-1137: Now cache a ProxyFactory instance by class (allowing reuse of
> > the enhanced class for all instances of that type) rather than creating a
> > newly enhanced class as currently.
> >
> >
> > Project: http://git-wip-us.apache.org/repos/asf/isis/repo
> > Commit: http://git-wip-us.apache.org/repos/asf/isis/commit/a4f924b1
> > Tree: http://git-wip-us.apache.org/repos/asf/isis/tree/a4f924b1
> > Diff: http://git-wip-us.apache.org/repos/asf/isis/diff/a4f924b1
> >
> > Branch: refs/heads/master
> > Commit: a4f924b11cf614535e2faf2fb4f7aacdb4c7e1e0
> > Parents: 69d5594
> > Author: Dan Haywood <da...@haywood-associates.co.uk>
> > Authored: Fri Apr 24 10:29:16 2015 +0100
> > Committer: Dan Haywood <da...@haywood-associates.co.uk>
> > Committed: Fri Apr 24 10:29:16 2015 +0100
> >
> > ----------------------------------------------------------------------
> > .../isis/core/commons/lang/ArrayExtensions.java | 4 +-
> > .../core/wrapper/WrapperFactoryAbstract.java | 6 +-
> > .../proxy/ProxyInstantiatorForJavassist.java | 71
> +++++++++++++++-----
> > 3 files changed, 61 insertions(+), 20 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/isis/blob/a4f924b1/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > ----------------------------------------------------------------------
> > diff --git
> >
> a/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> >
> b/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > index 25a6f88..df7238b 100644
> > ---
> >
> a/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > +++
> >
> b/core/metamodel/src/main/java/org/apache/isis/core/commons/lang/ArrayExtensions.java
> > @@ -27,6 +27,8 @@ import java.util.Arrays;
> > import java.util.Collection;
> > import java.util.List;
> >
> > +import com.google.common.collect.Lists;
> > +
> > import org.apache.isis.core.commons.exceptions.IsisException;
> >
> > public final class ArrayExtensions {
> > @@ -71,7 +73,7 @@ public final class ArrayExtensions {
> > }
> >
> > public static <T> T[] combine(final T[]... arrays) {
> > - final List<T> combinedList = new ArrayList<T>();
> > + final List<T> combinedList = Lists.newArrayList();
> > for (final T[] array : arrays) {
> > for (final T t : array) {
> > combinedList.add(t);
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/isis/blob/a4f924b1/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > ----------------------------------------------------------------------
> > diff --git
> >
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> >
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > index ceea4a5..2d251ed 100644
> > ---
> >
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > +++
> >
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/WrapperFactoryAbstract.java
> > @@ -51,11 +51,11 @@ public abstract class WrapperFactoryAbstract
> > implements WrapperFactory, Authenti
> > private AdapterManager adapterManager;
> > private ObjectPersistor objectPersistor;
> >
> > - private final ProxyContextHandler proxy;
> > + private final ProxyContextHandler proxyContextHandler;
> >
> > public WrapperFactoryAbstract(final ProxyInstantiator
> > proxyInstantiator) {
> >
> > - proxy = new ProxyContextHandler(proxyInstantiator);
> > + proxyContextHandler = new
> ProxyContextHandler(proxyInstantiator);
> >
> > dispatchersByEventClass.put(ObjectTitleEvent.class, new
> > InteractionEventDispatcherTypeSafe<ObjectTitleEvent>() {
> > @Override
> > @@ -221,7 +221,7 @@ public abstract class WrapperFactoryAbstract
> > implements WrapperFactory, Authenti
> > }
> >
> > protected <T> T createProxy(final T domainObject, final
> ExecutionMode
> > mode) {
> > - return proxy.proxy(domainObject, this, mode,
> > getAuthenticationSessionProvider(), getSpecificationLookup(),
> > getAdapterManager(), getObjectPersistor());
> > + return proxyContextHandler.proxy(domainObject, this, mode,
> > getAuthenticationSessionProvider(), getSpecificationLookup(),
> > getAdapterManager(), getObjectPersistor());
> > }
> >
> > @Override
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/isis/blob/a4f924b1/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > ----------------------------------------------------------------------
> > diff --git
> >
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> >
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > index 3d2b838..cc0ee19 100644
> > ---
> >
> a/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > +++
> >
> b/core/wrapper/src/main/java/org/apache/isis/core/wrapper/proxy/ProxyInstantiatorForJavassist.java
> > @@ -19,21 +19,38 @@
> >
> > package org.apache.isis.core.wrapper.proxy;
> >
> > -import java.lang.reflect.InvocationHandler;
> > import java.lang.reflect.Method;
> > +import java.util.Map;
> >
> > -import javassist.util.proxy.MethodFilter;
> > -import javassist.util.proxy.MethodHandler;
> > -import javassist.util.proxy.ProxyFactory;
> > -import javassist.util.proxy.ProxyObject;
> > +import com.google.common.collect.MapMaker;
> >
> > import org.apache.isis.applib.services.wrapper.WrapperObject;
> > +import org.apache.isis.applib.services.wrapper.WrappingObject;
> > import org.apache.isis.core.commons.lang.ArrayExtensions;
> > +import
> >
> org.apache.isis.core.metamodel.specloader.classsubstitutor.JavassistEnhanced;
> > import
> org.apache.isis.core.wrapper.handlers.DelegatingInvocationHandler;
> > import org.apache.isis.core.wrapper.internal.util.Util;
> >
> > +import javassist.util.proxy.MethodFilter;
> > +import javassist.util.proxy.MethodHandler;
> > +import javassist.util.proxy.Proxy;
> > +import javassist.util.proxy.ProxyFactory;
> > +
> > public class ProxyInstantiatorForJavassist implements ProxyInstantiator
> {
> >
> > + /**
> > + * Lazily constructed cache.
> > + */
> > + private final Map<Class, ProxyFactory> proxyFactoryByClass;
> > +
> >
>
>
>
> > + public ProxyInstantiatorForJavassist() {
> > + this(new MapMaker().concurrencyLevel(10).<Class,
> > ProxyFactory>makeMap());
> >
>
> I think this should use weak keys to prevent classloader leaks and allow
> class unloading.
>
>
> > + }
> > +
> > + public ProxyInstantiatorForJavassist(Map<Class, ProxyFactory>
> > proxyFactoryByClass) {
> > + this.proxyFactoryByClass = proxyFactoryByClass;
> > + }
> > +
> > @SuppressWarnings("unchecked")
> > public <T> T instantiateProxy(final DelegatingInvocationHandler<T>
> > handler) {
> >
> > @@ -44,24 +61,40 @@ public class ProxyInstantiatorForJavassist implements
> > ProxyInstantiator {
> > if (clazz.isInterface()) {
> > return Util.createInstance(clazz, handler,
> > WrapperObject.class);
> > } else {
> > - final Class<T> enhancedClass = createEnhancedClass(clazz,
> > handler, WrapperObject.class);
> > - ProxyObject proxyObject = (ProxyObject)
> > Util.createInstance(enhancedClass);
> > - proxyObject.setHandler(new MethodHandler() {
> > + final ProxyFactory proxyFactory = proxyFactoryFor(clazz);
> > +
> > + final Class<T> enhancedClass = proxyFactory.createClass();
> > + final Proxy proxy = (Proxy)
> > Util.createInstance(enhancedClass);
> > +
> > + proxy.setHandler(new MethodHandler() {
> > @Override
> > public Object invoke(Object self, Method thisMethod,
> > Method proceed, Object[] args) throws Throwable {
> > return handler.invoke(self, thisMethod, args);
> > }
> > });
> > - return (T) proxyObject;
> > +
> > + return (T) proxy;
> > }
> > }
> > -
> > - @SuppressWarnings("unchecked")
> > - private static <T> Class<T> createEnhancedClass(final Class<T>
> > toProxyClass, final InvocationHandler handler, final Class<?>...
> > auxiliaryTypes) {
> > -
> > +
> > + private <T> ProxyFactory proxyFactoryFor(final Class<T>
> toProxyClass)
> > {
> > + ProxyFactory proxyFactory =
> proxyFactoryByClass.get(toProxyClass);
> > + if(proxyFactory == null) {
> > + proxyFactory = createProxyFactoryFor(toProxyClass);
> > + proxyFactoryByClass.put(toProxyClass, proxyFactory);
> > + }
> > + return proxyFactory;
> > + }
> > +
> > + private <T> ProxyFactory createProxyFactoryFor(final Class<T>
> > toProxyClass) {
> > +
> > final ProxyFactory proxyFactory = new ProxyFactory();
> > +
> > proxyFactory.setSuperclass(toProxyClass);
> > -
> >
> proxyFactory.setInterfaces(ArrayExtensions.combine(toProxyClass.getInterfaces(),
> > new
> >
> Class<?>[]{org.apache.isis.core.metamodel.specloader.classsubstitutor.JavassistEnhanced.class},
> > auxiliaryTypes));
> > + final Class[] types = ArrayExtensions.combine(
> > + toProxyClass.getInterfaces(),
> > + new Class<?>[] { JavassistEnhanced.class,
> > WrappingObject.class });
> > + proxyFactory.setInterfaces(types);
> >
> > proxyFactory.setFilter(new MethodFilter() {
> > @Override
> > @@ -70,8 +103,14 @@ public class ProxyInstantiatorForJavassist implements
> > ProxyInstantiator {
> > return !m.getName().equals("finalize") || m.isBridge();
> > }
> > });
> > -
> > - return proxyFactory.createClass();
> > +
> > + // this is the default, I believe
> > + // calling it here only because I know that it will throw an
> > exception if the code were
> > + // in the future changed such that caching is invalid
> > + // (ie fail fast if future change could introduce a bug)
> > + proxyFactory.setUseCache(true);
> > +
> > + return proxyFactory;
> > }
> >
> > }
> >
> >
>