RFR: 8242440: use separate, destroyable JavaVM instances per libgraal compiler thread

Kim Barrett kbarrett at openjdk.java.net
Mon Apr 18 20:58:27 UTC 2022


On Mon, 18 Apr 2022 09:13:22 GMT, Doug Simon <dnsimon at openjdk.org> wrote:

>> I forgot that `_shared_library_javavm` is declared `volatile JavaVM*`.  So what's needed is a const_cast to discard the volatile.  But why is it pointer to volatile?  I'm not seeing any reason for that; the out argument for `JNI_CreateJavaVM` doesn't specify volatile.  I wonder if it's supposed to instead be `JavaVM* volatile` to (attempt to, though maybe not correctly) support unlocked reads with concurrent locked writes?
>
> It is declared volatile for correct double checked locking in the code prior to this PR:
> 
> JNIEnv* JVMCIRuntime::init_shared_library_javavm() {
>   JavaVM* javaVM = (JavaVM*) _shared_library_javavm;
>   if (javaVM == NULL) {
>     MutexLocker locker(JVMCI_lock);
>     // Check again under JVMCI_lock
>     javaVM = (JavaVM*) _shared_library_javavm;
>     if (javaVM != NULL) {
>       return NULL;
>     }
> 
> However, now that `JVMCIRuntime::_lock` is acquired at the entry to `JVMCIRuntime::init_shared_library_javavm` there is indeed no longer a need for it to be volatile.

The declaration was wrong for that usage - it should have been "volatile pointer to X" (`JavaVM* volatile`) but was declared "pointer to volatile X" (`volatile JavaVM*`).  That also wasn't a correct implementation of DCLP.  Good thing you are changing this.  So yes, the volatile should be nuked and this cast and the similar one in destroy_shared_library_javavm can be removed.

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

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


More information about the hotspot-dev mailing list