RFR: JDK-8294677: chunklevel::MAX_CHUNK_WORD_SIZE too small for some applications [v2]

[Paul Hohensee]

On Sat, 21 Jan 2023 09:04:05 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> This patch is a workaround for a regression caused by the JEP-387 metaspace revamp. That revamp introduced a maximum limit to the size an individual metaspace allocation could have (4m). 
>> 
>> 4m was deemed much more than enough; normally metaspace allocations are tiny. But we now had a customer trying to load a generated giant class. The class was inefficiently generated but valid nonetheless.
>> 
>> The patch increases the maximum chunk size in Metaspace from 4M to 16M. This fixes the customer case mentioned in the JBS note. However, this is just a workaround since we still have a limit, albeit a larger one. Therefore I am working on a prototype that removes the limit completely (https://bugs.openjdk.org/browse/JDK-8300729) but that needs some more cooking time.
>> 
>> For details, please see JBS text as well as JDK-8300729.
>> 
>> ---
>> 
>> Patch increases the maximum chunk size.
>> 
>> This affects the granularity used to *reserve* (not: commit - no RSS increase, just address space) metaspace memory, which in turn affects CompressedClassSpaceSize granularity (it gets silently rounded up to a multiple of root chunk size) as well as class space placement. Thankfully all of this is well abstracted behind `Metaspace::reserve_alignment()` so almost no changes were necessary. The increase from 4m to 16m is also small enough to be benign.
>> 
>> Increasing the maximum chunk size also increases the memory and runtime footprint of some tests that explicitly test metaspace chunk management. I checked all of these tests and changed some to keep the footprint low.
>> 
>> Finally, the patch uncovered some minor test errors in metaspace tests:
>> 
>> The gtest "metaspace:get_chunk_with_commit_limit" was supposed to test allocation from a metaspace chunk when hit by a commit limit. The limit was too high, effectively disabling the commit-failed path of this test. I expanded the test to test a variation of commit limits, not only one. The test also produced wrong positives with `MetaspaceReclaimPolicy=none` - those are fixed now (see comment). Note that I plan to remove the `MetaspaceReclaimPolicy` switch since, in practice, it seems of very limited use, and this would cut down on test variations and code complexity.
>> 
>> The gtest "metaspace:misc_sizes" tests that we can allocate the max. root chunk size (abstracted via `Metaspace::max_allocation_word_size`). A larger root chunk size caused us to hit the GC threshold now. I adjusted the test and expanded it to test both class space and non-class metaspace.
>> 
>> The elastic metaspace jtreg tests (test/hotspot/jtreg/runtime/Metaspace/elastic...) needed adaption since they have the root chunk size hard-coded. There is also a minor issue with using word size, but I left that untouched and opened https://bugs.openjdk.org/browse/JDK-8300732 to deal with this later.
>> 
>> ---
>> 
>> Tests:
>> 
>> I manually ran hotspot_metaspace tests (a collection of metaspace, gtests, cds tests and some GC tests) on x64 and x86, fastdebug and release. GHAs are in process, and more tests are planned.
>
> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix runtime/CompressedOops/CompressedClassSpaceSize.java

Lgtm.

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

Marked as reviewed by phh (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12115