RFR: 8072817: CardTableExtension kind() should be BarrierSet::CardTableExtension

[Jon Masamitsu]

Kim,

Looks good assuming, as Jesper suggests, you meant
an ||.

Jon


On 08/27/2015 11:48 AM, Jesper Wilhelmsson wrote:
> Hi Kim,
>
> Looks good in general.
>
> In src/cpu/aarch64/vm/macroAssembler_aarch64.cpp there is a comma 
> between the two checks:
>
> +  assert(bs->kind() == BarrierSet::CardTableForRS,
> +         bs->kind() == BarrierSet::CardTableExtension,
> +         "Wrong barrier set kind");
>
> I assume there should be a ||
>
> Besides that the patch seems to do what you described below.
>
> Thanks,
> /Jesper
>
>
> Den 27/8/15 kl. 19:51, skrev Kim Barrett:
>> Still looking for reviews.
>>
>> On Aug 19, 2015, at 1:42 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>>
>>> Please review this change to finally properly integrate the
>>> BarrierSet::CardTableExtension barrier set kind. This has been
>>> deferred for a long time; there was, until recently, a comment in
>>> CardTableExtension that having kind() return
>>> BarrierSet::CardTableExtension was "too risky for the 4/10/02
>>> putback".
>>>
>>> Much of the work involved in supporting the CardTableExtension tag had
>>> been done long ago.  The remaining work involved actually hooking it
>>> up.  The key missing step was to give CardTableModRefBSForCTRS its own
>>> tag (to distinguish from CardTableExtension) and switch to using that
>>> tag where needed.  The needed tag (BarrierSet::CardTableForRS) was
>>> defined in the fix for JDK-8069016.  This change makes use of it.
>>>
>>> This involved the following steps:
>>>
>>> 1. Most occurrences of BarrierSet::CardTableModRef are now
>>> BarrierSet::CardTableForRS. Occurrences of BarrierSet::CardTableForRS
>>> and BarrierSet::CardTableExtension are now nearly always paired.
>>> Exceptions are in the constructors of the corresponding classes and in
>>> the associated barrier_set_cast support.
>>>
>>> 2. The tests for kind() == BarrierSet::CardTableModRef that were
>>> controlling the devirtualization of write_ref_field_pre,
>>> write_ref_field, and write_region now test for either
>>> BarrierSet::CardTableForRS or BarrierSet::CardTableExtension. The
>>> FakeRttiSupport class was extended to improve the new test.
>>>
>>> 3. The remaining small number of tests for kind() ==
>>> BarrierSet::CardTableModRef are now tests for
>>> is_a(BarrierSet::CardTableModRef). These are places where we really do
>>> allow a CardTableModRefBS and don't care which subclass; all of these
>>> involve access to the card table. This now includes G1 barrier sets in
>>> a couple of places where they were previously inappropriately or
>>> unnecessarily excluded.
>>>
>>> 4. The CardTableModRefBSForCTRS class now specifies its kind tag be
>>> BarrierSet::CardTableForRS. The CardTableExtension class now specifies
>>> its kind tag be BarrierSet::CardTableExtension.  The extra
>>> FakeRttiSupport constructor that existed to support the old kludge for
>>> those classes has been removed.
>>>
>>> Also removed unused CollectorPolicy::barrier_set_name.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8072817
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~kbarrett/8072817/webrev.00/
>>>
>>> Testing:
>>> jprt
>>> aurora ad hoc GC/Runtime Nightly, quicktests, tonga, jtreg
>>
>>