RRF: JEP-271: Unified GC Logging
Bengt Rutisson
bengt.rutisson at oracle.com
Tue Dec 1 13:09:27 UTC 2015
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
>>>
>>
>
More information about the hotspot-gc-dev
mailing list