RFR: 8260349: Cannot programmatically retrieve Metaspace max set via JAVA_TOOL_OPTIONS [v4]

[Thomas Stuefe]

On Thu, 28 Jan 2021 10:53:02 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> A simple but long standing bug whereby the MemoryPool MXBean only sees the value of MaxMetaspaceSize if it was set directly on the command-line and not by other means (e.g. env var).
>> 
>> Fix is to check !FLAG_IS_DEFAULT rather than FLAG_IS_CMDLINE.
>> 
>> Expanded regression test added (based on the reproducer in the JBS issue) that checks all the ways to set the flag: cmd-line, env-var, hotspotrc file
>> 
>> Testing: regression test (before and after fix)
>>              tiers 1-3 (in progress)
>> 
>> Thanks,
>> David
>
> David Holmes 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 seven additional commits since the last revision:
> 
>  - Further formatting updates from Aleksey
>  - Merge branch 'master' into 8260349
>  - Cleaned up test formatting based on Aleksey's review
>  - Added missing test case for flag not set
>  - Boost max from 1MB to 9MB as 1MB was too low for Aarch64 to startup
>  - Reduced test code using loop for env-vars
>  - 8260349: Cannot programmatically retrieve Metaspace max set via JAVA_TOOLS_OPTIONS

Hi David,

this looks fine to me. Some nits below, but feel free to ignore them. Thanks for fixing this.

..Thomas

src/hotspot/share/services/memoryPool.cpp line 201:

> 199: size_t MetaspacePool::calculate_max_size() const {
> 200:   return !FLAG_IS_DEFAULT(MaxMetaspaceSize) ? MaxMetaspaceSize :
> 201:                                               MemoryUsage::undefined_size();

Somewhat more robust may be to compare against max_uint. In case we ever change the default to not be infinite. But I am fine with this too.

test/hotspot/jtreg/runtime/Metaspace/MaxMetaspaceSizeEnvVarTest.java line 66:

> 64: 
> 65:     public static void main(String... args) throws Exception {
> 66:         final String max = String.valueOf(9 * 1024 * 1024); // 9 MB

You could set this to a much larger value for this test, since it won't change how the test works and would not cause more footprint. Does not matter here but may matter if we ever backport this, since before jep387 initial metaspace usage was a lot higher (400K vs 5-6m).

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

Marked as reviewed by stuefe (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2275