RFR: 8341334: CDS: Parallel relocation [v7]

Aleksey Shipilev shade at openjdk.org
Wed Nov 6 11:47:35 UTC 2024


On Wed, 6 Nov 2024 10:00:07 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 13 additional commits since the last revision:
>> 
>>  - More perf touchups
>>  - Cap the max number of workers
>>  - Revert pre-touch parts: there are startup regressions on smaller thread counts
>>  - Handle single-threaded modes better
>>  - Merge branch 'master' into JDK-8341334-cds-parallel-relocation
>>  - Merge branch 'master' into JDK-8341334-cds-parallel-relocation
>>  - Make sure we gracefully shutdown whatever happens, refix shutdown race
>>  - Simpler bitmap distribution
>>  - Capitalize constants
>>  - Do not create worker threads too early: Mac/Windows are not yet ready to use Semaphores
>>  - ... and 3 more: https://git.openjdk.org/jdk/compare/7a297fc1...d4d739b1
>
> src/hotspot/share/cds/filemap.cpp line 1972:
> 
>> 1970:     BitMap::idx_t size  = bm->size();
>> 1971:     BitMap::idx_t start = MIN2(size, size * chunk / max_chunks);
>> 1972:     BitMap::idx_t end   = MIN2(size, size * (chunk + 1) / max_chunks);
> 
> Can you assert here that `(end - start) > 1`? Should never happen with the limited number of workers, but it would be nice to know. E.g. if someone changes the number of workers again.
> 
> If it were to happen, the effect would be that we silently skip relocation. Better to fall over this early.

Shouldn't it be `assert(end > start)` instead? I.e. check we never actually get empty slices.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21302#discussion_r1830880103


More information about the hotspot-runtime-dev mailing list