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

Stefan Johansson stefan.johansson at oracle.com
Tue Mar 27 09:20:19 UTC 2018

On 2018-03-27 10:58, Thomas Schatzl wrote:

Hi Sangheon,

thanks for your review. On Mon, 2018-03-26 at 15:38 -0700, sangheon.kim wrote: Hi Thomas, [..]

New webrevs: http://cr.openjdk.java.net/~tschatzl/8197573/webrev.1to2/ (diff) http://cr.openjdk.java.net/~tschatzl/8197573/webrev.2/ (full) webrev.2 looks good. I only have minor nits.

=============================== src/hotspot/share/gc/g1/concurrentMarkThread.cpp 362 if (!cm()->hasaborted()) { ... 364 } 365 366 if (!cm()->hasaborted()) { - Any reason for having 2 if clause at line:362 and 366? The delay might take a long while, so the abort state might have changed already. So it does not hurt checking to avoid going into the VM operation. =============================== src/hotspot/share/gc/g1/g1CollectedHeap.cpp (from original file) 1030 // following two methods. - Do you know which method is referring originally? I guess waitwhilefreeregionscoming() and cm->rootregions()->abort()? And this is why you didn't remove the comment above waitwhilefreeregionscoming(), right? As far as I understand the two methods were waitwhilefreeregionscoming() and waituntilscanfinished(). The latter is still there so I only adapted the comment. I removed the comment in the new webrev and filed JDK-8200291. We should not have todo-lists in the code. =============================== src/hotspot/share/gc/g1/heapRegionSet.cpp - Copyright year update Fixed. New webrevs (just the comment removals and the copyright update): http://cr.openjdk.java.net/~tschatzl/8197573/webrev.2to3/ (diff) http://cr.openjdk.java.net/~tschatzl/8197573/webrev.3/ (full) Still good! StefanJ Thanks, Thomas

More information about the hotspot-gc-dev mailing list