<div dir="ltr"><div>Hi Bengt, Tony, Thomas -- Â [+serviceability-dev (see below)]<br></div><div><div><br></div><div>Thanks for your (re)reviews, and Bengt, thanks also for shepherding the changes into the open jdk code. Much appreciated!!</div><div>(I'm hoping you can take care of the two white-space modifications Thomas suggested and push the changes.)</div><div><br></div><div>Thomas, I am not quite sure of the value (or, utility in economic terms) of something that checks for the format of the already somewhat ill-defined (or undefined, ad-hoc) verbose +PrintReferenceGC gc output. I'd just rely on the likes of Kirk and others who consume these verbose log options to catch such occasional changes. (For example, when Kim made the change for the processing of cleaners, she probably knew of the change in format and figured it was fine -- I would have probably felt the same. Although in the fullness of time, perhaps it made sense to make the logging more fine-grained and with more information.) Of course, I don't know if, with the new unified logging work going on, there's an existing corpus of tests for JVM generated logging where such a test might make sense. Are there such tests? If so, could someone post a pointer? (I included the serviceability alias for their possible input on the testing aspect of this & possible pointers.) Â In general, there have been so many changes to verbose gc log formatting (gc id's and such etc.) and the flux of changes is so high that I think writing tests for these kinds of things (which have no spec and change often at the whims of the developers) have limited utility. Checking the correctness of the <key, value>s emitted for various counters for something like JFR is another matter and definitely worthwhile.</div><div><br></div><div>thanks!</div><div>-- ramki</div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Sep 1, 2015 at 10:02 AM, Thomas Schatzl <span dir="ltr"><<a href="mailto:thomas.schatzl@oracle.com" target="_blank">thomas.schatzl@oracle.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Tue, 2015-09-01 at 14:54 +0200, Bengt Rutisson wrote:<br>
> Hi everyone,<br>
><br>
> Could I have a couple of reviews for this patch contributed by Ramki?<br>
><br>
> <a href="http://cr.openjdk.java.net/~brutisso/8133818/webrev.00/" rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~brutisso/8133818/webrev.00/</a><br>
> <a href="https://bugs.openjdk.java.net/browse/JDK-8133818" rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8133818</a><br>
><br>
<br>
</span>jvmtiTagMap.cpp, line 3408: the SIZE_FORMAT specifiers need a space<br>
before and after, otherwise it will not compile with newer C++<br>
compilers. Same for referenceProcessor.cpp, 307,<br>
<br>
jvmtiTagMap.hpp, line 125: please fix the format of the method parameter<br>
specifications.<br>
<br>
In general I would be really happy if there were a test checking the<br>
output of -XX:+TraceReferenceGC. If we had had that earlier, this issue<br>
would have not cropped up, and prevents future issues.<br>
<br>
I.e. just the format of "[whateverreference, <bb> refs, <cc> secs] ....<br>
[PhantomReference, <xx> refs, <yyy> secs] ... [Cleaners, <zz> refs,<br>
<aaa> secs] .."<br>
<br>
Thanks,<br>
  Thomas<br>
<br>
<br>
</blockquote></div><br></div>