RFR: 8273626: G1: Factor out concurrent segmented array from G1CardSetAllocator

[Hamlin Li]

On Thu, 23 Sep 2021 14:05:21 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

> Sorry for the late reply. Coincidentally around release there's also some housekeeping to do.
> 

Thanks Thomas, it's OK, this is not that urgent.

> My main question about this change is: Would it be possible to give a sneak-peek on the use of `G1SegmentedArray` in the [JDK-8254739](https://bugs.openjdk.java.net/browse/JDK-8254739) code? It is impossible to determine if the change fits the intended purpose, and I want to avoid changing `G1SegmentedArray` again later.
> 

Sure, I've put the merged code in a temporary branch, https://github.com/Hamlin-Li/jdk/tree/tmp-merge.segmented-array.speed-remove-self. "speed-remove-self" part is still the old one, I did not change it yet, including the suggestions you gave in #5181 and old home made segmented array implementation; I just use the new G1SegmentedArray in G1EvacuationFailureObjsInHR.

> What _I_ thought was that every region gets a set of elements using `G1SegmentedArray` attached to it, in whatever form. Multiple threads add to that at the same time.
> 

Yes, it's implemented in this way, although later in #5181 we may refactor how G1SegmentedArray is attached to a region, but G1SegmentedArray itself should be fine at that time.

> This means that multiple threads might add new segments to it at the same time, meaning that they may try to append new segments to it. What happens with the thread and the segment that fails? Shouldn't this allocation be reused somehow (maybe for other segmented arrays?).
> 

Current(original) implementation just drop the newly allocated G1SegmentedArrayBuffer if current thread fails, seems it's fine as the race should be rare, should we add the faield one to free list? seems adding to freelist will bring some benefit.

> Also, this seems to drop the concurrent freeing of these segments after GC, which is nice too.
> 
> The remembered set uses the memory provided by the segmented arrays in chunks too, i.e. the `G1CardSetArray` container. Not sure if something like this would be more appropriate here instead of directly using the chunks (but may well be as good).
> 

Not quite sure, seems both have benefit, in `G1CardSetArray` way, it has better design benefit; in G1SegmentedArray::iterate_nodes/G1EvacuationFailureObjsInHR::visit, it might have better performance. I'm OK with both ways, please kindly let me know your decision.

> What are your plans about all this?
> 
> Depending on your design choices (which I do not know) this change may be reasonable or not. Please detail this a bit.
> 
> Also note that the CR title currently is a bit misleading imho, indicating a small change supporting smaller pointer sizes. I would prefer if it would be named something like "Factor out concurrent segmented array from G1CardSetAllocator".

Modified.

> src/hotspot/share/gc/g1/g1SegmentedArray.hpp line 85:
> 
>> 83: 
>> 84:   char* start() const { return _buffer; }
>> 85: 
> 
> Unused, can be removed.

This is used in G1EvacuationFailureObjsInHR::visit, so it depends on the discussion below.

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

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