RFR: JDK-8134953: Make the GC ID available in a central place

[Bengt Rutisson]

Hi everyone,

This is mostly a GC related patch, but it adds a field to the Thread 
class, so I'm sending this out on the broader hotspot-dev list.

Could I have a couple of reviews for this patch?

http://cr.openjdk.java.net/~brutisso/8134953/webrev.01/
https://bugs.openjdk.java.net/browse/JDK-8134953

Currently the GC ID, that is used for event tracing and logging, is 
stored in the GCTracer object. That has caused some minor problems since 
there are multiple GCTracers active at the same time. The correct GC IDs 
need to be passed around in many places.

For some upcoming GC logging changes I would like to have a more 
consistent way of finding the currently active GC ID. I've played around 
with a couple of different solutions, but eventually I found that the 
simplest solution is to store the current GC ID in the thread. That way 
there is a single way to look it up in all places. It is also fairly 
straight forward to set it up.

I've reworked the GCId class a bit to support this and I've introduced a 
GCIdMark class that is a scoped object that helps to set up and tear 
down a current GC ID. By moving the GC ID out from the GCTracer class I 
got rid of a few minor issues as well. One is that we no longer need to 
keep track of the G1 "aborted concurrent GC ID". That was necessary 
before since we did logging *after* we had told the corresponding 
GCTracer that the concurrent cycle was aborted and it had thrown its GC 
ID away. Now the GC ID can stay in scope until we have completed all 
logging.

Also the HeapDumpBeforeFullGC and PrintClassHistogramBeforeFullGC used 
to have to create a new GC ID since their logging was done before we had 
the correct GCTracer set up. Now the GC ID can be available early enough 
for this logging to be using the same (and correct) GC ID as the rest of 
the GC. Same for HeapDumpAfterFullGC and PrintClassHistogramAfterFullGC.

I've added an uint to the Thread class to keep track of the currently 
active GC ID. In the current code there are actually only NamedThreads 
that need the GC ID. So, I'm not sure where the best place is for this 
field is. But it seems like the Thread class contains most of the data, 
even data that is only used by some subclasses, so I opted for putting 
the field in Thread as opposed to in NamedThread. This opens up for 
possible future extensions when Java threads may do part of the GC work. 
However, I'm open to moving the field down to NamedThread if that is 
seen as better.

In the GCTracer class there were many asserts that were checking that 
the GC ID was properly set up. Mostly these assert verify that start/end 
is called correctly and that the other methods are called inside of the 
start/end scope. Now that the GC ID is moved out of the GCTracer class 
these asserts had to be dealt with. I figured there were three ways to 
handle it; just remove them, replace them with check that the GC ID from 
the current thread is correct, or implement a new status field to keep 
track of GC start/end state. Personally I'm not sure these asserts are 
valuable enough to spend time on, so I went for the first approach: 
removing them. I don't think making them use the thread GC ID will be 
appropriate. But if anyone feels strongly about these asserts I can 
implement a separate start/end state.

There are a few "Tasks" in the GC code that are executed by worker 
threads. To make the worker threads use the correct GC ID I make sure 
that the Task constructors copy the GC ID from the initiating thread 
into a local variable. When the task is executed in its worker thread it 
sets up the GC ID based on the local variable.

The proposed change does not alter any logging (except for the small bug 
fix for 
HeapDumpBefore(After)FullGC/PrintClassHistogramBefore(After)FullGC. This 
means that no existing tests need to be updated. In particular the 
test/gc/logging/TestGCId.java test passes even after these changes.


A big thanks to Per, who pre-reviewed these changes and came with some 
really good feedback.

Thanks,
Bengt