RFR (M): 8201492: Properly implement non-contiguous generations for reference discovery

[Stefan Johansson]

Hi Thomas,

On 2018-04-26 16:12, Thomas Schatzl wrote:
> Hi all,
> 
>    could someone please have a look at this?
> 
> Thanks,
>    Thomas
> 
> On Wed, 2018-04-18 at 08:52 +0200, Thomas Schatzl wrote:
>> Hi all,
>>
>>    can I have reviews for this change that adapts the reference
>> discovery for the non-contiguous generations (and non-contiguous
>> generations in general) of G1 to solve one day-one performance issue?
>>
>> So reference discovery does not properly support non-contiguous
>> generations, resorting to an approximation of it.
>>
>> This in turn causes G1 to require the "preserve cm referents" phase
>> during GC during marking, which is very costly in some cases.
>>
>> The reason for the preserve cm referents phase is that References
>> (j.l.ref.References instances) that are discovered by concurrent
>> discovery, will currently prevent discovery and evacuation of
>> referents
>> in the STW pauses, as G1 thinks that it already has discovered that
>> Reference and skips it. Still their referents can be in young gen,
>> while the Reference is in old gen (young gc may iterate over it
>> during
>> card scanning), and this may cause crashes later.
>>
>> The preserve cm referents phase brute-force simply evacuates any
>> "leftover" referents and its followers.
>>
>> This is because the STW reference discovery currently does not treat
>> References in old gen as roots (i.e. to be scanned and referents
>> evacuated always), as the STW reference processor thinks that the g1
>> heap is a single generation spanning the whole heap.
>>
>> By giving the ref processor the correct idea of generations in G1,
>> this
>> automatically works, and obsoletes the "preserve cm referents" phase.
>>
>> To get an understanding how serious this issue may be, on
>> theKitchensink reference stress test program, the "Preserve CM
>> referents"
>> phase may take 105ms out of 115ms.
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8201492
>> Webrev:
>> http://cr.openjdk.java.net/~tschatzl/8201492/webrev/
The change looks good, really nice improvement.

The only thing I can comment on is the introduction of 
SpanReferenceProcessor. I think it would be nicer to just have a single 
ReferenceProcessor that always take a closure for deciding if discovery 
should be done, ie. let the other GCs explicitly create their 
SpanBasedDiscoverer and pass it in.

Cheers,
Stefan

>> Testing:
>> hs-tier1-3, performance verification, lots of Kitchensink reference
>> stress testing runs
>>
>> Thanks,
>>    Thomas
>>
>