RFR: 8328508: Unify the signatures of the methods 'address_for_index' and 'index_for'

[Albert Mingkun Yang]

On Wed, 20 Mar 2024 17:26:24 GMT, Guoxiong Li <gli at openjdk.org> wrote:

> Hi all,
> 
> This patch unifies the related methods of the block offset table in Serial, Parallel and GC. It mainly removes the methods `SerialBlockOffsetSharedArray::address_for_index`, `SerialBlockOffsetSharedArray::index_for`, `G1BlockOffsetTable::address_for_index` and `G1BlockOffsetTable::index_for` and adds the corresponding methods `addr_for_entry` and `entry_for_addr`. Their usages and the signatures of other methods are also adjusted.
> 
> I find some duplicated code of the block offset table in Serial and Parallel (and less in G1). Maybe, we should refactor the block offset table to reduce the duplicated code in the future.
> 
> Tier-1 is running locally.
> 
> Thanks for taking the time to review.
> 
> Best Regards,
> -- Guoxiong

src/hotspot/share/gc/parallel/objectStartArray.inline.hpp line 48:

> 46: HeapWord* ObjectStartArray::block_start_reaching_into_card(HeapWord* const addr) const {
> 47:   const uint8_t* entry = entry_for_addr(addr);
> 48:   uint8_t offset = *entry;

I kind of like the original style, as it has a single de-reference. (It's indeed rather subjective.) Also, this change is completely unrelated to this ticket.

src/hotspot/share/gc/serial/serialBlockOffsetTable.hpp line 53:

> 51:   uint8_t* _offset_base;
> 52: 
> 53:   void fill_range(const uint8_t* const start, size_t num_cards, uint8_t offset) {

I think the first arg should be `uint8_t* const start`. Then, the `void*` cast below can be dropped.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18405#discussion_r1532573825
PR Review Comment: https://git.openjdk.org/jdk/pull/18405#discussion_r1532566215