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
