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

[Tobias Hartmann]

On Fri, 22 Apr 2022 10:08:20 GMT, Doug Simon <dnsimon at openjdk.org> wrote:

>> Currently, libgraal runs in a single, permanent JavaVM instance loaded from libjvmcicompiler.so. This poses 2 problems:
>> 
>> 1. The memory used for libgraal is never released, even when the libgraal compiler queues are idle.
>> 2. Since all libgraal HotSpot compiler threads share the same SVM heap, libgraal compilation is effectively paused when a GC occurs (SVM currently has a stop the world collector). The more compiler threads there are, the more often this happens.
>> 
>> This PR implements the following solution to these problems:
>> 1. Create a new JavaVM instance per libgraal compiler thread (i.e. a separate SVM isolate per thread). This prevents GC in one libgraal thread from impacting another libgraal thread.
>> 2. Destroy the JavaVM instances when a libgraal compiler thread becomes idle and create a new one when subsequent compilation requests arrive.
>> 
>> Most of the changes are in JVMCI specific files. The most significant change to shared code is the addition of the `JavaThread:: _libjvmci_runtime` field. This is required as any thread (not just a `CompilerThread`) can be associated with a specific `JVMCIRuntime` object.
>
> Doug Simon has updated the pull request incrementally with one additional commit since the last revision:
> 
>   avoid resource allocation in JVMCIRuntime::release_and_clear_globals

I'm not an expert in the JVMCI code but it looks reasonable to me.

src/hotspot/share/compiler/compileBroker.cpp line 443:

> 441:     }
> 442: 
> 443:     AbstractCompiler *compiler = thread->compiler();

Suggestion:

    AbstractCompiler* compiler = thread->compiler();

src/hotspot/share/jvmci/jvmci.cpp line 204:

> 202:     }
> 203:     for (JVMCIRuntime* runtime = _compiler_runtimes; runtime != nullptr; runtime = runtime->_next) {
> 204:      runtime->_metadata_handles->do_unloading();

Suggestion:

    for (JVMCIRuntime* runtime = _compiler_runtimes; runtime != nullptr; runtime = runtime->_next) {
      runtime->_metadata_handles->do_unloading();

src/hotspot/share/jvmci/jvmci.cpp line 264:

> 262:     java_runtime->shutdown();
> 263:   }
> 264:   JVMCIRuntime *runtime = thread->libjvmci_runtime();

Suggestion:

  JVMCIRuntime* runtime = thread->libjvmci_runtime();

src/hotspot/share/jvmci/jvmci.hpp line 122:

> 120:   // Returns true iff there is a new shared library JavaVM per compilation.
> 121:   static bool one_shared_library_javavm_per_compilation() {
> 122:     return JVMCIThreadsPerNativeLibraryRuntime == 1 && JVMCICompilerIdleDelay == 0;

Why does this check for `JVMCICompilerIdleDelay == 0` and where is this method used?

src/hotspot/share/jvmci/jvmciCompilerToVM.cpp line 2269:

> 2267:     C2V_BLOCK(jboolean, isCurrentThreadAttached, (JNIEnv* env, jobject))
> 2268:     requireJVMCINativeLibrary(JVMCI_CHECK_0);
> 2269:     JVMCIRuntime *runtime = thread->libjvmci_runtime();

Suggestion:

    JVMCIRuntime* runtime = thread->libjvmci_runtime();

src/hotspot/share/jvmci/jvmciCompilerToVM.cpp line 2391:

> 2389:     requireJVMCINativeLibrary(JVMCI_CHECK_0);
> 2390:     requireInHotSpot("detachCurrentThread", JVMCI_CHECK_0);
> 2391:     JVMCIRuntime *runtime = thread->libjvmci_runtime();

Suggestion:

    JVMCIRuntime* runtime = thread->libjvmci_runtime();

src/hotspot/share/jvmci/jvmciCompilerToVM.cpp line 2414:

> 2412:       JNI_THROW_("detachCurrentThread", InternalError, "JVMCI shared library thread cannot release JVMCI shared library JavaVM", false);
> 2413:     }
> 2414:     JVMCIRuntime *runtime = thread->libjvmci_runtime();

Suggestion:

    JVMCIRuntime* runtime = thread->libjvmci_runtime();

src/hotspot/share/jvmci/jvmci_globals.hpp line 71:

> 69:           "compiler queue becomes idle, it is detached from its JVMCIRuntime. "\
> 70:           "Once the last thread is detached from a JVMCIRuntime, all "      \
> 71:           "all resources associated with the runtime are reclaimed.")       \

Suggestion:

          "Once the last thread is detached from a JVMCIRuntime, all "      \
          "resources associated with the runtime are reclaimed.")       \

src/hotspot/share/jvmci/vmStructs_jvmci.cpp line 412:

> 410:   declare_constant(JVM_ACC_FIELD_STABLE)                                  \
> 411:   declare_constant(JVM_ACC_FIELD_HAS_GENERIC_SIGNATURE)                   \
> 412:   declare_constant(JVM_ACC_IS_VALUE_BASED_CLASS)                          \

This looks Valhalla specific, why is it added here?

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

Marked as reviewed by thartmann (Reviewer).

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