RFR: 8329771: G1: Refactor G1BlockOffsetTable::verify

[Thomas Schatzl]

On Wed, 10 Apr 2024 10:45:00 GMT, Guoxiong Li <gli at openjdk.org> wrote:

> Hi all,
> 
> This patch verifies the BOT after writing the BOT and changes the method name for consistence, just like `SerialBlockOffsetTable::verify_for_block` and `ObjectStartArray::verify_for_block`.
> 
> The test `make test-tier1_gc` passed locally. Thanks for taking the time to review.
> 
> Best Regards,
> -- Guoxiong

Changes requested by tschatzl (Reviewer).

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

> 214: #endif
> 215: 
> 216:   debug_only(G1BlockOffsetTable::verify_for_block(blk_start, blk_end);)

I would prefer to make the `verify_for_block` method have a `NOT_DEBUG_RETURN` and enclose the actual method with an `#ifdef ASSERT` - this avoids the `debug_only` macro use which is kind of distracting imo.

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

> 232:            (uint) offset_array(current_card),
> 233:            (uint) offset_array(current_card),
> 234:            (uint) (CardTable::card_size_in_words() + BOTConstants::N_powers - 1));

The first assert could have the same descriptive assert message than the other; define a static method that gets the upper bound passed.
Well, maybe not because of the `> 0` condition, but that could be separated as an extra assert maybe.

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

PR Review: https://git.openjdk.org/jdk/pull/18711#pullrequestreview-1996061604
PR Review Comment: https://git.openjdk.org/jdk/pull/18711#discussion_r1562134832
PR Review Comment: https://git.openjdk.org/jdk/pull/18711#discussion_r1562132814