RFR: JDK-8149541: Use log_error() instead of log_info() when verification reports a problem

[Tom Benson]

Looks good!
Tom

On 2/10/2016 10:52 AM, Bengt Rutisson wrote:
>
> Hi Tom,
>
> On 2016-02-10 16:34, Tom Benson wrote:
>> Hi Bengt,
>> Did you miss one log.info in 
>> src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp at line 2228?
>>
>> - oop(addr)->print_on(log.info_stream());
>> + oop(addr)->print_on(log.error_stream());
>>        log.info(" (" INTPTR_FORMAT " should have been marked)", 
>> p2i(addr));
>>
>
> Good catch!
>
> Updated webrev:
> http://cr.openjdk.java.net/~brutisso/8149541/webrev.02/
>
> Diff compared to last version:
> http://cr.openjdk.java.net/~brutisso/8149541/webrev.01-02.diff/
>
> Thanks,
> Bengt
>
>> Tom
>>
>> On 2/10/2016 10:25 AM, Jesper Wilhelmsson wrote:
>>> Looks good!
>>> /Jesper
>>>
>>> Den 10/2/16 kl. 16:12, skrev Bengt Rutisson:
>>>>
>>>> Hi Jesper,
>>>>
>>>> Thanks for looking at this!
>>>>
>>>> On 2016-02-10 15:41, Jesper Wilhelmsson wrote:
>>>>> Hi,
>>>>>
>>>>> Slightly unrelated to the actual change, but the "Verification 
>>>>> failed" message
>>>>> in concurrentMarkSweepGeneration.cpp could be a bit more 
>>>>> informative, similar
>>>>> to the message printed by the fatal call below. I was about to 
>>>>> write that the
>>>>> log message was redundant due to this fatal call, but the log 
>>>>> message in the
>>>>> fatal call looks like it's not printed on all platforms.
>>>>
>>>> Sounds good. Here's an updated webrev:
>>>> http://cr.openjdk.java.net/~brutisso/8149541/webrev.00/
>>>>
>>>> and the diff compared to the last one:
>>>> http://cr.openjdk.java.net/~brutisso/8149541/webrev.00-01.diff/
>>>>
>>>>>
>>>>> Besides that it looks good.
>>>>
>>>> Great! Thanks!
>>>>
>>>> Bengt
>>>>
>>>>> /Jesper
>>>>>
>>>>>
>>>>> Den 10/2/16 kl. 13:43, skrev Bengt Rutisson:
>>>>>>
>>>>>> Hi everyone,
>>>>>>
>>>>>> Could I have a couple of reviews for this change?
>>>>>>
>>>>>> http://cr.openjdk.java.net/~brutisso/8149541/webrev.00/
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8149541
>>>>>>
>>>>>> When the GC code was converted to use the unified logging 
>>>>>> framework the
>>>>>> verification logging was changed to mostly use:
>>>>>>
>>>>>> log_info(gc, vefiy)("Information about verification failure");
>>>>>>
>>>>>> The problem with this is that some verification, in particular in 
>>>>>> G1, does not
>>>>>> report the relevant information in asserts and guarnatee 
>>>>>> messages. Instead the
>>>>>> information is logged ahead of time and at some later point there 
>>>>>> is something
>>>>>> like a "guarantee(false, "Verification failed.");"
>>>>>>
>>>>>> So, to know what went wrong you really need the information that 
>>>>>> was logged.
>>>>>> However when it is logged on log_info(gc, verify) you need to 
>>>>>> have remembered to
>>>>>> set -Xlog:gc* on the command line to get this information.
>>>>>>
>>>>>> A better solution is to log failure information at the error 
>>>>>> level. That way it
>>>>>> is always logged.
>>>>>>
>>>>>> Thanks,
>>>>>> Bengt
>>>>
>>
>