RFR (M): JDK-8043607: Add a GC id as a log decoration similar to PrintGCTimeStamps

Bengt Rutisson bengt.rutisson at oracle.com
Thu Jun 5 20:58:33 UTC 2014


Hi Thomas,

Thanks for looking at this!

On 6/5/14 4:28 PM, Thomas Schatzl wrote:
> Hi Bengt,
>
> On Wed, 2014-06-04 at 17:06 +0200, Bengt Rutisson wrote:
>> Hi Erik,
>>
>> On 5/28/14 2:29 PM, Erik Helin wrote:
>>> Hi Bengt,
>>>
>>> just a first quick general comment: would it make sense for GCId to be
>>> in its own file or class, so the GC logging don't have to depend on
>>> the GC trace framework?
>> Good point.
>>
>> Here is an updated webrev where I separeted out GCId to its own file:
>> http://cr.openjdk.java.net/~brutisso/8043607/webrev.02/
>>
>> Here's just the diff compared to the earlier version:
>> http://cr.openjdk.java.net/~brutisso/8043607/webrev.01-02.diff/
>>
>> Jesper, the updated webrev also contains the spelling mistake you
>> noticed. Thanks for catching that!
>>
>   two minor comments:
>
> - is it useful to check whether we are at a safepoint and running in the
> VM thread when incrementing the gc id?

Good idea. Added an assert for that.

>
> - also it would be nice to have some test case that checks for presence
> of the numbers in log messages (and their absence if disabled).

Yes, good point. I added a simple test.

Here's an updated webrev:
http://cr.openjdk.java.net/~brutisso/8043607/webrev.04/

And here is the diff compared to the previous version:
http://cr.openjdk.java.net/~brutisso/8043607/webrev.03-04.diff/

Note that I also found a minor bug. The peek() method should not use 
"+1" the _next_id is already  the next id, just like the name suggests 
;). This only affect CMS and I noticed the problem since some IDs could 
sometimes be skipped. Now it works fine for CMS too.

I also had to fix includes to make it compile without precompiled headers.

Thanks,
Bengt

>
> Thanks,
>    Thomas
>
>



More information about the hotspot-dev mailing list