<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix"><br>
Hi Ramki,<br>
<br>
On 02/09/15 01:36, Srinivas Ramakrishna wrote:<br>
</div>
<blockquote
cite="mid:CABzyjykSwDSwqz_qKAaU4OrXGe3hgH7r6XfHR8K37J+9Vo2hGA@mail.gmail.com"
type="cite">
<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>
</div>
</blockquote>
<br>
I've fixed the whit-space changes. Thanks for pointing them out,
Thomas!<br>
<br>
<blockquote
cite="mid:CABzyjykSwDSwqz_qKAaU4OrXGe3hgH7r6XfHR8K37J+9Vo2hGA@mail.gmail.com"
type="cite">
<div dir="ltr">
<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>
</div>
</blockquote>
<br>
It's very easy to write a test that verifies the log entries. And I
agree with Thomas that it is worth while to have that test. No
really as a specification, but Just to make us aware when we do a
change. <br>
<br>
I've included the test in the updated webrev that I just sent out.<br>
<br>
Thanks,<br>
Bengt<br>
<br>
<blockquote
cite="mid:CABzyjykSwDSwqz_qKAaU4OrXGe3hgH7r6XfHR8K37J+9Vo2hGA@mail.gmail.com"
type="cite">
<div dir="ltr">
<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 moz-do-not-send="true"
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 moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Ebrutisso/8133818/webrev.00/"
rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~brutisso/8133818/webrev.00/</a><br>
> <a moz-do-not-send="true"
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>
</blockquote>
<br>
</body>
</html>