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