RFR: 8202082: Remove explicit CMS checks in CardTableBarrierSetAssembler
Erik Osterlund
erik.osterlund at oracle.com
Thu Apr 26 20:46:02 UTC 2018
Hi Kim,
Thank you for the review.
/Erik
On 26 Apr 2018, at 22:34, Kim Barrett <kim.barrett at oracle.com> wrote:
>> On Apr 26, 2018, at 2:52 AM, Erik Österlund <erik.osterlund at oracle.com> wrote:
>>
>> Hi Kim,
>>
>> On 2018-04-26 06:36, Kim Barrett wrote:
>>>> On Apr 25, 2018, at 9:53 AM, Erik Österlund <erik.osterlund at oracle.com> wrote:
>>>>
>>>> Hi Kim,
>>>>
>>>> Okay, it looks like we kind of agree that this is a step in the right direction for now. Thank you for the review.
>>>>
>>>> Here is my final webrev with "disp" not refetched (and renamed to card_table_addr):
>>>> http://cr.openjdk.java.net/~eosterlund/8202082/webrev.01/
>>>>
>>>> Incremental:
>>>> http://cr.openjdk.java.net/~eosterlund/8202082/webrev.00_01/
>>> Only partially addressed my comment:
>>> src/hotspot/cpu/x86/gc/shared/cardTableBarrierSetAssembler_x86.cpp
>>> 105 intptr_t card_table_addr = (intptr_t)ctbs->card_table()->byte_map_base();
>>>
>>> s/ctbs->card_table()/ct/
>>>
>>> But wait, that version shouldn’t even compile?
>>> 107 card_addr = Address(noreg, obj, Address::times_1, disp);
>>> still refers to “disp”.
>>
>> Fixed.
>>
>>> card_table_add is better than disp.
>>
>> Actually now that I think of it, I decided the name should be byte_map_base instead. That is indeed what the comments already refer to and what other platforms consistently call this variable. And it is not a real address.
>
> Yes, that is better.
>
> Looks good.
>
>>
>> Incremental:
>> http://cr.openjdk.java.net/~eosterlund/8202082/webrev.01_02/
>>
>> Full:
>> http://cr.openjdk.java.net/~eosterlund/8202082/webrev.02/
>>
>> Thanks,
>> /Erik
>
>
More information about the hotspot-dev
mailing list