RFR: 8285710: miscalculation of G1CardSetAllocator wasted memory size

[Thomas Schatzl]

On Wed, 27 Apr 2022 11:03:18 GMT, tqxia <duke at openjdk.java.net> wrote:

> when calculating the wasted memory size of G1CardSetAllocator, the code erroneously substracted both _segmented_array.num_allocated_slots() and _free_slots_list.pending_count() from _segmented_array.num_available_slots().
> 
> The correct formula should be: num_wasted_slots = _segmented_array.num_available_slots() - (_segmented_array.num_allocated_slots() - (uint)_free_slots_list.pending_count()).
> 
> This can potentially leads to an arithmetic overflow and misleading information will be displayed when G1SummarizeRSetStatsPeriod is set.

Hi @tqxia !

  after some internal discussion we found that the term "wasted" is too overloaded - it's typically used in gc for memory that is free but can't be used for some reason (which is in some cases arguable which is which). This is not the case here, the value returned by the `wasted_mem_size()` method is simply the amount of memory that is free for allocation in the remset data structure.

Further, there is a problem in the documentation of `SegmentedArray::_num_available_slots` and `_num_allocated_slots` that might have led to this issue, and as far as we can tell also indicates that this change is incorrect too. 

So I filed a new issue https://bugs.openjdk.java.net/browse/JDK-8286189 that fixes that and I am going to post later. I recommend waiting just a bit for this simple rename.

src/hotspot/share/gc/g1/g1CardSetMemory.cpp line 66:

> 64:   uint num_wasted_slots = _segmented_array.num_available_slots() -
> 65:                          (_segmented_array.num_allocated_slots() -
> 66:                           (uint)_free_slots_list.pending_count());

I think the new calculation is wrong too. This is probably because of wrong documentation of `SegmentedArray::_num_available_slots` and `_num_allocated_slots`; in particular, `_num_allocated_slots` contains both free and pending counts.

So the correct solution would be (a bit abbreviated) I think:

unused_slots = (available - allocated) + _free_slots_list.free_count()

The first term calculates the not yet used slots, and the second directly adds the free slots.

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

Changes requested by tschatzl (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8424