RFR: 8322296: Introduce CardWord for word iteration on CardTable

[Kim Barrett]

On Mon, 18 Dec 2023 14:15:58 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

> Simple adding a new type for card-word in `CardTable` in order to avoid repeating the same logic/typedef in card based collectors.

Changes requested by kbarrett (Reviewer).

src/hotspot/share/gc/g1/g1RemSet.cpp line 559:

> 557: 
> 558:     CardValue* find_first_dirty_card(CardValue* i_card) const {
> 559:       while (!CardTable::is_word_aligned(i_card)) {

pre-existing: This loop assumes initial i_card < _end_card and _end_card is word aligned.  Should be
asserted. Similarly in find_first_non_dirty_card.

src/hotspot/share/gc/g1/g1RemSet.cpp line 571:

> 569: 
> 570:         if (has_dirty_cards_in_word) {
> 571:           for (uint i = 0; i < sizeof(CardWord); ++i) {

This loop assumes _end_card is word aligned.  Should be asserted, or different loop bound.
Similarly in find_first_non_dirty_card.

src/hotspot/share/gc/g1/g1RemSet.cpp line 615:

> 613:       _end_card(end_card) {
> 614:         assert(CardTable::is_word_aligned(start_card), "precondition");
> 615:         assert(CardTable::is_word_aligned(end_card), "precondition");

pre-existing: Strange indentation / formatting.

src/hotspot/share/gc/serial/cardTableRS.cpp line 365:

> 363:   // Word comparison
> 364:   while (current_card + sizeof(CardWord) <= end_card) {
> 365:     CardWord* current_word = reinterpret_cast<CardWord*>(current_card);

When the idea of something like CardWord was suggested, a named accessor to package this was part
of that suggestion.

src/hotspot/share/gc/shared/cardTable.hpp line 94:

> 92:   virtual ~CardTable() = default;
> 93: 
> 94:   static bool is_word_aligned(const CardValue* const card) {

Maybe is_word_aligned => is_card_word_aligned?  The existing name seems overly generic, and has other
(semantic, though probably not operational) meanings elsewhere.

src/hotspot/share/gc/shared/cardTable.hpp line 202:

> 200:   static constexpr CardValue clean_card_val()          { return clean_card; }
> 201:   static constexpr CardValue dirty_card_val()          { return dirty_card; }
> 202:   static constexpr CardWord clean_card_word_val()   { return clean_card_word; }

Body is weirdly misaligned with the others.  I'm personally not a fan of that sort of alignment, but a mix
of alignments is even worse.

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

PR Review: https://git.openjdk.org/jdk/pull/17144#pullrequestreview-1793665996
PR Review Comment: https://git.openjdk.org/jdk/pull/17144#discussion_r1434518318
PR Review Comment: https://git.openjdk.org/jdk/pull/17144#discussion_r1434519380
PR Review Comment: https://git.openjdk.org/jdk/pull/17144#discussion_r1434520828
PR Review Comment: https://git.openjdk.org/jdk/pull/17144#discussion_r1434521706
PR Review Comment: https://git.openjdk.org/jdk/pull/17144#discussion_r1434509129
PR Review Comment: https://git.openjdk.org/jdk/pull/17144#discussion_r1434510491