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

Stefan Johansson stefan.johansson at oracle.com
Thu Mar 8 12:28:56 UTC 2018

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 "mixedgcpending" 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.0to1 (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

More information about the hotspot-gc-dev mailing list