RFR: 6983726: Reimplement MethodHandleProxies.asInterfaceInstance [v20]

[Jorn Vernee]

On Tue, 4 Jul 2023 21:00:30 GMT, Mandy Chung <mchung at openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line 324:
>> 
>>> 322:             r = PROXY_LOOKUPS.get(intfc);
>>> 323:         }
>>> 324:         return r.get();
>> 
>> This doesn't look right to me. AFAICT the Lookup can be GC'd before we call `get`.
>> 
>>> No, it wouldn't. PROXY_LOOKUPS.get(intfc) returns the WeakReference of the lookup class which is strongly reachable in the stack frame.
>> 
>> I'm not sure how the lookup class (`intfc` ?) is keeping the `Lookup` alive here?
>
> Sorry I re-read Chen's question and missing the last half of the sentence.   My reply was about:  If the referent is not GC'ed, `r.get()` returns non-null which will be returned to the caller.  If the referent becomes weakly reachable, `r.get()` will return null.
> 
> OK.  In any case, I was thinking to use a local variable to keep the lookup object strongly reachable.   I adjusted the patch to keep a class value of a holder of `WeakReference` ([ab3d8b2](https://github.com/mlchung/jdk/commit/ab3d8b2f086dfac4f8fda1ddce415270c0c53d2b)):
> 
> 
>     private static Lookup getProxyClassLookup(Class<?> intfc) {
>         WeakReferenceHolder<Lookup> r = PROXY_LOOKUPS.get(intfc);
>         Lookup lookup = r.get();
>         if (lookup == null) {
>              // If the referent is cleared, create a new value and update cached weak reference.
>             lookup = newProxyLookup(intfc);
>             r.set(lookup);
>         }
>         return lookup;
>     }

Ok, thanks. This is similar to the solution I would have suggested.

The only thing I'm unsure about is races. ClassValue allows multiple threads to race to compute the final value, but in the end only 1 is chosen that is returned to all threads.

If multiple threads race to create a new proxy lookup here, they will all get a distinct lookup returned to them. I think this is okay? Though, it would mean we have less sharing of proxy classes.

Maybe this code should synchronize on `r` to make sure only 1 lookup is created per interface. (Letting threads race to compute the value is also a possibility, but it also leads some threads to do work that is then discarded, and then the VM has to clean up the class they defined as well).

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1252373506