Proxy.isProxyClass scalability

[Peter Levart]

Hi Mandy,

On 04/12/2013 11:31 PM, Mandy Chung wrote:
> Hi Peter,
>
> Thank you for rebasing the patch. This is very good work and I hope to 
> make time to work with you to get your patch to jdk8 over the next 
> couple weeks.

I hope I can be of assistance,

>
> On 4/10/2013 5:35 AM, Peter Levart wrote:
>> Hi Alan,
>>
>> I have prepared new webrev of the patch rebased to current tip of 
>> jdk8/tl repo:
>>
>> https://dl.dropboxusercontent.com/u/101777488/jdk8-tl/proxy/webrev.04/index.html 
>>
>>
> [...]
>>
>> I also devised an alternative caching mechanism with scalability in 
>> mind which uses WeakReferences for keys (for example ClassLoader) and 
>> values (for example Class) that could be used in this situation in 
>> case adding a field to ClassLoader is not an option:
>>
>
> I would also consider any alternative to avoid adding the 
> proxyClassCache field in ClassLoader as Alan commented previously.
>
> My observation of the typical usage of proxies is to use the 
> interface's class loader to define the proxy class. So is it necessary 
> to maintain a per-loader cache?  The per-loader cache maps from the 
> interface names to a proxy class defined by one loader. I would think 
> it's reasonable to assume the number of loaders to define proxy class 
> with the same set of interfaces is small.  What if we make the cache 
> as "interface names" as the key to a set of proxy class suppliers that 
> can have only one proxy class per one unique defining loader.  If the 
> proxy class is being generated i.e. ProxyClassFactory supplier, the 
> loader is available for comparison. When there are more than one 
> matching proxy classes, it would have to iterate all in the set.

I would assume yes, proxy class for a particular set of interfaces is 
typically defined by one classloader only. But the API allows to specify 
different loaders as long as the interfaces implemented by proxy class 
are "visible" from the loader that defines the proxy class. If we're 
talking about interface names - as opposed to interfaces - then the 
possibility that a particular set of interface names would want to be 
used to define proxy classes with different loaders is even bigger, 
since an interface name can refer to different interfaces with same name 
(think of interfaces deployed as part of an app in an application 
server, say a set of annotations used by different apps but deployed as 
part of each individual app).

The scheme you're proposing might be possible, though not simple: The 
factory Supplier<Class> would become a Function<ClassLoader, Class> and 
would have to maintain it's own set of cached proxy classes. There would 
be a single ConcurrentMap<List<String>, Function<ClassLoader, Class>> to 
map sets of interface names to factory Functions, but the cached classes 
in a particular factory Function would still have to be weakly 
referenced. I see some difficulties in implementing such a scheme:
- expunging cleared WeakReferences could only reliably clear the cache 
inside each factory Function but removing the entry from the map of  
factory Functions when last proxy class for a particular set of 
interface names is expunged  would become a difficult task if not 
impossible with all the scalability constraints in mind (just thinking 
about concurrent requests into same factory Function where one is 
requesting new proxy class and the other is expunging cleared 
WeakReference which represents the last element in the set of cached 
proxy classes).
- one of my past ideas of implementing scalable Proxy.isProxyClass() was 
to maintain a Set<Class> in each ClassLoader populated with all the 
proxy classes defined by a particular ClassLoader. Benchmarking such 
solution showed that Class.getClassLoader() is a peformance hog, so I 
scraped it in favor of ClassValue<Boolean> that is now incorporated in 
the patch. In order to "choose" the right proxy class from the set of 
proxy classes inside a particular factory Function, the 
Class.getClassLoader() method would have to be used, or entries would 
have to (weakly) reference a particular ClassLoader associated with each 
proxy class.

Considering all that, such solution starts to look unappealing. It might 
even be more space-hungry then the presented WeakCache.

WeakCache is currently the following:

ConcurrentMap<WeakReferenceWithInterfaceNames<ClassLoader>, 
WeakReference<Class>>

another alternative would be:

ConcurrentMap<WeakReference<ClassLoader>, ConcurrentMap<InterfaceNames, 
WeakReference<Class>>>

...which might need a little less space than WeakCache (only one 
WeakReference per proxy class + one per ClassLoader instead of two 
WeakReferences per proxy class) but would require two map lookups during 
fast-path retrieval. It might not be performance critical and the 
expunging could be performed easily too.


Regards, Peter

>
> This alternative is sub-optimal than the per-loader cache. I'd be 
> interested in your thought of this alternative and any rough idea of 
> the performance difference with and without the per-loader cache 
> approach for the annotation case.
>
> Coding convention: we use /** ... */ for javadoc and /*...*/ or // for 
> comments.  Your patch uses /**...*/ style as comments that need 
> fixing.  The style you have for
>    try {
>    }
>    catch {
>    }
>    finally {
>    }
>
> Mandy
>
>> https://github.com/plevart/jdk8-tl/blob/proxy/test/src/test/WeakCache.java 
>>
>>
>>
>> Regards, Peter
>>
>>
>