RFR: 8254739: G1: Optimize evacuation failure for regions with few failed objects [v7]

[Hamlin Li]

On Fri, 10 Sep 2021 12:44:58 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix compilation error on windows
>
> Performance is good as is, particularly the Remove self-forwards phase is much much faster now. I added a figure to the CR [here](https://bugs.openjdk.java.net/secure/attachment/96321/20210902-remove-self-forwards-improvement.png). Really nice.
> 
> Although there are a few existing abnormalities with this change (not newly introduced, the old code is as bad), see [JDK-8273309](https://bugs.openjdk.java.net/browse/JDK-8273309).
> 
> There are a few things that need to be improved:
> * we talked about this before, but I do not think putting `G1EvacuationFailureObjsInHR`, which is something only used in evacuation failure, into `HeapRegion`, should be done. At least at the moment, it is almost never used.
> * I do not like the use and the implementation of `G1EvacuationFailureObjsInHR`: it recreates something thatt is done better elsewhere (mark stack, DCQS buffer allocator, and in particular in the remembered set cardset allocator) with less features, in a non-fitting style.
> Examples are something like reuse of available chunks, concurrent freeing of chunks, automatic resizing of blocks on demand to decrease allocations and potentially other stuff seems something we would really really want.
> * as far as I understand the implementation of `G1EvacuationFailureObjsInHR` ;) - there is zero documentation about the basic structure - it seems to allocate quite a lot of memory that is almost never used. Even in the case of evacuation failure it's likely to be almost empty.
> `G1EvacuationFailureObjsInHR` seems to basically be a preallocated `ArrayList` of chunks (the `_array_list` member).
> Afaict it uses like 1/256 of total heap size (i.e. 0.3%), that's way way too much ( 1 / (256 * sizeof(HeapWord)) * sizeof(pointer); in `G1EvacuationFailureObjsInHR.cpp:86`; the `Array` constructor in `G1EvacuationFailureObjsInHR.hpp:124)) ). We will also get into trouble about startup time about this, I'm sure. That, tbh, alone makes it a no-go if I did not miscalculate.
> 
> Let's work on renaming and reusing the infrastructure provided by the remembered set (`G1CardSetAllocator` etc) in `g1CardSetMemory.hpp`.

@tschatzl Hi Thomas, as we discussed earlier, failed object array should be attached to HeapRegion, and a new container data structure will be added in 8272978, I see 8272978 is still in progress, so for now do you think it's OK for me to just attach G1EvacuationFailureObjsInHR to HeapRegion, and later refactor it after 8272978?

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

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