RFR (XS) 8144853: Print the names of callees in PrintAssembly/PrintInterprete

[David Holmes]

Sorry for the delay. This looks fine to me.

Thanks,
David

On 14/12/2015 9:48 AM, Ioi Lam wrote:
> Hi David,
>
> I have updated the patch according to your feedback:
>
> http://cr.openjdk.java.net/~iklam/jdk9/8144853-print-interpreter-callee-names.v3/
>
>
> The buf[] variable is now allocated using NEW_RESOURCE_ARRAY, and the
> useless 'found' variable has been removed. The comment is also fixed.
>
> Thanks
> - Ioi
>
> On 12/8/15 8:47 PM, David Holmes wrote:
>> Hi Ioi,
>>
>> src/share/vm/code/nmethod.cpp
>>
>> Your use of the static buffer seems not thread-safe.
>>
>> Minor nit:
>>
>> 3057             bool found;
>> 3058             found = os::dll_address_to_function_name(dest, buf,
>> sizeof(buf), &offset);
>>
>> could be:
>>
>> 3057             bool found = os::dll_address_to_function_name(dest,
>> buf, sizeof(buf), &offset);
>>
>>
>> ---
>>
>> src/share/vm/compiler/disassembler.cpp
>>
>> Your use of the static buffer seems not thread-safe.
>>
>> 366   bool found;
>>
>> This variable seems unused.
>>
>> Typo:
>>
>> 369     // Don't do this for native methods, as the function name will
>> be printed in
>> 370     // by nmethod::reloc_string_for().
>>
>> delete 'in' or 'by'.
>>
>> Thanks,
>> David
>> -----
>>
>>
>> On 8/12/2015 5:32 AM, Ioi Lam wrote:
>>> Hi Vladimir,
>>>
>>> Thanks for the suggestion. I've changed the patch to handle the nmethods
>>> differently than PrintInterpreter:
>>>
>>> http://cr.openjdk.java.net/~iklam/jdk9/8144853-print-interpreter-callee-names.v2/
>>>
>>>
>>>
>>> The nmethod dump now looks like this:
>>>
>>>    0x00007f607940c657: callq    0x00007f607186bb60  ;
>>> ImmutableOopMap{[0]=Oop }
>>>                                                  ;*ifeq {reexecute=1
>>> rethrow=0 return_oop=0}
>>>                                                  ; -
>>> java.lang.String::charAt at 4 (line 685)
>>>                                                  ; {runtime_call
>>> UncommonTrapBlob}
>>>    0x00007f607940c65c: callq    0x00007f608b05eb00  ;*ifeq {reexecute=0
>>> rethrow=0 return_oop=0}
>>>                                                  ; -
>>> java.lang.String::charAt at 4 (line 685)
>>>                                                  ; {runtime_call
>>> os::breakpoint()}
>>>
>>>
>>> Thanks
>>> - Ioi
>>>
>>>
>>> On 12/7/15 10:58 AM, Vladimir Ivanov wrote:
>>>> Ioi,
>>>>
>>>> Nice improvement!
>>>>
>>>> Why don't you print "{runtime_call os::breakpoint()}" for
>>>> -XX:+PrintAssemble instead? It looks more natural w.r.t. current
>>>> format.
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>>
>>>> On 12/7/15 9:18 PM, Ioi Lam wrote:
>>>>> Please review a very small fix:
>>>>>
>>>>> http://cr.openjdk.java.net/~iklam/jdk9/8144853-print-interpreter-callee-names/
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Bug: Print the names of callees in PrintAssembly/PrintInterprete
>>>>>
>>>>>      https://bugs.openjdk.java.net/browse/JDK-8144853
>>>>>
>>>>> Summary of fix:
>>>>>
>>>>>      In -XX:+PrintAssembly and -XX:+PrintInterpreter, sometimes only
>>>>>      the address of a callee is printed, and the name is missing.
>>>>>
>>>>>      The fix is to use os::dll_address_to_function_name() to find the
>>>>>      names of such functions and print them out if possible.
>>>>>
>>>>>      EXAMPLES:
>>>>>      -XX:+PrintInterpreter:
>>>>>      0x00007f1b018447c3: callq 0x00007f1b19b9bba0 =
>>>>> InterpreterRuntime::newarray(JavaThread*, BasicType, int)
>>>>>
>>>>>      -XX:+PrintAssembly
>>>>>      <....>
>>>>>      0x00007f75d87b9629: xchg %ax,%ax
>>>>>      0x00007f75d87b962b: callq 0x00007f75d0c11b60 ;
>>>>> ImmutableOopMap{rbp=Oop }
>>>>>                                                    ;*iflt {reexecute=1
>>>>> rethrow=0 return_oop=0}
>>>>>                                                    ; -
>>>>> java.lang.StringLatin1::charAt at 1 (line 43)
>>>>>                                                    ; {runtime_call
>>>>> UncommonTrapBlob}
>>>>>      0x00007f75d87b9630: callq 0x00007f75e8e41370 = os::breakpoint()
>>>>>                                                    ;*iflt {reexecute=0
>>>>> rethrow=0 return_oop=0}
>>>>>                                                    ; -
>>>>> java.lang.StringLatin1::charAt at 1 (line 43)
>>>>>                                                    ; {runtime_call}
>>>>>
>>>>>
>>>>> TESTS:
>>>>>      RBT hotspot/test/:hotspot_all (this includes tests cases with
>>>>> -XX:+PrintAssembly)
>>>>>
>>>>> Thanks
>>>>> - Ioi
>>>
>