On Thu, 26 Nov 2020 21:23:16 GMT, Vladimir Ivanov <vliva...@openjdk.org> wrote:

> Concurrent updates may lead to redundant LambdaForms created and unnecessary 
> class loading when those are compiled. 
> 
> Most notably, it severely affects MethodHandle customization: when a 
> MethodHandle is called from multiple threads, every thread starts 
> customization which takes enough time for other threads to join, but only one 
> of those customizations will be picked.
> 
> Coordination between threads requesting the updates and letting a single 
> thread proceed avoids the aforementioned problem. Moreover, there's no need 
> to wait until the update in-flight is over: all other threads (except the one 
> performing the update) can just proceed with the invocation using the 
> existing MH.form. 
> 
> Testing:
> - manually monitored the behavior on a stress test from 
> [JDK-8252049](https://bugs.openjdk.java.net/browse/JDK-8252049)
> - tier1-4

src/java.base/share/classes/java/lang/invoke/MethodHandle.java line 1783:

> 1781:             } finally {
> 1782:                 updateInProgress = false;
> 1783:             }

Line 1782. I understand that in case the try block fails to update `form` 
field, resetting `updateInProgress` is desirable in order for next caller to 
attempt to update the form next time, but in case (the majority case) the try 
block successfully updates the `form` field, resetting the `updateInProgress` 
back to `false` opens a theoretical window (very improbable) for a concurrent 
thread to re-execute the updater function at least. Perhaps you could write 
`catch (Throwable e)` instead of `finally` and only in case of failure, reset 
the `updateInProgress` field and re-throw the exception. WDYT?

It is also very strange that the code is updating a final `form` field using 
Unsafe. Is this only to ensure that reads of that field scattered around in 
code are accompanied with a suitable read fence if needed (on some 
architectures)? How does this work considering (I may be wrong, but I think I 
heard) that VM trusts final fields in java.lang.invoke package.

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

PR: https://git.openjdk.java.net/jdk/pull/1472

Reply via email to