RFR (M): 8087198: G1 card refinement: batching, sorting

[erik.osterlund at oracle.com]

Hi Man,

Wow - a blast from the past! Since this is based on my old prototype 
patch from 2015, I thought I should probably provide some feedback.

First of all, thanks for productizing my idea.

The idea why I saved the MemRegions in my prototype, as far as I can 
remember, was so that I could coalesce consecutive ranges, which to me 
had the perceived benefit of avoiding redundant lookups for e.g. which 
object is the first from the block offset table, when you have 
consecutive cards. It might have been a premature optimization, I'm not 
sure. But I thought that I should at least explain what the thought was 
(if I remember correctly - it was a while ago). Investigating if that's 
worth it or not as a separate thing seems reasonable.

Looks good to me, and thanks for taking this further.

Thanks,
/Erik

On 11/12/19 3:26 AM, Man Cao wrote:
> Hi all,
>
> Can I have reviews for an updated implementation for batching card
> refinement?
> RFE: https://bugs.openjdk.java.net/browse/JDK-8087198
> Webrev: https://cr.openjdk.java.net/~manc/8087198/webrev.00/
>
> Old review thread is:
> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-June/013798.html.
> Major differences from the 2015 webrev:
>   - New version does not save the MemRegions for the cards in a buffer. I
> noticed considerable memory overhead with BigRamTester if we save the
> MemRegions.
>   - New version handles SuspendibleThreadSetJoiner::should_yield() in a more
> timely fashion. Instead of forcing refining all buffered cards, the new
> version can abandon the buffered cards.
>   - New version only batches and sorts the cards, not joining and
> prefetching. I have not investigated whether joining and prefetching help
> much. I think it is OK to investigate them in a separate RFE later.
> Please refer to the RFE page for some performance results.
>
> For correctness, tested with:
>   - Submit repo: tier1
>   - Local fastdebug build: tier2
>   - Fastdebug stress testing DaCapo h2 and BigRamTester with following
> option combinations in addition to -XX:+VerifyRememberedSets:
>     default options
>    -XX:-G1UseAdaptiveConcRefinement -XX:G1UpdateBufferSize=4
> -XX:G1ConcRefinementGreenZone=0 -XX:G1ConcRefinementYellowZone=1
>    -XX:G1ConcRefinementThreads=0
>
> -Man