RFR (XS) 8144853: Print the names of callees in PrintAssembly/PrintInterprete
David Holmes
david.holmes at oracle.com
Wed Dec 16 05:49:03 UTC 2015
As per IM - ResourceMark needed to be added.
Thanks,
David
On 15/12/2015 3:14 PM, David Holmes wrote:
> 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
>>>>
>>
More information about the hotspot-dev
mailing list