RFR: 8328573: Add ASSERT macro and not use 'CardTable::card_shift_in_words' in 'G1BlockOffsetTable::check_index'

[Kim Barrett]

On Wed, 20 Mar 2024 08:39:07 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hi all,
>> 
>> This patch adds the ASSERT macro to the declaration and usages of the method `G1BlockOffsetTable::check_index`. And the `CardTable::card_shift_in_words` is changed to `CardTable::card_shift` in order to prepare [JDK-8328508](https://bugs.openjdk.org/browse/JDK-8328508).
>> 
>> Thanks for taking the time to review.
>> 
>> Best Regards,
>> -- Guoxiong
>
> src/hotspot/share/gc/g1/g1BlockOffsetTable.hpp line 75:
> 
>> 73:   inline void set_offset_array(size_t left, size_t right, u_char offset);
>> 74: 
>> 75: #ifdef ASSERT
> 
> I disagree with this change - it is common usage to use the `NOT_DEBUG_RETURN` or similar macros to hide the `#ifdef`s.
> The `#ifdef`s only clutter the code for no gain as you can see.

I agree with @tschatzl - don't make this and related parts of the change.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18391#discussion_r1532540270