mandy.chung at oracle.com
Fri Apr 19 05:33:28 UTC 2013
On 4/17/13 7:18 AM, Peter Levart wrote:
> As expected, the Proxy.getProxyClass() is yet a little slower than
> with FlattenedWeakCache, but still much faster than original Proxy
> implementation. Another lookup in the ConcurrentHashMap and another
> indirection have a price, but we also get something in return - space.
> So we loose approx. 32 bytes (32bit addresses) or 48 bytes (64 bit
> addresses) for each proxy class compared to original code when using
> FlattenedWeakCache, but we gain 8 bytes (32 bit or 64 bit addresses)
> for each proxy class cached compared to original code when using
> TwoLevelWeakCache. So which to favour, space or time?
> With TwoLevelWeakCache, there is a "step" of 108 bytes (32bit
> addresses) when new ClassLoader is encoutered (new 2nd level
> ConcurrentHashMap is allocated and new entry added to 1st level CHM.
> There's no such "step" in FlattenedWeakCache (modulo the steps when
> the CHMs are itself resized). So we roughly have 108 bytes wasted for
> each new ClassLoader encountered with TwoLevelWeakCache vs.
> FlattenedWeakCache, but we also have 40 bytes spared for each proxy
> class cached. TwoLevelWeakCache starts to pay off if there are at
> least 3 proxy classes defined per ClassLoader in average.
Thanks for the detailed measurement and analysis. Although the extra
lookup on the per-loader cache incurs additional overhead on
Proxy.getProxyClass, it still shows 10x speed on your performance
measurement result which is very good.
Since the performance improvement is significant on both approaches, the
memory saving would be the desirable choice.Especially Java SE 8
profiles  can run on small devices and we should be cautious about
the space and speed tradeoff.I'll support going for per-loader cache
(i.e. two-level weak cache).
Another point - Proxies are used in the JDK to implement annotations,
java.beans.EventHandler, JMX MXBeans mapping, JMX MBean proxies,
Invocable interface by script engines, and RMI/serialization. JAXWS
also uses proxies to support @javax.xml.bind.annotation.XmlElement. For
the case of annotations and JMX, the number of generated proxy classes
would typically not be a couple that depends on what interfaces
application define. In EE environment, there will be many class loaders
running. It'd be better to prepare to work the moderate number, if not
high, of proxy classes and multiple loaders case.
> What about package-private in java.lang.reflect? It makes Proxy itself
> much easier to read. When we decide which way to go, I can remove the
> interface and only leave a single package-private class...
thanks. Moving it to a single package-private classin
java.lang.reflectand remove the interface sounds good.
I have merged your patch with the recent TL repo and did some clean up
while reviewing it. Some comments:
1. getProxyClass0 should validate the input interfaces and throw IAE if
any illegal argument (e.g. interfaces are not visible to the given
loader) before calling proxyClassCache.get(loader, interfaces). I moved
back the validation code from ProxyClassFactory.apply to getProxyClass0.
2. I did some cleanup to restore some original comments to make the
diffs clearer where the change is.
3. I removed the newInstance method which is dead code after my previous
code. Since we are in this code, I took the chance to clean that up and
also change a couple for-loop to use for-each.
I have put the merged and slightly modified Proxy.java and webrev at:
We will use this bug for your contribution:
7123493 : (proxy) Proxy.getProxyClass doesn't scale under high load
For the weak cache class, since it's for proxy implementation use, I
suggest to take out the supportContainsValueOperation flagas it always
keeps the reverse map for isProxyClass lookup.
You can simply call Objects.requireNonNull(param) instead of
requireNonNull(param, "param-name") since the proxy implementation
should have made sure the argument is non-null.
68 Object cacheKey = CacheKey.valueOf(
should be: all in one line or line break on a long-line. Same for
237 void expungeFrom(
238 ConcurrentMap<?, ? extends ConcurrentMap<?, ?>> map,
239 ConcurrentMap<?, Boolean> reverseMap
void expungeFrom(ConcurrentMap<?, ? extends ConcurrentMap<?, ?>> map,
ConcurrentMap<?, Boolean> reverseMap);
so that it'll be more consistent with the existing code. I'll do a
detailed review on the weak cache class as you will finalize the code
per the decision to go with the two-level weak cache.
Thanks again for the good work.
More information about the core-libs-dev