RRF: JEP-271: Unified GC Logging

[David Lindholm]

Hi Bengt,

My code review for the rest of the files:

defNewGeneration.cpp:

517-520: Indentation
766: Shouldn't the tags be "gc, promotion"?

collectorPolicy.cpp:

489: Shouldn't the tags be "gc, heap"?

genCollectedHeap.cpp:

1150+1152: Indentation

generation.cpp:

162: Indentation

plab.cpp:

153: Plab_size = " SIZE_FORMAT " desired_net_plab_sz = " SIZE_FORMAT ": 
Why the capital P in this printout of one variable name but not the 
other? I think it should be consistent.

referenceProcessor.cpp:

515: Indentation

referenceProcessor.cpp:

948+953: Indentation

metaspace.cpp:

1505: Indentation
1578+1580: Indentation
2695: Indentation

runtimeService.cpp:

123-124: Indentation


Thanks,
David

On 2015-11-23 18:25, Bengt Rutisson wrote:
>
> Hi everyone,
>
> Here is an updated webrev based on the comments from the first review. 
> The changes that have been discussed have only been to the hotspot repo:
>
> http://cr.openjdk.java.net/~brutisso/JEP-271/review.01/webrev.01/
>
> The changes to the JDK repo are the same as in the first webrev:
> http://cr.openjdk.java.net/~brutisso/JEP-271/review.00/jdk-webrev.00/
>
>
> Some partial diffs to make it easier for those who already looked at 
> the first webrev.
>
> The unified logging framework was changed in the way it handles the 
> develop logging. Here are the changes I had to do to accommodate the 
> new develop logging:
>
> http://cr.openjdk.java.net/~brutisso/JEP-271/review.01/webrev.01-develop-logging 
>
>
> Here are the changes I have made to address comments in the code 
> (based on top of the develop logging changes):
>
> http://cr.openjdk.java.net/~brutisso/JEP-271/review.01/webrev.00-01-code-diff 
>
>
> And here are the changes that David has made to address comment in the 
> tests:
>
> http://cr.openjdk.java.net/~brutisso/JEP-271/review.01/webrev.00-01-test-diff 
>
>
> Enjoy!
> Bengt
>
> On 2015-11-19 16:29, Bengt Rutisson wrote:
>>
>> Hi everyone,
>>
>> After three pre-reviews it is time for the fist official review 
>> request for JEP-271 Unified GC Logging.
>>
>> http://openjdk.java.net/jeps/271
>>
>> Most code changes are in the hotspot code:
>> http://cr.openjdk.java.net/~brutisso/JEP-271/review.00/webrev.00/
>>
>> Some tests in the JDK repo have been updated:
>> http://cr.openjdk.java.net/~brutisso/JEP-271/review.00/jdk-webrev.00/
>>
>> As with the pre-reviews I have put togther some examples of what the 
>> new logging looks like:
>> http://cr.openjdk.java.net/~brutisso/JEP-271/review.00/compare.html
>>
>> The intent is that this should cover the bulk of the logging changes. 
>> There will most definitely be some follow up changes where we fix 
>> details in the log messages etc.
>>
>> Among many other old logging flags this changeset removes the two 
>> flags PringGC and PrintGCDetails. These two will be added back with a 
>> follow up changeset, but when they are added back they will be marked 
>> as deprecated.
>>
>> The reason for first removing them and then adding them back is to 
>> get testing without these flags. That way we will know that we clean 
>> out all usages of these flags from our testing.
>>
>> Thanks,
>> Bengt
>