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