RFR: 8277372: Add getters for BOT and card table members [v4]

Thomas Schatzl tschatzl at openjdk.java.net
Tue Nov 30 16:16:10 UTC 2021


On Tue, 30 Nov 2021 13:39:41 GMT, Vishal Chand <duke at openjdk.java.net> wrote:

>> Changed the visibility, added getters and refactored the following:
>> 
>> 1. Card Table Members
>> 2. BOT members
>> 3. ObjectStartArray block members
>
> Vishal Chand has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Rename BOTConstants

Getting good :) Some minor comments.

src/hotspot/share/gc/g1/g1BlockOffsetTable.cpp line 290:

> 288:     assert(_bot->offset_array(j) > 0 &&
> 289:            _bot->offset_array(j) <=
> 290:              (u_char) (BOTConstants::bot_card_size_words()+BOTConstants::N_powers-1),

Suggestion:

             (u_char) (BOTConstants::bot_card_size_words() + BOTConstants::N_powers - 1),

Pre-existing: operator has no spaces around it

src/hotspot/share/gc/g1/g1BlockOffsetTable.cpp line 295:

> 293:            (uint) _bot->offset_array(j),
> 294:            (uint) _bot->offset_array(j),
> 295:            (uint) (BOTConstants::bot_card_size_words()+BOTConstants::N_powers-1));

Suggestion:

           (uint) (BOTConstants::bot_card_size_words() + BOTConstants::N_powers - 1));

Pre-existing: spaces around operator

src/hotspot/share/gc/parallel/objectStartArray.hpp line 52:

> 50:   static uint _block_size;
> 51:   static uint _block_size_in_words;
> 52: 

Almost the same naming issue as in the `BlockOffsetTable/SharedArray`; I would prefer if these members (and getters) here were named similarly to the ones there.
It is true that `ObjectStartArray` and `BlockOffsetTable` are basically the same thing, but any eventual merge is another issue.

src/hotspot/share/gc/shared/cardTable.cpp line 416:

> 414:                dirty_cards++, next_entry++);
> 415:           MemRegion cur_cards(addr_for(cur_entry),
> 416:                               dirty_cards*_card_size_in_words);

Suggestion:

                              dirty_cards * _card_size_in_words);

Pre-existing: spaces around operator

src/hotspot/share/gc/shared/cardTable.cpp line 442:

> 440:                dirty_cards++, next_entry++);
> 441:           MemRegion cur_cards(addr_for(cur_entry),
> 442:                               dirty_cards*_card_size_in_words);

Suggestion:

                              dirty_cards * _card_size_in_words);

Pre-existing: spaces around operator

-------------

Changes requested by tschatzl (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6570


More information about the hotspot-dev mailing list