Proxy.isProxyClass scalability
Peter Levart
peter.levart at gmail.com
Sat Apr 20 07:31:00 UTC 2013
Hi Mandy,
I have another idea. Before jumping to implement it, I will first ask
what do you think of it. For example:
- have an optimal interface names key calculated from interfaces.
- visibility of interfaces and other validations are pushed to slow-path
(inside ProxyClassFactory.apply)
- the proxy Class object returned from WeakCache.get() is post-validated
with a check like:
for (Class<?> intf : interfaces) {
if (!intf.isAssignableFrom(proxyClass)) {
throw new IllegalArgumentException();
}
}
// return post-validated proxyClass from getProxyClass0()...
I feel that Class.isAssignableFrom(Class) check could be much faster and
that the only reason the check can fail is if some interface is not
visible from the class loader.
Am I correct?
Regards, Peter
On 04/19/2013 04:36 PM, Peter Levart wrote:
> Hi Mandy,
>
> On 04/19/2013 07:33 AM, Mandy Chung wrote:
>>>
>>> https://dl.dropboxusercontent.com/u/101777488/jdk8-tl/proxy-wc/webrev.02/index.html
>>>
>>> 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.
>
> Right.
>
>>
>> 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.
>
> Ops, you're right. There could be a request with interface(s) with
> same name(s) but loaded by different loader(s) and such code could
> return wrong pre-cached proxy class instead of throwing exception.
> Unfortunately, moving validation to slow-path was the cause of major
> performance and scalability improvement observed. With validation
> moved back to getProxyClass0 (in spite of using two-level WeakCache),
> we have the following performance (WeakCache#1):
>
>
> Summary (4 Cores x 2 Threads i7 CPU):
>
> Test Threads ns/op Original WeakCache#1
> ======================= ======= ============== ===========
> Proxy_getProxyClass 1 2,403.27 2,174.51
> 4 3,039.01 2,555.00
> 8 5,193.58 4,273.42
>
> Proxy_isProxyClassTrue 1 95.02 43.14
> 4 2,266.29 43.23
> 8 4,782.29 72.06
>
> Proxy_isProxyClassFalse 1 95.02 1.36
> 4 2,186.59 1.36
> 8 4,891.15 2.72
>
> Annotation_equals 1 240.10 195.68
> 4 1,864.06 201.41
> 8 8,639.20 337.46
>
> It's a little better than original getProxyClass(), but not much. The
> isProxyClass() and consequently Annotation.equals() are far better
> though. But as you might have guessed, I kind of solved that too...
>
> My first attempt was to optimize the interface validation. Only the
> "visibility" part is necessary to be performed on fast-path.
> Uniqueness and other things can be performed on slow-path. With
> split-validation and hacks like:
>
> private static final MethodHandle findLoadedClass0MH,
> findBootstrapClassMH;
> private static final ClassLoader dummyCL = new ClassLoader() {};
>
> static {
> try {
> Method method =
> ClassLoader.class.getDeclaredMethod("findLoadedClass0", String.class);
> method.setAccessible(true);
> findLoadedClass0MH =
> MethodHandles.lookup().unreflect(method);
>
> method =
> ClassLoader.class.getDeclaredMethod("findBootstrapClass", String.class);
> method.setAccessible(true);
> findBootstrapClassMH =
> MethodHandles.lookup().unreflect(method);
> } catch (NoSuchMethodException e) {
> throw (Error) new
> NoSuchMethodError(e.getMessage()).initCause(e);
> } catch (IllegalAccessException e) {
> throw (Error) new
> IllegalAccessError(e.getMessage()).initCause(e);
> }
> }
>
> static Class<?> findLoadedClass(ClassLoader loader, String name) {
> try {
> if (VM.isSystemDomainLoader(loader)) {
> return (Class<?>)
> findBootstrapClassMH.invokeExact(dummyCL, name);
> } else {
> return (Class<?>)
> findLoadedClass0MH.invokeExact(loader, name);
> }
> } catch (RuntimeException | Error e) {
> throw e;
> } catch (Throwable t) {
> throw new UndeclaredThrowableException(t);
> }
> }
>
>
> ... using findLoadedClass(loader, intf.getName()) and only doing
> Class.forName(intf.getName(), false, loader) if the former returned
> null ... I managed to reclaim some performance (WeakCache#1+):
>
>
> Test Threads ns/op Original WeakCache#1
> WeakCache#1+
> ======================= ======= ============== =========== ============
> Proxy_getProxyClass 1 2,403.27 2,174.51 1,589.36
> 4 3,039.01 2,555.00 1,929.11
> 8 5,193.58 4,273.42 3,409.77
>
>
> ...but that was still not very satisfactory.
>
> I modified the KeyFactory to create keys that weakly-reference
> interface Class objects and implement hashCode/equals in terms of
> comparing those Class objects. With such keys, no false aliasing can
> occur and the whole validation can be pushed back to slow-path. I
> tried hard to create keys with as little space overhead as possible:
>
> http://dl.dropboxusercontent.com/u/101777488/jdk8-tl/proxy-wc-wi/webrev.01/index.html
>
>
> ...but there can be no miracles. The good news is that the performance
> is back (WeakCache#2):
>
>
> Summary (4 Cores x 2 Threads i7 CPU):
>
> Test Threads ns/op Original WeakCache#1 WeakCache#2
> ======================= ======= ============== =========== ===========
> Proxy_getProxyClass 1 2,403.27 2,174.51 163.57
> 4 3,039.01 2,555.00 211.70
> 8 5,193.58 4,273.42 322.14
>
> Proxy_isProxyClassTrue 1 95.02 43.14 41.23
> 4 2,266.29 43.23 42.20
> 8 4,782.29 72.06 72.21
>
> Proxy_isProxyClassFalse 1 95.02 1.36 1.36
> 4 2,186.59 1.36 1.36
> 8 4,891.15 2.72 2.72
>
> Annotation_equals 1 240.10 195.68 194.61
> 4 1,864.06 201.41 198.81
> 8 8,639.20 337.46 342.90
>
>
> ... and the most common usage (proxy class implementing exactly one
> interface) uses even less space than with interface-names-key - 16
> bytes saved per proxy class vs. 8 bytes saved (32 bit addressing mode):
>
> --------------------------------------
> Original j.l.r.Proxy
> 1 interfaces / proxy class
>
> class proxy size of delta to
> loaders classes caches prev.ln.
> -------- -------- -------- --------
> 0 0 400 400
> 1 1 768 368
> 1 2 920 152
> 1 3 1072 152
> 1 4 1224 152
> 1 5 1376 152
> 1 6 1528 152
> 1 7 1680 152
> 1 8 1832 152
> 2 9 2152 320
> 2 10 2304 152
> 2 11 2456 152
> 2 12 2672 216
> 2 13 2824 152
> 2 14 2976 152
> 2 15 3128 152
> 2 16 3280 152
>
> --------------------------------------
> Patched j.l.r.Proxy
> 1 interfaces / proxy class
>
> class proxy size of delta to
> loaders classes caches prev.ln.
> -------- -------- -------- --------
> 0 0 240 240
> 1 1 792 552
> 1 2 928 136
> 1 3 1064 136
> 1 4 1200 136
> 1 5 1336 136
> 1 6 1472 136
> 1 7 1608 136
> 1 8 1744 136
> 2 9 2088 344
> 2 10 2224 136
> 2 11 2360 136
> 2 12 2560 200
> 2 13 2696 136
> 2 14 2832 136
> 2 15 2968 136
> 2 16 3104 136
>
>
> Did you know, that Proxy.getProxyClass() can generate proxy classes
> implementing 0 interfaces? It can. There's exactly one such class
> generated per class loader:
>
>
> --------------------------------------
> Original j.l.r.Proxy
> 0 interfaces / proxy class
>
> class proxy size of delta to
> loaders classes caches prev.ln.
> -------- -------- -------- --------
> 0 0 400 400
> 1 1 760 360
> 2 2 1072 312
>
> --------------------------------------
> Patched j.l.r.Proxy
> 0 interfaces / proxy class
>
> class proxy size of delta to
> loaders classes caches prev.ln.
> -------- -------- -------- --------
> 0 0 240 240
> 1 1 728 488
> 2 2 1040 312
>
>
> With 2 or more interfaces implemented by proxy class the space
> overhead increases faster than with original Proxy:
>
>
> --------------------------------------
> Original j.l.r.Proxy
> 2 interfaces / proxy class
>
> class proxy size of delta to
> loaders classes caches prev.ln.
> -------- -------- -------- --------
> 0 0 400 400
> 1 1 768 368
> 1 2 920 152
> 1 3 1072 152
> 1 4 1224 152
> 1 5 1376 152
> 1 6 1528 152
> 1 7 1680 152
> 1 8 1832 152
> 2 9 2152 320
> 2 10 2304 152
> 2 11 2456 152
> 2 12 2672 216
> 2 13 2824 152
> 2 14 2976 152
> 2 15 3128 152
> 2 16 3280 152
>
> --------------------------------------
> Patched j.l.r.Proxy
> 2 interfaces / proxy class
>
> class proxy size of delta to
> loaders classes caches prev.ln.
> -------- -------- -------- --------
> 0 0 240 240
> 1 1 832 592
> 1 2 1008 176
> 1 3 1184 176
> 1 4 1360 176
> 1 5 1536 176
> 1 6 1712 176
> 1 7 1888 176
> 1 8 2064 176
> 2 9 2448 384
> 2 10 2624 176
> 2 11 2800 176
> 2 12 3040 240
> 2 13 3216 176
> 2 14 3392 176
> 2 15 3568 176
> 2 16 3744 176
>
> --------------------------------------
> Original j.l.r.Proxy
> 3 interfaces / proxy class
>
> class proxy size of delta to
> loaders classes caches prev.ln.
> -------- -------- -------- --------
> 0 0 400 400
> 1 1 776 376
> 1 2 936 160
> 1 3 1096 160
> 1 4 1256 160
> 1 5 1416 160
> 1 6 1576 160
> 1 7 1736 160
> 1 8 1896 160
> 2 9 2224 328
> 2 10 2384 160
> 2 11 2544 160
> 2 12 2768 224
> 2 13 2928 160
> 2 14 3088 160
> 2 15 3248 160
> 2 16 3408 160
>
> --------------------------------------
> Patched j.l.r.Proxy
> 3 interfaces / proxy class
>
> class proxy size of delta to
> loaders classes caches prev.ln.
> -------- -------- -------- --------
> 0 0 240 240
> 1 1 864 624
> 1 2 1072 208
> 1 3 1280 208
> 1 4 1488 208
> 1 5 1696 208
> 1 6 1904 208
> 1 7 2112 208
> 1 8 2320 208
> 2 9 2736 416
> 2 10 2944 208
> 2 11 3152 208
> 2 12 3424 272
> 2 13 3632 208
> 2 14 3840 208
> 2 15 4048 208
> 2 16 4256 208
>
> --------------------------------------
> Original j.l.r.Proxy
> 4 interfaces / proxy class
>
> class proxy size of delta to
> loaders classes caches prev.ln.
> -------- -------- -------- --------
> 0 0 400 400
> 1 1 776 376
> 1 2 936 160
> 1 3 1096 160
> 1 4 1256 160
> 1 5 1416 160
> 1 6 1576 160
> 1 7 1736 160
> 1 8 1896 160
> 2 9 2224 328
> 2 10 2384 160
> 2 11 2544 160
> 2 12 2768 224
> 2 13 2928 160
> 2 14 3088 160
> 2 15 3248 160
> 2 16 3408 160
>
> --------------------------------------
> Patched j.l.r.Proxy
> 4 interfaces / proxy class
>
> class proxy size of delta to
> loaders classes caches prev.ln.
> -------- -------- -------- --------
> 0 0 240 240
> 1 1 896 656
> 1 2 1136 240
> 1 3 1376 240
> 1 4 1616 240
> 1 5 1856 240
> 1 6 2096 240
> 1 7 2336 240
> 1 8 2576 240
> 2 9 3024 448
> 2 10 3264 240
> 2 11 3504 240
> 2 12 3808 304
> 2 13 4048 240
> 2 14 4288 240
> 2 15 4528 240
> 2 16 4768 240
>
>
> There's an increase of 8 bytes per proxy class key for each 2
> interfaces added to proxy class in original Proxy code, but there's an
> increase of 32 bytes per proxy class key for each single interface
> added in patched Proxy code.
>
> I think the most common usage is to implement a single interface and
> there is 16 bytes gained for each such usage compared to original
> Proxy code.
>
>> 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:
>> http://cr.openjdk.java.net/~mchung/jdk8/webrevs/7123493/webrev.00/
>>
>> We will use this bug for your contribution:
>> 7123493 : (proxy) Proxy.getProxyClass doesn't scale under high load
>
> I took j.l.r.Proxy file from your webrev and just changed the
> KeyFactory implementation. WeakCache is generic and can be used
> unchanged with either implementation of KeyFactory.
>
>>
>> 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.
>>
>> Formatting nits:
>>
>> 68 Object cacheKey = CacheKey.valueOf(
>> 69 key,
>> 70 refQueue
>> 71 );
>>
>> should be: all in one line or line break on a long-line. Same for
>> method signature.
>>
>> 237 void expungeFrom(
>> 238 ConcurrentMap<?, ? extends ConcurrentMap<?, ?>> map,
>> 239 ConcurrentMap<?, Boolean> reverseMap
>> 240 );
>>
>> should be:
>>
>> 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.
>
> I hope I have addressed all that in above webrev.
>
> Regards, Peter
>
>>
>> Thanks again for the good work.
>>
>> Mandy
>> [1] http://openjdk.java.net/jeps/161
>
More information about the core-libs-dev
mailing list