RFR: 8257230: assert(InitialHeapSize >= MinHeapSize) failed: Ergonomics decided on incompatible initial and minimum heap sizes [v3]

[Stefan Johansson]

On Thu, 3 Dec 2020 11:15:15 GMT, Jie Fu <jiefu at openjdk.org> wrote:

>> Hi all,
>> 
>> Ergonomics for InitialHeapSize can be broken if the memory resource is limited by the administrator.
>> For example, this assert [1] fired on our testing boxes.
>> 
>> It can be reproduced by the following two steps on Linux-64:
>>   1) ulimit -v 8388608
>>   2) java -XX:MinHeapSize=5g -version
>> The reason was that limit_by_allocatable_memory() [2] returns a value less than MinHeapSize.
>> 
>> One more important fact is that this bug can be more common on Linux-32 systems.
>> Since the virtual memory is limited to 3800M [3] on Linux-32, it can be always reproduced when MinHeapSize > 1900M.
>> 
>> Testing:
>>   - tier1 ~ tier3 on Linux/x64
>> 
>> Thanks.
>> Best regards,
>> Jie
>> 
>> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/gc/shared/gcArguments.cpp#L96
>> [2] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/runtime/arguments.cpp#L1907
>> [3] https://github.com/openjdk/jdk/blob/master/src/hotspot/os/posix/os_posix.cpp#L567
>
> Jie Fu 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 five additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8257230
>  - Refinement & jtreg test
>  - Merge branch 'master' into JDK-8257230
>  - Merge branch 'master' into JDK-8257230
>  - 8257230: assert(InitialHeapSize >= MinHeapSize) failed: Ergonomics decided on incompatible initial and minimum heap sizes

Took a closer look at the test now, some comment below.

test/hotspot/jtreg/gc/ergonomics/TestMinHeapSize.java line 33:

> 31:  *
> 32:  * @comment Not run on AIX as it does not support ulimit -v
> 33:  * @requires os.family != "aix"

I would change this to only be run on Linux:
 * @requires os.family == "linux"

test/hotspot/jtreg/gc/ergonomics/TestMinHeapSize.java line 46:

> 44:         String cmd = ProcessTools.getCommandLine(ProcessTools.createTestJvm(
> 45:                 "-XX:MinHeapSize=" + "260m", "-version"));
> 46:         cmd = escapeCmd(cmd);

If we change to only run on Linux, this is not needed.

test/hotspot/jtreg/gc/ergonomics/TestMinHeapSize.java line 45:

> 43:     public static void main(String[] args) throws Throwable {
> 44:         String cmd = ProcessTools.getCommandLine(ProcessTools.createTestJvm(
> 45:                 "-XX:MinHeapSize=" + "260m", "-version"));

`createTestJvm()` will pick up additional options passed by the test runner and there might be conflicting options. So I would suggest not picking up any additional options.

test/hotspot/jtreg/gc/ergonomics/TestMinHeapSize.java line 59:

> 57: 
> 58:         oa.shouldNotContain("hs_err")
> 59:           .shouldNotContain("Internal Error");

Should it also check that the exit status is ok, or can't we expect that for sure?

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

Changes requested by sjohanss (Reviewer).

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