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

[sangheon.kim]

Hi Thomas,

On 03/27/2018 01:58 AM, 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.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.
Right.

>
>> ===============================
>> 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.
Okay, thanks for filing the CR.

>
>> ===============================
>> 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)
webrev.3 looks good to me.

Thanks,
Sangheon


>
> Thanks,
>    Thomas
>