RFR: 8194823: Serial GC does not account GCs caused by TLAB allocation in GC overhead limit [v2]

[Guoxiong Li]

On Thu, 25 May 2023 10:48:49 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Guoxiong Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix parameter list formatting issues.
>
> Lgtm apart from parameter list formatting issues. Please fix before committing.
> 
> It's a bit unfortunate that support for this feature for one collector causes so many changes everywhere else. (But also indicates an issue with all other collectors not supporting it ;)). Maybe the parameters could be wrapped into something like an "AllocationRequest", but this is a) a separate issue, and b) needs to be discussed first with others.

@tschatzl Thanks for the review. I fixed the formatting issues just now.

> src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp line 518:
> 
>> 516: 
>> 517: HeapWord* ParallelScavengeHeap::allocate_new_tlab(size_t min_size, size_t requested_size, size_t* actual_size,
>> 518:                                                   bool* gc_overhead_limit_was_exceeded) {
> 
> Suggestion:
> 
> HeapWord* ParallelScavengeHeap::allocate_new_tlab(size_t min_size,
>                                                   size_t requested_size,
>                                                   size_t* actual_size,
>                                                   bool* gc_overhead_limit_was_exceeded) {
> 
> Parameter list formatting

Fixed.

> src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp line 106:
> 
>> 104:  protected:
>> 105:   HeapWord* allocate_new_tlab(size_t min_size, size_t requested_size, size_t* actual_size,
>> 106:                               bool* gc_overhead_limit_was_exceeded) override;
> 
> Suggestion:
> 
>   HeapWord* allocate_new_tlab(size_t min_size,
>                               size_t requested_size,
>                               size_t* actual_size,
>                               bool* gc_overhead_limit_was_exceeded) override;
> 
> Parameter formatting

Fixed.

> src/hotspot/share/gc/shared/memAllocator.cpp line 325:
> 
>> 323:   size_t min_tlab_size = ThreadLocalAllocBuffer::compute_min_size(_word_size);
>> 324:   mem = Universe::heap()->allocate_new_tlab(min_tlab_size, new_tlab_size, &allocation._allocated_tlab_size,
>> 325:                                             &allocation._overhead_limit_exceeded);
> 
> Suggestion:
> 
>   mem = Universe::heap()->allocate_new_tlab(min_tlab_size,
>                                             new_tlab_size,
>                                             &allocation._allocated_tlab_size,
>                                             &allocation._overhead_limit_exceeded);
> 
> Parameter formatting

Fixed.

> src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp line 535:
> 
>> 533: 
>> 534:   HeapWord* allocate_new_tlab(size_t min_size, size_t requested_size, size_t* actual_size,
>> 535:                               bool* gc_overhead_limit_was_exceeded) override;
> 
> Suggestion:
> 
>   HeapWord* allocate_new_tlab(size_t min_size,
>                               size_t requested_size,
>                               size_t* actual_size,
>                               bool* gc_overhead_limit_was_exceeded) override;

Fixed.

> src/hotspot/share/gc/x/xCollectedHeap.cpp line 150:
> 
>> 148: 
>> 149: HeapWord* XCollectedHeap::allocate_new_tlab(size_t min_size, size_t requested_size, size_t* actual_size,
>> 150:                                             bool* gc_overhead_limit_was_exceeded) {
> 
> Suggestion:
> 
> HeapWord* XCollectedHeap::allocate_new_tlab(size_t min_size,
>                                             size_t requested_size,
>                                             size_t* actual_size,
>                                             bool* gc_overhead_limit_was_exceeded) {

Fixed.

> src/hotspot/share/gc/z/zCollectedHeap.cpp line 145:
> 
>> 143: 
>> 144: HeapWord* ZCollectedHeap::allocate_new_tlab(size_t min_size, size_t requested_size, size_t* actual_size,
>> 145:                                             bool* gc_overhead_limit_was_exceeded) {
> 
> Suggestion:
> 
> HeapWord* ZCollectedHeap::allocate_new_tlab(size_t min_size,
>                                             size_t requested_size,
>                                             size_t* actual_size,
>                                             bool* gc_overhead_limit_was_exceeded) {

Fixed.

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

PR Comment: https://git.openjdk.org/jdk/pull/14120#issuecomment-1562734243
PR Review Comment: https://git.openjdk.org/jdk/pull/14120#discussion_r1205370192
PR Review Comment: https://git.openjdk.org/jdk/pull/14120#discussion_r1205370232
PR Review Comment: https://git.openjdk.org/jdk/pull/14120#discussion_r1205370289
PR Review Comment: https://git.openjdk.org/jdk/pull/14120#discussion_r1205370349
PR Review Comment: https://git.openjdk.org/jdk/pull/14120#discussion_r1205370388
PR Review Comment: https://git.openjdk.org/jdk/pull/14120#discussion_r1205370436