RFR: 8338912: CDS: Segmented roots array [v8]
Calvin Cheung
ccheung at openjdk.org
Thu Sep 12 23:01:09 UTC 2024
On Tue, 10 Sep 2024 09:59:37 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`
>> - [x] linux-aarch64-server-fastdebug, `all`
>> - [x] linux-x86_64-server-fastdebug, `all`
>
> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
>
> Make sure HeapRootSegments representation does not hold garbage
Looks good. I have couple of minor suggestions.
I'm also running tiers 1 - 4 testing with your patch. Results look good so far.
src/hotspot/share/cds/archiveUtils.cpp line 387:
> 385: }
> 386:
> 387: size_t HeapRootSegments::segment_offset(size_t seg_idx) {
Suggestion: add assert on `seg_idx` (as in line 378).
src/hotspot/share/cds/filemap.cpp line 293:
> 291: st->print_cr("- mapped_base_address: " INTPTR_FORMAT, p2i(_mapped_base_address));
> 292: st->print_cr("- heap_root_segments.roots_count: %d" , _heap_root_segments.roots_count());
> 293: st->print_cr("- heap_root_segments.base_offset: " SIZE_FORMAT, _heap_root_segments.base_offset());
Existing offset value are being printed with the format SIZE_FORMAT_X. Maybe use the same format here.
-------------
Marked as reviewed by ccheung (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/20858#pullrequestreview-2301553299
PR Review Comment: https://git.openjdk.org/jdk/pull/20858#discussion_r1757685887
PR Review Comment: https://git.openjdk.org/jdk/pull/20858#discussion_r1757686975
More information about the hotspot-runtime-dev
mailing list