RFR: 8318647: Serial: Refactor BlockOffsetTable

Albert Mingkun Yang ayang at openjdk.org
Wed Oct 25 11:20:38 UTC 2023


On Wed, 25 Oct 2023 10:43:56 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> The diff is too large; maybe it's better to read the new impl directly, which I believe is much easier to follow.
>> 
>> There is some duplication with G1's implementation, which should probably be dealt with in its own PR.
>
> src/hotspot/share/gc/serial/serialBlockOffsetTable.hpp line 43:
> 
>> 41: // systems using card-table-based write barriers, the efficiency of this
>> 42: // operation may be important.  Implementations of the BlockOffsetTable
>> 43: // class may be useful in providing such efficient implementations.
> 
> The comment is completely outdated, talking about subtypes where there are none any more of this class.

I think the subtypes refer to heaps from different collectors. Not sth around `BlockOffsetArray`.

> src/hotspot/share/gc/serial/serialBlockOffsetTable.hpp line 45:
> 
>> 43: // class may be useful in providing such efficient implementations.
>> 44: 
>> 45: class BlockOffsetSharedArray: public CHeapObj<mtGC> {
> 
> Should be prefixed by `Serial`, same as `BlockOffsetTable` (since this is a complete reimplementation anyway).

Maybe this renaming can be done in its own (trivial) PR for easier reviewing.

> src/hotspot/share/gc/serial/serialBlockOffsetTable.hpp line 55:
> 
>> 53:   // address.
>> 54:   VirtualSpace _vs;
>> 55:   u_char* _offset_array;          // byte array keeping backwards offsets
> 
> I always wondered whether `u_char` is what we want here, maybe `uint8_t` is more appropriate?

Can probably be done in its own PR.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16304#discussion_r1371567566
PR Review Comment: https://git.openjdk.org/jdk/pull/16304#discussion_r1371566528
PR Review Comment: https://git.openjdk.org/jdk/pull/16304#discussion_r1371565659


More information about the hotspot-gc-dev mailing list