On 4/16/2013 7:18 AM, Peter Levart wrote:
Hi Mandy,

I prepared a preview variant of j.l.r.Proxy using WeakCache (turned into an interface and a special FlattenedWeakCache implementation in anticipation to create another variant using two-levels of ConcurrentHashMaps for backing storage, but with same API) just to compare performance:

https://dl.dropboxusercontent.com/u/101777488/jdk8-tl/proxy-wc/webrev.01/index.html



thanks for getting this prototype done quickly.

As the values (Class objects of proxy classes) must be wrapped in a WeakReference, the same instance of WeakReference can be re-used as a key in another ConcurrentHashMap to implement quick look-up for Proxy.isProxyClass() method eliminating the need to use ClassValue, which is quite space-hungry.


I also think maintaining another ConcurrentHashMap is a good replacement with the use of ClassValue to avoid its memory overhead.

Comparing the performance, here's a summary of all 3 variants (original, patched using a field in ClassLoader and this variant):

[...]

The improvement is still quite satisfactory, although a little slower than the direct-field variant. The scalability is the same as with direct-field variant.


Agree - the improvement is quite good.

Space consumption of cache structure, calculated as deep-size of the structure, ignoring interned Strings, Class and ClassLoader objects unsing single non-bootstrap ClassLoader for defining the proxy classes and using 32 bit addressing is the following:

[...]

So with new ConcurrentHashMap the patched Proxy uses about 32 bytes more per proxy class.

Is this satisfactory or should we also try a variant with two-levels of ConcurrentHashMaps?


The overhead seems okay to trade off the scalability.

Since you have prepared for doing another variant, it'd be good to compare two prototypes if this doesn't bring too much work :) I would imagine that there might be slight difference in your measurement when comparing with proxies defined by a single class loader but the code might be simpler (might not be if you keep the same API but different implementation).

Regardless of which approach to use - you have added a general purpose WeakCache and the implementation class in the sun.misc package. While it's good to have such class for other jdk class to use, I am more comfortable in keeping it as a private class for proxy implementation to use. We need existing applications to migrate away from sun.misc and other private APIs to prepare for modularization.

Nits: can you wrap the lines around 80 columns including comments? try-catch-finally statements need some formatting fixes. Our convention is to have 'catch', or 'finally' following the closing bracket '}' in the same line. Your editor breaks 'catch' or 'finally' into the next line.


Even without SecurityManager installed the performance of native getClassLoader0 was a hog. I don't know why? Isn't there an implicit reference to defining ClassLoader from every Class object?

That's right - it looks for the caller class only if the security manager is installed. The defining class loader is kept in the VM's Klass object (language-level Class instance representation in the VM) and there is no computation needed to obtain a defining class loader of a given Class object. I can only think of the Java <-> native transition overhead that could be one factor. Class.getClassLoader0 is not intrinsified. I'll find out (others on this mailing list may probably know).

Mandy

Reply via email to