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

[Thomas Schatzl]

On Thu, 24 Sep 2020 17:37:12 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   sjohanss review comments #3
>
> 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.

Currently the only reason for an abort is a full gc, calling `G1CollectedHeap::abort_concurrent_cycle`. I would like to
avoid putting all callers of that method in this comment because they might change.

`concurrent_mark_cycle_do()` has one of these `if (_cm->has_aborted())` checks at the end of every phase implicitly,
i.e. the many

`if (some_phase) return;`

encapsulate it. G1 can't abort before the root scan phase in `concurrent_mark_cycle_do`, because of this reason in the
comment in `G1CollectedHeap::abort_concurrent_cycle`:

  // If we start the [full gc] compaction before the CM threads finish
  // scanning the root regions we might trip them over as we'll
  // be moving objects / updating references. So let's wait until
  // they are done. By telling them [root region scan] to abort, they should complete
  // early.

and we can't prevent the pause ("join an STS") in root region scanning or before because of the existing comment in
`G1CollectedHeap::abort_concurrent_cycle`.

I can add a reference to the `G1CollectedHeap::abort_concurrent_cycle` comment to the one in
`concurrent_mark_cycle_do` - would that satisfy your concern?

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

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