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

Thomas Schatzl thomas.schatzl at oracle.com
Tue Mar 27 08:58:19 UTC 2018

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 wait_while_free_regions_coming() and wait_until_scan_finished(). 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.2_to_3/ (diff) http://cr.openjdk.java.net/~tschatzl/8197573/webrev.3/ (full)

Thanks, Thomas

More information about the hotspot-gc-dev mailing list