RFR: 8145270: Need to eagerly initialize JVMCI compiler under -Xcomp

> On 14 Dec 2015, at 22:24, Christian Thalinger <[hidden email]> wrote:
>
>
>> On Dec 13, 2015, at 11:49 AM, Doug Simon <[hidden email]> wrote:
>>
>> In blocking compilation mode (i.e., -UseInterpreter), certain compilations are forced to be non-blocking if a JVMCI compiler is being used (i.e., +UseJVMCICompiler). This is to prevent deadlocks than can occur between an application thread and a JVMCI compiler thread. One condition for forcing a compilation to be non-blocking is if the JVMCI compiler not yet initialized. This is problematic for tests that attempt to force a method to be compiled by JVMCI (e.g., -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI -XX:+UseJVMCICompiler -XX:-TieredCompilation -Xcomp -XX:CompileCommand=compileonly,Tester_*::*). If the test is small enough, JVMCI initialization (which is lazy) may still be executing when the test methods are scheduled to be compiled.
>>
>> The solution is to make JVMCI compiler initialization eager in blocking compilation mode.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8145270
>> http://cr.openjdk.java.net/~dnsimon/8145270/
>
> Looks good.  The only comment I have is at that point in initialization we can handle exceptions already and I think we should.  This gives us even useful stack traces:
>
> Error occurred during initialization of VM
> java.lang.Error: just testing
> at jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.getCompiler(HotSpotJVMCIRuntime.java:181)
>
> Here is the updated patch:
>
> diff -r d84bd22ab531 src/share/vm/compiler/compileBroker.cpp
> --- a/src/share/vm/compiler/compileBroker.cpp Wed Dec 09 14:54:40 2015 +0100
> +++ b/src/share/vm/compiler/compileBroker.cpp Mon Dec 14 12:20:18 2015 -1000
> @@ -56,6 +56,7 @@
> #if INCLUDE_JVMCI
> #include "jvmci/jvmciCompiler.hpp"
> #include "jvmci/jvmciRuntime.hpp"
> +#include "jvmci/jvmciJavaClasses.hpp"
> #include "runtime/vframe.hpp"
> #endif
> #ifdef COMPILER2
> @@ -498,7 +499,7 @@ CompilerCounters::CompilerCounters() {
> // CompileBroker::compilation_init
> //
> // Initialize the Compilation object
> -void CompileBroker::compilation_init() {
> +void CompileBroker::compilation_init(TRAPS) {
>   _last_method_compiled[0] = '\0';
>
>   // No need to initialize compilation system if we do not use it.
> @@ -529,6 +530,17 @@ void CompileBroker::compilation_init() {
>       } else {
>         c1_count = JVMCIHostThreads;
>       }
> +
> +      if (!UseInterpreter) {
> +        // Force initialization of JVMCI compiler otherwise JVMCI
> +        // compilations will not block until JVMCI is initialized
> +        ResourceMark rm;
> +        TempNewSymbol getCompiler = SymbolTable::new_symbol("getCompiler", CHECK);
> +        TempNewSymbol sig = SymbolTable::new_symbol("()Ljdk/vm/ci/runtime/JVMCICompiler;", CHECK);
> +        Handle jvmciRuntime = JVMCIRuntime::get_HotSpotJVMCIRuntime(CHECK);
> +        JavaValue result(T_OBJECT);
> +        JavaCalls::call_virtual(&result, jvmciRuntime, HotSpotJVMCIRuntime::klass(), getCompiler, sig, CHECK);
> +      }
>     }
>   }
> #endif // INCLUDE_JVMCI
> diff -r d84bd22ab531 src/share/vm/compiler/compileBroker.hpp
> --- a/src/share/vm/compiler/compileBroker.hpp Wed Dec 09 14:54:40 2015 +0100
> +++ b/src/share/vm/compiler/compileBroker.hpp Mon Dec 14 12:20:18 2015 -1000
> @@ -276,7 +276,7 @@ public:
>     CompileQueue *q = compile_queue(comp_level);
>     return q != NULL ? q->size() : 0;
>   }
> -  static void compilation_init();
> +  static void compilation_init(TRAPS);
>   static void init_compiler_thread_log();
>   static nmethod* compile_method(const methodHandle& method,
>                                  int osr_bci,
> diff -r d84bd22ab531 src/share/vm/runtime/thread.cpp
> --- a/src/share/vm/runtime/thread.cpp Wed Dec 09 14:54:40 2015 +0100
> +++ b/src/share/vm/runtime/thread.cpp Mon Dec 14 12:20:18 2015 -1000
> @@ -3628,7 +3628,7 @@ jint Threads::create_vm(JavaVMInitArgs*
>
>   // initialize compiler(s)
> #if defined(COMPILER1) || defined(COMPILER2) || defined(SHARK) || INCLUDE_JVMCI
> -  CompileBroker::compilation_init();
> +  CompileBroker::compilation_init(CHECK_JNI_ERR);
> #endif
>
>   // Pre-initialize some JSR292 core classes to avoid deadlock during class loading.
>

> On Dec 14, 2015, at 12:46 PM, Doug Simon <[hidden email]> wrote:
>
> I was tempted to do that myself but opted for keeping the patch minimal. However, the solution that propagates the exception is obviously better.
>
>> On 14 Dec 2015, at 22:24, Christian Thalinger <[hidden email]> wrote:
>>
>>
>>> On Dec 13, 2015, at 11:49 AM, Doug Simon <[hidden email]> wrote:
>>>
>>> In blocking compilation mode (i.e., -UseInterpreter), certain compilations are forced to be non-blocking if a JVMCI compiler is being used (i.e., +UseJVMCICompiler). This is to prevent deadlocks than can occur between an application thread and a JVMCI compiler thread. One condition for forcing a compilation to be non-blocking is if the JVMCI compiler not yet initialized. This is problematic for tests that attempt to force a method to be compiled by JVMCI (e.g., -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI -XX:+UseJVMCICompiler -XX:-TieredCompilation -Xcomp -XX:CompileCommand=compileonly,Tester_*::*). If the test is small enough, JVMCI initialization (which is lazy) may still be executing when the test methods are scheduled to be compiled.
>>>
>>> The solution is to make JVMCI compiler initialization eager in blocking compilation mode.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8145270
>>> http://cr.openjdk.java.net/~dnsimon/8145270/
>>
>> Looks good.  The only comment I have is at that point in initialization we can handle exceptions already and I think we should.  This gives us even useful stack traces:
>>
>> Error occurred during initialization of VM
>> java.lang.Error: just testing
>> at jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.getCompiler(HotSpotJVMCIRuntime.java:181)
>>
>> Here is the updated patch:
>>
>> diff -r d84bd22ab531 src/share/vm/compiler/compileBroker.cpp
>> --- a/src/share/vm/compiler/compileBroker.cpp Wed Dec 09 14:54:40 2015 +0100
>> +++ b/src/share/vm/compiler/compileBroker.cpp Mon Dec 14 12:20:18 2015 -1000
>> @@ -56,6 +56,7 @@
>> #if INCLUDE_JVMCI
>> #include "jvmci/jvmciCompiler.hpp"
>> #include "jvmci/jvmciRuntime.hpp"
>> +#include "jvmci/jvmciJavaClasses.hpp"
>> #include "runtime/vframe.hpp"
>> #endif
>> #ifdef COMPILER2
>> @@ -498,7 +499,7 @@ CompilerCounters::CompilerCounters() {
>> // CompileBroker::compilation_init
>> //
>> // Initialize the Compilation object
>> -void CompileBroker::compilation_init() {
>> +void CompileBroker::compilation_init(TRAPS) {
>>  _last_method_compiled[0] = '\0';
>>
>>  // No need to initialize compilation system if we do not use it.
>> @@ -529,6 +530,17 @@ void CompileBroker::compilation_init() {
>>      } else {
>>        c1_count = JVMCIHostThreads;
>>      }
>> +
>> +      if (!UseInterpreter) {
>> +        // Force initialization of JVMCI compiler otherwise JVMCI
>> +        // compilations will not block until JVMCI is initialized
>> +        ResourceMark rm;
>> +        TempNewSymbol getCompiler = SymbolTable::new_symbol("getCompiler", CHECK);
>> +        TempNewSymbol sig = SymbolTable::new_symbol("()Ljdk/vm/ci/runtime/JVMCICompiler;", CHECK);
>> +        Handle jvmciRuntime = JVMCIRuntime::get_HotSpotJVMCIRuntime(CHECK);
>> +        JavaValue result(T_OBJECT);
>> +        JavaCalls::call_virtual(&result, jvmciRuntime, HotSpotJVMCIRuntime::klass(), getCompiler, sig, CHECK);
>> +      }
>>    }
>>  }
>> #endif // INCLUDE_JVMCI
>> diff -r d84bd22ab531 src/share/vm/compiler/compileBroker.hpp
>> --- a/src/share/vm/compiler/compileBroker.hpp Wed Dec 09 14:54:40 2015 +0100
>> +++ b/src/share/vm/compiler/compileBroker.hpp Mon Dec 14 12:20:18 2015 -1000
>> @@ -276,7 +276,7 @@ public:
>>    CompileQueue *q = compile_queue(comp_level);
>>    return q != NULL ? q->size() : 0;
>>  }
>> -  static void compilation_init();
>> +  static void compilation_init(TRAPS);
>>  static void init_compiler_thread_log();
>>  static nmethod* compile_method(const methodHandle& method,
>>                                 int osr_bci,
>> diff -r d84bd22ab531 src/share/vm/runtime/thread.cpp
>> --- a/src/share/vm/runtime/thread.cpp Wed Dec 09 14:54:40 2015 +0100
>> +++ b/src/share/vm/runtime/thread.cpp Mon Dec 14 12:20:18 2015 -1000
>> @@ -3628,7 +3628,7 @@ jint Threads::create_vm(JavaVMInitArgs*
>>
>>  // initialize compiler(s)
>> #if defined(COMPILER1) || defined(COMPILER2) || defined(SHARK) || INCLUDE_JVMCI
>> -  CompileBroker::compilation_init();
>> +  CompileBroker::compilation_init(CHECK_JNI_ERR);
>> #endif
>>
>>  // Pre-initialize some JSR292 core classes to avoid deadlock during class loading.
>>
>

> On 14 Dec 2015, at 23:00, Christian Thalinger <[hidden email]> wrote:
>
> Alright, I’ll push that.
>
>> On Dec 14, 2015, at 12:46 PM, Doug Simon <[hidden email]> wrote:
>>
>> I was tempted to do that myself but opted for keeping the patch minimal. However, the solution that propagates the exception is obviously better.
>>
>>> On 14 Dec 2015, at 22:24, Christian Thalinger <[hidden email]> wrote:
>>>
>>>
>>>> On Dec 13, 2015, at 11:49 AM, Doug Simon <[hidden email]> wrote:
>>>>
>>>> In blocking compilation mode (i.e., -UseInterpreter), certain compilations are forced to be non-blocking if a JVMCI compiler is being used (i.e., +UseJVMCICompiler). This is to prevent deadlocks than can occur between an application thread and a JVMCI compiler thread. One condition for forcing a compilation to be non-blocking is if the JVMCI compiler not yet initialized. This is problematic for tests that attempt to force a method to be compiled by JVMCI (e.g., -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI -XX:+UseJVMCICompiler -XX:-TieredCompilation -Xcomp -XX:CompileCommand=compileonly,Tester_*::*). If the test is small enough, JVMCI initialization (which is lazy) may still be executing when the test methods are scheduled to be compiled.
>>>>
>>>> The solution is to make JVMCI compiler initialization eager in blocking compilation mode.
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8145270
>>>> http://cr.openjdk.java.net/~dnsimon/8145270/
>>>
>>> Looks good.  The only comment I have is at that point in initialization we can handle exceptions already and I think we should.  This gives us even useful stack traces:
>>>
>>> Error occurred during initialization of VM
>>> java.lang.Error: just testing
>>> at jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.getCompiler(HotSpotJVMCIRuntime.java:181)
>>>
>>> Here is the updated patch:
>>>
>>> diff -r d84bd22ab531 src/share/vm/compiler/compileBroker.cpp
>>> --- a/src/share/vm/compiler/compileBroker.cpp Wed Dec 09 14:54:40 2015 +0100
>>> +++ b/src/share/vm/compiler/compileBroker.cpp Mon Dec 14 12:20:18 2015 -1000
>>> @@ -56,6 +56,7 @@
>>> #if INCLUDE_JVMCI
>>> #include "jvmci/jvmciCompiler.hpp"
>>> #include "jvmci/jvmciRuntime.hpp"
>>> +#include "jvmci/jvmciJavaClasses.hpp"
>>> #include "runtime/vframe.hpp"
>>> #endif
>>> #ifdef COMPILER2
>>> @@ -498,7 +499,7 @@ CompilerCounters::CompilerCounters() {
>>> // CompileBroker::compilation_init
>>> //
>>> // Initialize the Compilation object
>>> -void CompileBroker::compilation_init() {
>>> +void CompileBroker::compilation_init(TRAPS) {
>>> _last_method_compiled[0] = '\0';
>>>
>>> // No need to initialize compilation system if we do not use it.
>>> @@ -529,6 +530,17 @@ void CompileBroker::compilation_init() {
>>>     } else {
>>>       c1_count = JVMCIHostThreads;
>>>     }
>>> +
>>> +      if (!UseInterpreter) {
>>> +        // Force initialization of JVMCI compiler otherwise JVMCI
>>> +        // compilations will not block until JVMCI is initialized
>>> +        ResourceMark rm;
>>> +        TempNewSymbol getCompiler = SymbolTable::new_symbol("getCompiler", CHECK);
>>> +        TempNewSymbol sig = SymbolTable::new_symbol("()Ljdk/vm/ci/runtime/JVMCICompiler;", CHECK);
>>> +        Handle jvmciRuntime = JVMCIRuntime::get_HotSpotJVMCIRuntime(CHECK);
>>> +        JavaValue result(T_OBJECT);
>>> +        JavaCalls::call_virtual(&result, jvmciRuntime, HotSpotJVMCIRuntime::klass(), getCompiler, sig, CHECK);
>>> +      }
>>>   }
>>> }
>>> #endif // INCLUDE_JVMCI
>>> diff -r d84bd22ab531 src/share/vm/compiler/compileBroker.hpp
>>> --- a/src/share/vm/compiler/compileBroker.hpp Wed Dec 09 14:54:40 2015 +0100
>>> +++ b/src/share/vm/compiler/compileBroker.hpp Mon Dec 14 12:20:18 2015 -1000
>>> @@ -276,7 +276,7 @@ public:
>>>   CompileQueue *q = compile_queue(comp_level);
>>>   return q != NULL ? q->size() : 0;
>>> }
>>> -  static void compilation_init();
>>> +  static void compilation_init(TRAPS);
>>> static void init_compiler_thread_log();
>>> static nmethod* compile_method(const methodHandle& method,
>>>                                int osr_bci,
>>> diff -r d84bd22ab531 src/share/vm/runtime/thread.cpp
>>> --- a/src/share/vm/runtime/thread.cpp Wed Dec 09 14:54:40 2015 +0100
>>> +++ b/src/share/vm/runtime/thread.cpp Mon Dec 14 12:20:18 2015 -1000
>>> @@ -3628,7 +3628,7 @@ jint Threads::create_vm(JavaVMInitArgs*
>>>
>>> // initialize compiler(s)
>>> #if defined(COMPILER1) || defined(COMPILER2) || defined(SHARK) || INCLUDE_JVMCI
>>> -  CompileBroker::compilation_init();
>>> +  CompileBroker::compilation_init(CHECK_JNI_ERR);
>>> #endif
>>>
>>> // Pre-initialize some JSR292 core classes to avoid deadlock during class loading.
>>>
>>
>