Integrated: JDK-8294677: chunklevel::MAX_CHUNK_WORD_SIZE too small for some applications

[Thomas Stuefe]

On Fri, 20 Jan 2023 15:15:01 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.

This pull request has now been integrated.

Changeset: 2292ce13
Author:    Thomas Stuefe <stuefe at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/2292ce137c16accf0622600d5a096403b8a8058d
Stats:     106 lines in 10 files changed: 37 ins; 9 del; 60 mod

8294677: chunklevel::MAX_CHUNK_WORD_SIZE too small for some applications

Reviewed-by: simonis, phh

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

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