RFR [S][5/7]: 8197573: Remove concurrent cleanup and secondary free list handling

[Stefan Johansson]

On 2018-03-08 12:59, Thomas Schatzl wrote:
> Hi,
>
> On Mon, 2018-03-05 at 16:16 +0100, Thomas Schatzl wrote:
>> Hi all,
>>
>>    can I have reviews for this change that removes the concurrent
>> cleanup phase and the secondary free list handling entirely. It moves
>> leftover work into the Cleanup pause.
>>
>> In the previous change the remembered set scrubbing, which took in
>> cases where it mattered like 95%+ of time, has been removed because
>> it
>> is not required any more.
>>
>> The reasons for removal are:
>> - due to JDK-8180415 *freeing* remembered sets (as opposed to
>> calculating occupancy) is really fast, so this phase has never been
>> very long for a long time.
>> - it adds a lock in the LAB allocation path
>>
>> It also allows us to remove the "mixed_gc_pending" flag in the
>> CollectorState, as there is no further time to wait until G1 can
>> schedule the "last young gc".
>>
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8197573
>> Webrev:
>> http://cr.openjdk.java.net/~tschatzl/8197573/webrev/index.html
>> Testing:
>> hs-tier 1-3, ....
>>
>> (I consider this an "S" changeset because it is mostly removal of
>> code).
>>
> Some ripple-through after latest changes to JDK-8180415 requires an
> update of the webrevs:
> http://cr.openjdk.java.net/~tschatzl/8197573/webrev.0_to_1 (diff)
> http://cr.openjdk.java.net/~tschatzl/8197573/webrev.1 (full)
Looks lovely, just some minor things:
src/hotspot/share/gc/g1/concurrentMarkThread.cpp
436           log_info(gc, marking)("Concurrent Mark Abort");

This line was removed, and it looks like will never log a concurrent 
mark abort now. Looks like this could be added to 
G1ConcurrentMark::concurrent_cycle_end() where we trace a potential cm 
failure.
---
src/hotspot/share/gc/g1/g1FullGCPrepareTask.cpp
106   _g1h->free_humongous_region(hr, &dummy_free_list);

Should be safe to call free_humongous_region with locked = true here, 
since this will be the only thread working on that region.

115   _g1h->card_table()->clear(MemRegion(hr->bottom(), hr->end()));

Pre-existing, but since you have added the clear_cardtable() function we 
could use it here as well, seems to be the only place where the old 
"scheme" is used.
---

> I also found some more opportunity to do minor cleanup after Stefan's
> last remark about log messages...
Nice, and you also restructured the code exactly like I planned to 
comment :)

Thanks,
Stefan
> Thanks,
>    Thomas
>