RFR: 8288966: Better handle very spiky promotion in G1 [v3]

[Kim Barrett]

On Mon, 8 Aug 2022 13:15:03 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   please review this change that implements "boosting" the PLAB sizes after every "significant" amount of refills.
>> 
>> This removes pause time spikes during garbage collection if the PLAB sizing algorithm has not picked up a good value yet (at startup) or the load is very spiky (e.g. SPECjbb2015).
>> 
>> This works by defining a "tolerated amount of refills", after every time this threshold is met we double the PLAB. The reason for not doing this doubling every time is waste - on tight(er) heaps this can waste a lot of memory for very small allocations, causing more GCs and effective slowdowns.
>> 
>> So this threshold is based on the number of expected refills (per thread); allowing a little more than these expected refills (and discounting the initial PLAB refill).
>> This boosting is also only active with `ResizePLAB` - it's on by default anyway with G1.
>> 
>> The CR contains a graph showing massive pause time spikes on SPECjbb2015 that are fixed by this, as otherwise during these spikes there would be 500k+ PLAB refills that in particular seem to be very harmful on aarch64 (the machines seem to be very problematic wrt to contention on a single atomic counter with lots of threads).
>> 
>> Overall performance (of SPECjbb2015, on aarch64) is slightly better with that change, and in general also improves startup pauses. x64 isn't really affected either way.
>> 
>> I'm aware that now `G1PLABAllocator` has lots of members that are arrays of the same size (the number of generations). I want to fix that to an array of structs in a separate change.
>> 
>> Thanks,
>>   Thomas
>> 
>> Testing: tier1-5
>
> Thomas Schatzl has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:
> 
>  - Merge branch 'master' into 8288966-plab-boost
>  - iwalulya review
>  - Fix whitespace
>  - Only boost with ResizePLAB
>  - Refactoring, fix boundless boosting
>  - some refinement
>  - Some cleanup
>  - More removal of conditional code
>  - Optimize
>  - some cleanup
>  - ... and 2 more: https://git.openjdk.org/jdk/compare/7676be8a...ce2701cb

Looks good.

src/hotspot/share/gc/g1/g1Allocator.cpp line 314:

> 312:     // Make the tolerated refills a huge number; -2 because we add one to this
> 313:     // value later and it would overflow otherwise.
> 314:     _tolerated_refills = (size_t)-2;

`SIZE_MAX - 1` would be more obvious.

src/hotspot/share/gc/g1/g1ParScanThreadState.hpp line 155:

> 153:   // Pass locally gathered statistics to global state. Returns the total number of
> 154:   // HeapWords copied.
> 155:   size_t flush(size_t* surviving_young_words, uint num_workers);

[pre-existing] I dislike the name of this function.  Seems like it should be `flush_stats` or something like that.

-------------

Marked as reviewed by kbarrett (Reviewer).

PR: https://git.openjdk.org/jdk/pull/9726