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

[Bengt Rutisson]

Thanks, Tom!

Bengt

On 2016-02-10 17:00, Tom Benson wrote:
> 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
>>>>>
>>>
>>
>