RFR: 8255746: Make PrintCompilation available on a per method level

[Christian Hagedorn]

On Wed, 12 Oct 2022 19:14:12 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> I will move `elapsedTimer` to top. No strong opinion from myself either.
>> 
>>> I would suggest to keep global PrintCompilation code and add variable to mark that line is already printed. Then check it in you code which checks directive to avoid duplicate.
>> 
>> I don't understand this. Doesn't this just print everything?
>> 
>> ---
>> 
>> We can do method specific compilation before the assert. This change seems to be working:
>> 
>> 
>> diff --git a/src/hotspot/share/compiler/compileBroker.cpp b/src/hotspot/share/compiler/compileBroker.cpp
>> index 8d0a87afc17..181dd28ba45 100644
>> --- a/src/hotspot/share/compiler/compileBroker.cpp
>> +++ b/src/hotspot/share/compiler/compileBroker.cpp
>> @@ -2093,6 +2093,7 @@ CompilerDirectives* DirectivesStack::_bottom = NULL;
>>  //
>>  void CompileBroker::invoke_compiler_on_method(CompileTask* task) {
>>    task->print_ul();
>> +  elapsedTimer time;
>> 
>>    CompilerThread* thread = CompilerThread::current();
>>    ResourceMark rm(thread);
>> @@ -2117,11 +2118,16 @@ void CompileBroker::invoke_compiler_on_method(CompileTask* task) {
>>      // native.  The NoHandleMark before the transition should catch
>>      // any cases where this occurs in the future.
>>      methodHandle method(thread, task->method());
>> -    assert(!method->is_native(), "no longer compile natives");
>> 
>>      // Look up matching directives
>>      directive = DirectivesStack::getMatchingDirective(method, comp);
>>      task->set_directive(directive);
>> +    if (task->directive()->PrintCompilationOption) {
>> +      ResourceMark rm;
>> +      task->print_tty();
>> +    }
>> +
>> +    assert(!method->is_native(), "no longer compile natives");
>> 
>>      // Update compile information when using perfdata.
>>      if (UsePerfData) {
>> @@ -2131,12 +2137,6 @@ void CompileBroker::invoke_compiler_on_method(CompileTask* task) {
>>      DTRACE_METHOD_COMPILE_BEGIN_PROBE(method, compiler_name(task_level));
>>    }
>> 
>> -  if (task->directive()->PrintCompilationOption) {
>> -    ResourceMark rm;
>> -    task->print_tty();
>> -  }
>> -  elapsedTimer time;
>> -
>>    should_break = directive->BreakAtCompileOption || task->check_break_at_flags();
>>    if (should_log && !directive->LogOption) {
>>      should_log = false;
>> 
>> 
>> Although there is still possible that there is failed assertion before printing, i.e. when calling `CompilerThread::current()`. I feel like its ok to just look at hs_err in these cases.
>
> May be we are looking the wrong way. Can we do `task->set_directive(directive)` when we create `task`?
> Then you don't need to move PrintCompilation code - `task->directive()` will be available at the beginning of this method.

> elapsedTimer time; just declares variable. TraceTime uses it (with start/stop) later to accumulate time.
So declaration can be placed anywhere before JVMCI code below.

Got it, thanks for the clarification - then it does not matter much where we place this declaration.

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

PR: https://git.openjdk.org/jdk/pull/10668