RFR: 8338912: CDS: Segmented roots array [v4]

[Aleksey Shipilev]

On Fri, 6 Sep 2024 17:47:12 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Amend printout as well
>
> src/hotspot/share/cds/archiveHeapLoader.cpp line 381:
> 
>> 379:     int max_size = roots.segment_max_size_bytes();
>> 380:     HeapShared::init_root_segment_max_size(max_size);
>> 381:     intptr_t base_roots_oop = bottom + roots.base_offset();
> 
> Since people are more likely to read this function (using the roots) than archiveHeapWriter.cpp (writing the roots), I think it's better to add a comment here. `base_roots_oop` should also be renamed for clarity.
> 
> 
> // The heap roots are stored in one or more segments that are laid out consecutively.
> // The byte size of each segment (except for the last one) is max_size.
> intptr_t first_segment_addr = bottom + roots.base_offset();

Doing this in new commit.

> src/hotspot/share/cds/archiveHeapWriter.cpp line 223:
> 
>> 221: 
>> 222:   HeapRoots heap_roots(_buffer_used,
>> 223:                        roots->length(),
> 
> We use `GrowableArrayCHeap<oop, mtClassShared>* roots` in a few places in this file, and having another variable `heap_roots` makes it difficult to distinguish between the two.
> 
> Maybe use this instead? I think local variables can be called `segments` instead of `heap_root_segments` when it's obvious.
> 
> 
> HeapRootSegments segments(...)

Right. I will rename this in new commit.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20858#discussion_r1750066597
PR Review Comment: https://git.openjdk.org/jdk/pull/20858#discussion_r1750066799