RFR: 8240556: Abort concurrent mark after effective eager reclamation of humongous objects [v4]

[Albert Mingkun Yang]

On Thu, 24 Sep 2020 12:22:04 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>>   please review this change that implements concurrent mark abort as proposed by Liang Mao from Alibaba.
>> 
>> Basically, if at the end of the concurrent start pause we notice that we went below the IHOP threshold (and it has been
>> a concurrent start pause caused by humongous allocation), instead of scheduling a mark operation, we schedule a
>> "concurrent undo" operation that undoes the changes.  Regarding the removal of the
>> test/hotspot/jtreg/gc/stress/jfr/TestStressBigAllocationGCEventsWithG1.java test: it only ever accidentally worked in
>> G1. G1 never sent the AllocationRequiringGC events for GCs caused by going over the IHOP threshold for humongous
>> allocations that the test actually expects.  That test previously only worked because G1 could not reclaim the
>> humongous objects fast enough so crossing the IHOP threshold causes a full concurrent mark. Allocations during that
>> concurrent mark do not cause a GC that can reclaim these objects, so ultimately some young GC that sends the
>> AllocationRequiringGC event will be sent.  With concurrent undo this is not guaranteed any more, i.e. only in
>> environments where concurrent undo is slow (and we'll improve it soon) this test works.  The test is too timing
>> dependent, and checking for the wrong thing in this case, so removing it.  Testing: tbd.
>
> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
> 
>   sjohanss review comments #3

Marked as reviewed by ayang (Author).

src/hotspot/share/gc/g1/g1ConcurrentMarkThread.cpp line 324:

> 322:   // We can (and should) abort if there has been a concurrent cycle abort for
> 323:   // some reason.
> 324:   if (_cm->has_aborted()) { return; }

I think more doc could be added here to explain in what situation this condition becomes true. I don't see sth similar
in `concurrent_mark_cycle_do`; why is it not needed there? I believe the answers could be placed in the comments to
help future readers better understand the code.

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

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