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