[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-runtime-dev
Subject: Re: RFR: 8146947: Remove PrintOopAddress rather than converting to UL
From: Rachel Protacio <rachel.protacio () oracle ! com>
Date: 2016-03-24 13:34:19
Message-ID: 56F3ECDB.7060305 () oracle ! com
[Download RAW message or body]
Thanks for the reviews, Coleen and Stefan!
Rachel
On 3/23/2016 6:30 PM, Stefan Karlsson wrote:
> On 23/03/16 22:33, Coleen Phillimore wrote:
>>
>> Yeah, I like that. Stefan?
>
> I'm fine with that. Reviewed.
>
> I do appreciate that we want to make the exception log lines look
> pretty, and maybe we could change the code to print something like:
> [0.591s][info][exceptions] Exception java/lang/ClassNotFoundException
> thrown in
> java/net/URLClassLoader.findClass(Ljava/lang/String;)Ljava/lang/Class
>
> on the info level, and print the rest at the debug or trace level.
> But, if we do want that, we should probably handle that as a separate
> RFE.
>
> Thanks,
> StefanK
>
>> Coleen
>>
>>
>> On 3/23/16 5:20 PM, Rachel Protacio wrote:
>>> Ok, how about I just delete the repeat address in the exceptions
>>> logging, i.e. the part in the parentheses? Here's a webrev with
>>> that: http://cr.openjdk.java.net/~rprotacio/8146947.01/
>>>
>>> Rachel
>>>
>>> On 3/23/2016 4:54 PM, Coleen Phillimore wrote:
>>>>
>>>> I thought it was ugly.
>>>>
>>>> busaa027% java -Xlog:exceptions -XX:+PrintOopAddress xxx
>>>> [0.577s][info][exceptions] Exception <a
>>>> 'java/lang/ClassNotFoundException'{0x0000000101c4f120}: xxx>
>>>> (0x0000000101c4f120)
>>>> thrown in interpreter method <{method} {0x00007f670f6b0d70}
>>>> 'findClass' '(Ljava/lang/String;)Ljava/lang/Class;' in 'java/net/URLC>
>>>> at bci 44 for thread 0x00007f694c017000
>>>>
>>>>
>>>> vs.
>>>>
>>>> busaa027% java -Xlog:exceptions xxx
>>>> [0.591s][info][exceptions] Exception <a
>>>> 'java/lang/ClassNotFoundException': xxx> (0x0000000101c4f120)
>>>> thrown in interpreter method <{method} {0x00007f540b0b9d70}
>>>> 'findClass' '(Ljava/lang/String;)Ljava/lang/Class;' in 'java/net/URLC>
>>>>
>>>>
>>>> And redundant! At least here.
>>>>
>>>> Coleen
>>>>
>>>> On 3/23/16 4:31 PM, Stefan Karlsson wrote:
>>>>> Hi,
>>>>>
>>>>> On 22/03/16 23:16, Coleen Phillimore wrote:
>>>>>>
>>>>>>
>>>>>> On 3/22/16 4:06 PM, Rachel Protacio wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 3/22/2016 12:09 PM, Coleen Phillimore wrote:
>>>>>>>>
>>>>>>>> Hi Rachel,
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~rprotacio/8146947/src/share/vm/oops/oop.cpp.udiff.html
>>>>>>>>
>>>>>>>>
>>>>>>>> I think the line
>>>>>>>>
>>>>>>>> *-if (PrintOopAddress)print_address_on(st);*
>>>>>>>>
>>>>>>>>
>>>>>>>> should be removed. For the case of a String that is in output
>>>>>>>> for logging, it doesn't seem like it adds anything and then you
>>>>>>>> wouldn't have to change the tests.
>>>>>>>>
>>>>>>> The part printing from the exceptions logging isn't from
>>>>>>> oop.cpp, but klass.cpp#Klass::print_on(). Should I delete it
>>>>>>> from there instead?
>>>>>>
>>>>>> Ah, thank you for correcting my mistake. I think the address
>>>>>> shouldn't be printed in the logging statement because these are
>>>>>> used by people for debugging Java code, and not jvm code. So,
>>>>>> yes, I think it should be deleted there also. I don't think
>>>>>> printing these addresses are useful for debugging anymore, since
>>>>>> we removed PermGen.
>>>>>
>>>>> Is there a reason you want to remove this except for not having to
>>>>> change the tests? Getting object addresses printed is an important
>>>>> debugging tool for us in the GC team.
>>>>>
>>>>> Thanks,
>>>>> StefanK
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Coleen
>>>>>>
>>>>>>>
>>>>>>> Rachel
>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Coleen
>>>>>>>>
>>>>>>>> On 3/22/16 10:08 AM, Rachel Protacio wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> Please review this fix removing PrintOopAddress as a command
>>>>>>>>> line flag. The printing functionality has been made default,
>>>>>>>>> except for one block which has been removed (see bug
>>>>>>>>> description for justification). A compatibility request has
>>>>>>>>> been accepted with respect to this change.
>>>>>>>>>
>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8146947
>>>>>>>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8146947/
>>>>>>>>>
>>>>>>>>> Passes JPRT and RBT hotspot and non-colo testing.
>>>>>>>>>
>>>>>>>>> Thank you,
>>>>>>>>> Rachel
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
[prev in list] [next in list] [prev in thread] [next in thread]