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

[Aleksey Shipilev]

On Thu, 28 Jan 2021 04:41:58 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 incrementally with one additional commit since the last revision:
> 
>   Boost max from 1MB to 9MB as 1MB was too low for Aarch64 to startup

The fix looks right. I suggest polishing the test a little.

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

> 34:  * @library /test/lib
> 35:  * @run driver MaxMetaspaceSizeEnvVarTest
> 36:  */

I believe the conventional style is to do jtreg test definition the first thing, and then do imports.

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

> 86:                              "_JAVA_OPTIONS",
> 87:                              "JAVA_TOOL_OPTIONS"
> 88:         };

This is an odd style. Suggestion:

String[] envVars = { 
    "JDK_JAVA_OPTIONS",
    "_JAVA_OPTIONS",
    "JAVA_TOOL_OPTIONS"
};

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

> 74:         int test = 1;
> 75:         String msg = "Test " + test +": normal command-line flag";
> 76:         report(msg);

What is the point of declaring local variables `msg` here? AFAICS, it might as well be:

report("Test " + test +": normal command-line flag");

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

> 73: 
> 74:         int test = 1;
> 75:         String msg = "Test " + test +": normal command-line flag";

I think you can dispense with `test` variable, and just print "Test: normal command-line flag", and later the name of env opts.

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

Marked as reviewed by shade (Reviewer).

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