RRF: JEP-271: Unified GC Logging

Wed Dec 2 15:13:11 UTC 2015

Hi,

I’m surprised that -Xmx512m is still being used here given that -mx is the fully supported method of specifying max heap size.

Regards,
Kirk

> On Dec 2, 2015, at 2:47 PM, Bengt Rutisson <bengt.rutisson at oracle.com> wrote:
> 
> 
> 
> On 2015-12-02 14:07, David Lindholm wrote:
>> Hi Bengt,
>> 
>> These changes looks good now. Reviewed.
> 
> 
> Thanks for looking at this, David!
> 
> Bengt
> 
>> 
>> /David
>> 
>> On 2015-12-01 14:09, Bengt Rutisson wrote:
>>> 
>>> Hi everyone,
>>> 
>>> Here are updated webrevs that include the comments on the last review request.
>>> 
>>> HotSpot changes:
>>> http://cr.openjdk.java.net/~brutisso/JEP-271/review.02/hotspot-webrev.02/ 
>>> 
>>> Complete changes:
>>> 
>>> Diff compared to last webrev:
>>> http://cr.openjdk.java.net/~brutisso/JEP-271/review.02/hotspot-webrev.01-02.diff/ 
>>> 
>>> 
>>> JDK changes:
>>> 
>>> Complete changes:
>>> http://cr.openjdk.java.net/~brutisso/JEP-271/review.02/jdk-webrev.02/
>>> 
>>> Diff compared to last webrev:
>>> http://cr.openjdk.java.net/~brutisso/JEP-271/review.02/jdk-webrev.01-02.diff/ 
>>> 
>>> 
>>> Except for the comment that were discussed in this email thread there are a couple of other changes:
>>> 
>>> - Most of the implementation parts in gcTraceTime.hpp were moved to gcTraceTime.inline.hpp.
>>> - New tests and new code that use logging were updated.
>>> - Some tests were changed to use less extensive logging.
>>> - A bug gave duplicate timing for Full GCs when going through GenCollectedHeap
>>> - The used calculation for G1 old logging could overflow.
>>> - The @ignore tag was added back to TestLogging.java.
>>> 
>>> Thanks,
>>> Bengt
>>> 
>>> On 2015-11-25 14:00, Bengt Rutisson wrote:
>>>> 
>>>> Hi David,
>>>> 
>>>> Thanks for looking at this again!
>>>> 
>>>> On 2015-11-25 13:29, David Lindholm wrote:
>>>>> Hi Bengt,
>>>>> 
>>>>> My code review for the rest of the files:
>>>>> 
>>>>> defNewGeneration.cpp:
>>>>> 
>>>>> 517-520: Indentation
>>>> 
>>>> Fixed.
>>>> 
>>>>> 766: Shouldn't the tags be "gc, promotion"?
>>>> 
>>>> Yes. Fixed.
>>>> 
>>>>> 
>>>>> collectorPolicy.cpp:
>>>>> 
>>>>> 489: Shouldn't the tags be "gc, heap"?
>>>> 
>>>> Yes, fixed.
>>>> 
>>>>> 
>>>>> genCollectedHeap.cpp:
>>>>> 
>>>>> 1150+1152: Indentation
>>>>> 
>>>>> generation.cpp:
>>>>> 
>>>>> 162: Indentation
>>>> 
>>>> Fixed.
>>>> 
>>>>> 
>>>>> 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.
>>>> 
>>>> Agreed. Fixed.
>>>> 
>>>> 
>>>>> 
>>>>> referenceProcessor.cpp:
>>>>> 
>>>>> 515: Indentation
>>>>> 
>>>>> referenceProcessor.cpp:
>>>>> 
>>>>> 948+953: Indentation
>>>>> 
>>>>> metaspace.cpp:
>>>>> 
>>>>> 1505: Indentation
>>>>> 1578+1580: Indentation
>>>>> 2695: Indentation
>>>>> 
>>>>> runtimeService.cpp:
>>>>> 
>>>>> 123-124: Indentation
>>>> 
>>>> Fixed.
>>>> 
>>>> Thanks,
>>>> Bengt
>>>> 
>>>>> 
>>>>> 
>>>>> 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
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20151202/d0119a58/attachment.htm>