RFR: 8165443: Free Collection Set serial phase takes very long on large heaps

[Stefan Johansson]

Hi Kim,

> 6 dec. 2019 kl. 01:33 skrev Kim Barrett <kim.barrett at oracle.com>:
> 
>> On Dec 5, 2019, at 1:27 PM, Stefan Johansson <stefan.johansson at oracle.com> wrote:
>> Thanks, here are updated webrevs:
>> Full: http://cr.openjdk.java.net/~sjohanss/8165443/02/
>> Inc: http://cr.openjdk.java.net/~sjohanss/8165443/01-02/
> 
> Looks good.
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1CollectedHeap.cpp
> 4207   class G1FreeCollectionSetClosure : public HeapRegionClosure {
> 
> Since it's a nested class, it could be called FreeCSetClosure, to be
> similar to FreeCSetStats.  Just a suggestion.
> 
I like to keep names similar, so I changed this as you suggested.

> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/heapRegionManager.cpp
> 680   uint const num_workers = clamp(max_length(), 1u, workers->active_workers());
> 
> Maybe this was already discussed, but it looks to me that the cost per
> region for the rebuild task is pretty low, so that it might be that
> this should use one thread per N regions, N > 1.  So use "max_length()
> / N".  I don't know what N should be.  For a similar case in reference
> processing we added the experimental option ReferencesPerThread with a
> default value of 1000.  (Note that I'm not encouraging the addition of
> an option here.)
> 
> This should probably be deferred for further experimentation and measurement.
Yes, I mentioned that I didn’t add any heuristic for this and I agree that one worker per region is to much, and in most cases we will already have fewer active workers for small heaps, this clamp was just a way to make sure we don’t have more workers than ever needed. 

I also agree that we could do some experiments and see if we can come up with a better way to size the number of threads here, but for now I think this is ok.

New webrev, full one also contain a change to work with latest WorkerDataArray changes:
Full: http://cr.openjdk.java.net/~sjohanss/8165443/03/
Inc: http://cr.openjdk.java.net/~sjohanss/8165443/02-03/

Thanks for the review,
Stefan

> 
> ------------------------------------------------------------------------------
>