RFR (S/M): 8202017: Merge Reference Enqueuing phase with phase 3 of Reference processing
Kim Barrett
kim.barrett at oracle.com
Thu Apr 26 23:03:47 UTC 2018
> On Apr 24, 2018, at 5:26 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>
> Hi Kim,
>
> On Mon, 2018-04-23 at 19:10 -0400, Kim Barrett wrote:
>>> On Apr 19, 2018, at 3:14 PM, Thomas Schatzl <thomas.schatzl at oracle.
>>> com> wrote:
>>>
>>> Hi all,
>>>
>>> can I have reviews for this change that merges the Reference
>>> Enqueuing phase of reference processing with the respective phase 3
>>> of actual Reference Processing?
>>>
> [...]
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8202017
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8202017/webrev/
>>> Testing:
>>> hs-tier1-4
>>>
>>> Thanks,
>>> Thomas
>>
>> Looks good. Just a couple very minor things.
>>
>> -------------------------------------------------------------------
>> -----------
>> src/hotspot/share/gc/shared/referenceProcessor.cpp
>> 607 _refs_lists[i].set_head(NULL);
>> 608 _refs_lists[i].set_length(0);
>>
>> 790 refs_lists[i].set_head(NULL);
>> 791 refs_lists[i].set_length(0);
>>
>> Maybe move these into process_phase3?
>
> Done.
>
>>
>> -------------------------------------------------------------------
>> -----------
>> src/hotspot/share/gc/g1/g1CollectedHeap.hpp
>> 515 // If during an initial mark pause we install a pending list
>> head which is not otherwise reachable
>>
>> Perhaps s/install/may install/
>>
>
> Done.
>
>> src/hotspot/share/gc/shared/genCollectedHeap.cpp
>> 518 rp->disable_discovery();
>>
>> Ick! This whole little dance around discovery_is_atomic,
>> enqueueing_is_done, &etc looks like it is related to CMS and the
>> distinction between CMS and Serial. See also the TODO-ish block
>> comment at line 489.
>>
>> This looks ripe for refactoring as part of the cleanup of CMS-
>> specific intrusions. Probably should have a followup RFE for this.
>
> I will file an RFE for this.
>
> New webrevs:
> http://cr.openjdk.java.net/~tschatzl/8202017/webrev.0_to_1 (diff)
> http://cr.openjdk.java.net/~tschatzl/8202017/webrev.1/ (full)
>
> Thomas
In the comment for G1CollectedHeap::make_pending_list_reachable():
516 // otherwise reachable ensure that it is marked in the bitmap for concurrent marking
s/reachable ensure/reachable. Ensure/
Otherwise looks good. No need for a new webrev for that comment change.
More information about the hotspot-gc-dev
mailing list