RFR: JDK-8151605: Change warning() to log_warning(gc) in the GC code

[Bengt Rutisson]

Hi Jesper,

Thanks for looking at this!

On 2016-03-10 14:35, Jesper Wilhelmsson wrote:
> Looks good in general. A few minor nits (just vaguely related to your 
> change):
>
> * gc/cms/concurrentMarkSweepGeneration.cpp The comment say it prints a 
> warning, but we print a debug message. Not sure if we care.
> 5777 // We print a warning message only once per CMS cycle.
> 5778 log_debug(gc)(" (benign) Hit CMSMarkStack max size limit");

Yes, not sure what is best here. It's a message that something is not 
perfect, so in that sense a warning, but we won't run in to real 
problems. So, it is probably not right to use log_warning() but it is 
kind of a warning message anyway. I would prefer to just leave it as it is.

>
> * gc/parallel/gcTaskThread.cpp you changed the indentation to four 
> spaces. The line below (unchanged) is also badly indented and could be 
> fixed.

Nice catch. Fixed.

>
> * gc/parallel/psOldGen.cpp the if around the logging could use { }

Right. But is also nice to keep this to one line. I'll leave it as it is.

>
> * gc/parallel/psParallelCompact.cpp you changed the number of spaces 
> in the first two log messages. I'm not sure if anyone is parsing this 
> logging and I would argue that your change makes it better. Just 
> wanted to point out that the actual message is changed here to make 
> sure it was done on purpose.

Good catch. Yes, this was intentional. The messages looked very odd 
before. I should have mentioned that in my original post.

>
> * gc/shared/cardGeneration.cpp the if around the logging could use { }


Same as in psOldGen.cpp. I'll leave this as it is.

>
> Fix it if you feel like it :)  No new webrev is called for.

Thanks again for looking at this!

Bengt

>
> Thanks,
> /Jesper
>
>
> Den 10/3/16 kl. 11:00, skrev Bengt Rutisson:
>>
>> Hi everyone,
>>
>> Could I have a couple of reviews for this change?
>>
>> https://bugs.openjdk.java.net/browse/JDK-8151605
>> http://cr.openjdk.java.net/~brutisso/8151605/webrev.00/
>>
>> With the new logging framework we should use log_warning() instead of 
>> warning().
>>
>> Thanks,
>> Bengt