RFR: 8335167: Test runtime/Thread/TestAlwaysPreTouchStacks.java failed with Expected a higher ratio between stack committed and reserved [v5]

[Afshin Zafari]

On Thu, 19 Sep 2024 16:06:45 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   two new expectations added
>
> test/hotspot/jtreg/runtime/Thread/TestAlwaysPreTouchStacks.java line 72:
> 
>> 70:                 },
>> 71:                 "TestThread-" + num, threadStackSizeMB * MB);
>> 72:         t.setDaemon(true);
> 
> Since you are here, could you add a comment here?
> 
> Suggestion:
> 
>         // Let test threads run as daemons to ensure that they are still running and
>         // that their stacks are still allocated when the JVM shuts down and the final
>         // NMT report is printed.
>         t.setDaemon(true);

Done.

> test/hotspot/jtreg/runtime/Thread/TestAlwaysPreTouchStacks.java line 181:
> 
>> 179:           }
>> 180:           double estimated_stack_usage = 1 * MB;
>> 181:           double expected_stack_use_with_pretouch = 0.75 * threadStackSizeMB * MB;
> 
> Can you hard-code both values up where the other constants are?
> 
> I also would name them clearly for what they are. "estimated_stack_usage" sounds wrong, since we don't really estimate that, the real number should be far lower (for no pretouch) and far higher (for pretouch).
> 
> Proposal: `max_stack_usage_with_pretouch` and `min_stack_usage_with_pretouch`

Done.

> test/hotspot/jtreg/runtime/Thread/TestAlwaysPreTouchStacks.java line 187:
> 
>> 185:           double expected_delta = numThreads * (expected_stack_use_with_pretouch - estimated_stack_usage);
>> 186:           if ((pretouch_result.committed - no_pretouch_result.committed) < expected_delta) {
>> 187:             throw new RuntimeException("Expected a higher delta between stack committed of with and without pretouch.");
> 
> Can you print out the expected values, please?

Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20531#discussion_r1770940677
PR Review Comment: https://git.openjdk.org/jdk/pull/20531#discussion_r1770941050
PR Review Comment: https://git.openjdk.org/jdk/pull/20531#discussion_r1770941379