RFR: 8273626: G1: Factor out concurrent segmented array from G1CardSetAllocator [v5]

[Albert Mingkun Yang]

On Thu, 14 Oct 2021 09:12:13 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> To finish https://bugs.openjdk.java.net/browse/JDK-8254739, we need a segmented array to store a growing regions index array, in the initial version of that patch, a newly home made segmented array was used, but the memory efficiency is not as good as expected, G1CardSetAllocator is a potential candidate to fullfill the requirement, but need some enhancement.
>> 
>> This is a try to enhance G1CardSetAllocator(and G1CardSetBuffer, G1CardSetBufferList) to support element size less pointer size, and strip this basic function as a more generic segmented array (G1SegmentedArray).
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   refine code; fix polymorphic issue in G1SegmentedArray's alloc_options

Changes requested by ayang (Reviewer).

src/hotspot/share/gc/g1/g1CardSet.cpp line 94:

> 92:   _bitmap_hash_mask = ~(~(0) << _log2_num_cards_in_howl_bitmap);
> 93: 
> 94:   init_card_set_alloc_options();

There's much code duplication btw the two constructors of `G1CardSetConfiguration::G1CardSetConfiguration`. Now that this PR touches it, it could be good if this can be dealt with, instead of repeating more code, `init_card_set_alloc_options();` specifically.

src/hotspot/share/gc/g1/g1CardSet.hpp line 118:

> 116:   // Returns the memory allocation options for the memory objects on the card set heap. The returned
> 117:   // array must be freed by the caller.
> 118:   const G1CardSetAllocOptions* mem_object_alloc_options(uint idx);

Now that this method doesn't do mem alloc, the comment should be revised.

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

> 129:             num_available_nodes,
> 130:             percent_of(num_allocated_nodes - _num_pending_nodes, num_available_nodes),
> 131:             first_array_buffer != nullptr ? first_array_buffer->num_elems() : 0,

How about extracting the logic from the prinf function? Sth like:


const uint highest = _segmented_array.first_array_buffer() != nullptr
                   ? _segmented_array.first_array_buffer()->num_elems()
                   : 0;

src/hotspot/share/gc/g1/g1CardSetMemory.inline.hpp line 44:

> 42: Elem* G1CardSetAllocator<Elem>::allocate() {
> 43:   uint elem_size = _segmented_array.elem_size();
> 44:   assert(elem_size > 0, "instance size not set.");

The var declaration is unnecessary.

src/hotspot/share/gc/g1/g1SegmentedArray.hpp line 127:

> 125:   static const uint BufferAlignment = 4;
> 126:   static const uint MinimumBufferSize = 8;
> 127:   static const uint MaximumBufferSize =  UINT_MAX / 2;

It's not obvious to me why they are public.

src/hotspot/share/gc/g1/g1SegmentedArray.hpp line 129:

> 127:   static const uint MaximumBufferSize =  UINT_MAX / 2;
> 128: 
> 129:   G1SegmentedArrayAllocOptions(uint elem_size, uint initial_num_elems = MinimumBufferSize,

I only see one place calling this constructor, specifying all args. The default arg-value can be drop if they are not used, IMO.

src/hotspot/share/gc/g1/g1SegmentedArray.inline.hpp line 211:

> 209: Elem* G1SegmentedArray<Elem, flag>::allocate() {
> 210:   uint es = elem_size();
> 211:   assert(es > 0, "instance size not set.");

Unnecessary var declaration.

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

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