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