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