RFR: 8345659: Fix broken alignment after ReservedSpace splitting in GC code [v2]

[Axel Boldt-Christmas]

On Wed, 11 Dec 2024 11:41:13 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> The Serial and Parallel GCs create a ReservedSpace for the total heap and then splits it into a young generation ReservedSpace and an old generation ReservedSpace. The latter operation creates an ReservedSpace with an alignment that doesn't match the base address. This bug is benign because the ReservedSpaces are short-lived and we don't look at the alignment. However, if we are to add stricter checks when creating ReservedSpaces we need to fix this. 
>> 
>> Tested with tier1-3
>
> Stefan Karlsson has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Initialize with GenAlignment for both generations

lgtm.

The alignment always require extra thinking as it has multiple meanings. It is both the required alignment of the base, but it is also the granularity / alignment of the size. But we have the same meaning when it comes to HeapAlignment, SpaceAlignment and GenAlignment. So the partition size does not break this invariant.

It is unfortunate that we do not assert that these invariants are maintained when we partition a reserved space. Something like:

ReservedSpace::ReservedSpace(char* base, size_t size, size_t alignment, size_t page_size,
                             bool special, bool executable) : _fd_for_heap(-1) {
  assert((size % os::vm_allocation_granularity()) == 0,
         "size not allocation aligned");
+ assert(alignment != 0, "must be set");
+ assert(size % alignment == 0, "must be");
+ assert((uintptr_t)base % alignment == 0, "must be");
  initialize_members(base, size, alignment, page_size, special, executable);
}


But I know you are working on refactoring and improving the ReservedSpace. Let us hope we can make this more robust in the future.

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

Marked as reviewed by aboldtch (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22602#pullrequestreview-2495453184