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

[Ioi Lam]

On Mon, 9 Sep 2024 12:04:53 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Attempt to drop the min region alignment with [JDK-8337828](https://bugs.openjdk.org/browse/JDK-8337828) highlights an interesting trouble. The roots array we are creating during the dump time can easily be larger than the min region alignment. We are currently "lucky" none of our tests hit this limit. AFAICS, about 128K classes would be enough to hit the current 1M min region alignment. Dropping the min region alignment to 256K starts to fail the test with "only" 30K classes, [JDK-8338856](https://bugs.openjdk.org/browse/JDK-8338856).
>> 
>> We can slice that heap root array, and thus untie the roots count from the min region alignment. I am submitting something that works, but this might not be the final form for it. I would like @iklam to poke holes in this approach :)
>> 
>> Additional testing:
>>  - [x] macos-aarch64-server-fastdebug, `runtime/cds`
>>  - [ ] linux-aarch64-server-fastdebug, `all`
>>  - [ ] linux-x86_64-server-fastdebug, `all`
>
> Aleksey Shipilev has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Replace divisions with power-of-two math
>  - Renames, adjustments after review

Looks good to me. Just two small nits.

src/hotspot/share/cds/archiveHeapWriter.cpp line 185:

> 183: }
> 184: 
> 185: objArrayOop ArchiveHeapWriter::manifest_root_segment(size_t offset, int element_count) {

The meaning of `manifest` is not very clear to me, and I can't find `manifest` used in this sense in any HotSpot header files. Maybe use `allocate_root_segment` instead?

src/hotspot/share/cds/archiveHeapWriter.cpp line 242:

> 240:     objArrayOop seg_oop = manifest_root_segment(oop_offset, size_elems);
> 241:     for (int i = 0; i < size_elems; i++) {
> 242:       root_segment_at_put(seg_oop, i, roots->at(seg_start + i));

`HeapRootSegments::roots_offset()` is used only here, so I think it's better to avoid having an extra function that you have to understand by reading both of it definition and it usage here. Maybe this can be simplified with:


int root_index = 0;
for (size_t seg_idx = 0; seg_idx < segments.count(); seg_idx++) {
   ...
    for (int i = 0; i < size_elems; i++) {
      root_segment_at_put(seg_oop, i, roots->at(root_index++));
 ```

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

Marked as reviewed by iklam (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20858#pullrequestreview-2290780460
PR Review Comment: https://git.openjdk.org/jdk/pull/20858#discussion_r1750828084
PR Review Comment: https://git.openjdk.org/jdk/pull/20858#discussion_r1750818706