RRF: JEP-271: Unified GC Logging

Stefan Johansson stefan.johansson at oracle.com
Fri Nov 20 16:37:53 UTC 2015

Hi Bengt,

Overall I think the change is great, a few comments below.

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/
  632     if (value == 0) {
  633       LogConfiguration::parse_log_arguments("stdout", "gc", NULL, 
  634     } else {
  635       LogConfiguration::parse_log_arguments("stdout", "gc=off", 
  636     }

The condition should be changed to "value != 0" to get the same behavior 
as the old code.

  217                     // We found start of output emitted by 

Please update the comment to say "by -Xlog:gc+age=trace" instead.

   45     // VerifyBeforeGC:[Verifying threads heap tenured eden syms 
strs zone dict metaspace chunks hand C-heap code cache ]
   46     public static final String VERIFY_BEFORE_GC_PATTERN = 
"Verifying Before GC";
   47     // VerifyBeforeGC: VerifyBeforeGC: VerifyBeforeGC:
   48     public static final String VERIFY_BEFORE_GC_CORRUPTED_PATTERN 
= "VerifyBeforeGC:(?!\\[Verifying[^]]+\\])";
   50     // VerifyAfterGC:[Verifying threads heap tenured eden syms 
strs zone dict metaspace chunks hand C-heap code cache ]
   51     public static final String VERIFY_AFTER_GC_PATTERN = 
"Verifying After GC";
   52     // VerifyAfterGC: VerifyAfterGC: VerifyAfterGC:
   53     public static final String VERIFY_AFTER_GC_CORRUPTED_PATTERN = 

Please update the pattern comments to match the new logging better and 
maybe also remove the regexp parts of the corrupted pattern.

   57  new String[] { "-Xlog:gc+verify=debug",
   58                 "-XX:+UseGCLogFileRotation",

UseGCLogFileRotation has been removed and should be removed from the 
test as well.

   52     // UseDynamicNumberOfGCThreads and TraceDynamicGCThreads enabled

Comment should be changed to use -Xlog:gc+task=trace as well.

While looking through the testing directories I found some additional 
changes that could be done.

PrintGC and PrintGCDetails are used here and should be changed to use 
unified logging.

Xloggc is used here with an unmatched quote, so the test is fine but 
would be nice to change it to another X-option.


I will take another round over the hotspot changes next week but so far 
everything else looks good.

> Some tests in the JDK repo have been updated:
> http://cr.openjdk.java.net/~brutisso/JEP-271/review.00/jdk-webrev.00/
JDK changes looks good.


> 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