RFR [S][5/7]: 8197573: Remove concurrent cleanup and secondary free list handling
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.1_to_2/ (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()->has_aborted()) {
> ...
> 364 }
> 365
> 366 if (!cm()->has_aborted()) {
> - 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
> wait_while_free_regions_coming() and _cm->root_regions()->abort()?
> And this is why you didn't remove the comment above
> wait_while_free_regions_coming(), 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