RFR: JDK-8138717: TestGCEventMixedWithG1ConcurrentMark.java fails

[David Lindholm]

Bengt,

The change looks good for fixing the test failure. Reviewed.

But we should really have the discussion about what such a test can expect.


Thanks,
David

On 2015-10-08 12:53, Bengt Rutisson wrote:
>
> Hi all,
>
> Could I have a couple of reviews for this fix?
>
> http://cr.openjdk.java.net/~brutisso/8138717/webrev.00/
> https://bugs.openjdk.java.net/browse/JDK-8138717
>
> The fix for JDK-8134953 tried to keep creation of new GC ids the same 
> as before. But there was one exception. When we create heap dumps or 
> do heap inspection before or after a GC we no longer create a new GC 
> id for those events. Instead they use the same GC id as the 
> "enclosing" GC.
>
> This was an intentional change. But in retrospect it should probably 
> not have been done as part of the other changes since
> it breaks some tests that expect the GC id for the heap inspections to 
> be different than the GC id for the actual GC. It should be a separate 
> discussion of whether or not we should have different GC ids for these 
> activities.
>
> Here's the change to collectedHeap.cpp that removes the creation of 
> new GC ids:
> http://hg.openjdk.java.net/jdk9/hs-rt/hotspot/diff/983c56341c80/src/share/vm/gc/shared/collectedHeap.cpp 
>
>
> The proposed patch adds the creation of new GC ids back at the same 
> places. The patch also introduces a GCIdMarkAndRestore class since the 
> "GC" for a heap dump or heap inspected will be nested inside of a 
> normal GC. This is the same behavior as before JDK-8134953, but it 
> means that we can't use a GCIdMark since that would reset the GC id to 
> undefinied in its destructor.
>
> In fact there is one more place where we nest GC ids. It's in 
> GenCollectedHeap::do_collection(). This isn't a real problem (yet) 
> since no one is using the GC id after it has been set to undefinied. 
> But now that we have the GCIdMarkAndRestore anyway, we can use that 
> here too to have the correct GC id available at all times.
>
> Thanks,
> Bengt