RFR 8161696 [TESTBUG] runtime/StackGuardPages/testme.sh uses invalid argument -Xss328k

[Daniel D. Daugherty]

On 7/27/16 7:48 AM, Gerald Thornbrugh wrote:
> Hi Fred,
>
> Thanks for the information.
>
> How does everyone feel about implementing Fred's suggestion of a stack 
> size of 1MB?
> As Fred states this not a complete fix but it would provide relief 
> from this issue for the
> foreseeable future.

Fred, sorry for the mis-attribution. I'm not sure why I thought you
wrote the test...

Short answer:

I'm good with changing the test to use a stack size of 1MB.


Longer answer (with some historical bits):

Looks like Dmitry S. created the current version of the test with
this changeset:

$ hg log -v -r 16432bba405a
changeset:   6482:16432bba405a
user:        dsamersoff
date:        Thu May 22 13:22:09 2014 -0700
files:       test/runtime/6929067/T.java 
test/runtime/6929067/Test6929067.sh test/runtime/6929067/invoke.c 
test/runtime/InitialThreadOverflow/DoOverflow.java 
test/runtime/InitialThreadOverflow/invoke.c 
test/runtime/InitialThreadOverflow/testme.sh 
test/runtime/StackGuardPages/DoOverflow.java 
test/runtime/StackGuardPages/invoke.c test/runtime/StackGuardPages/testme.sh
description:
8042155: [TESTBUG] Tests for stack guard pages have to be cleaned up
Summary: Tests for 6929067 and 8009062 should be composed to single test 
that provide better testing for stack guard pages behaviour
Reviewed-by: coleenp, dcubed, dsimms

Sifting through the history implies that the test for 8009062 was
derived from 6929067; both tests have -Xss320k in their invoke.c
implementations.

$ hg log -v -r 3b3d12e645e7
changeset:   1320:3b3d12e645e7
parent:      1292:12d91eb0f579
user:        coleenp
date:        Fri Mar 12 10:42:16 2010 -0500
files:       src/os/linux/vm/os_linux.cpp 
src/os/solaris/vm/os_solaris.cpp src/os/windows/vm/os_windows.cpp 
src/share/vm/runtime/os.hpp src/share/vm/runtime/thread.cpp 
test/runtime/6929067/T.java test/runtime/6929067/Test6929067.sh 
test/runtime/6929067/invoke.c
description:
6929067: Stack guard pages should be removed when thread is detached
Summary: Add code to unmap stack guard area when thread is detached.
Reviewed-by: coleenp, kamg

Based on my email archive for 6929067, the original reproducer was
provided by Andrew Haley back on 2010.02.23 and that reproducer
contained this line:

options[0].optionString = "-Xss320k";

The original test program is pretty simple and doesn't do anything
to grow the stack so it is not clear why the stack is limited to
320K. Force of habit for Andrew? Dunno.

Dan


>
> Thanks,
>
> Jerry
>> Hi Jerry,
>>
>> On 26/07/2016 22:55, Gerald Thornbrugh wrote:
>>> Hi David,
>>>> Hi Jerry,
>>>>
>>>> On 27/07/2016 2:06 AM, Gerald Thornbrugh wrote:
>>>>> Hi Everyone,
>>>>>
>>>>> I would like to have the following change reviewed:
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8161696
>>>>>
>>>>> Webrev:http://cr.openjdk.java.net/~gthornbr/8161696/hotspot-webrev.01/ 
>>>>>
>>>>> <http://cr.openjdk.java.net/%7Egthornbr/8161696/hotspot-webrev.01/>
>>>>>
>>>>> The stack size for this test needs to be increased to 384K for some
>>>>> platforms.
>>>>
>>>> Is there a specific reason we set -Xss in the first place instead of
>>>> just taking the platform defaults? If the platform stack settings
>>>> change this test may need updating again.
>>>
>>> It was my understand (maybe incorrectly) that setting -Xss was an
>>> attempt to limit the memory and time
>>> resources the test takes.  Using the smallest stack size allowable will
>>> allocate less memory and reduce the
>>> amount of time the test takes.  Since I think Fred wrote the test he
>>> would know better than I would.
>>>
>>> Fred, what is the reason behind specifying "-Xss" for this test?
>>
>> I'm not the author of this test, but I can try to contribute my
>> two cents.
>>
>> I'm not seeing any particular reason to limit the stack size to
>> 320KB other than reducing time and resources required for this
>> test.
>>
>> Looking at the test history, Dmitry did the current version of the
>> test, but the "-Xss320k" argument was already there before Dmitry
>> re-wrote the test to fix 8042155. So it looks like a relic from
>> old code for which the rational has been lost.
>>
>> Regarding the test, limiting the stack size looks a good idea to
>> avoid wasting too much time and resources during testing. However,
>> I'm seeing to reason to make the stack size "as small as possible".
>> A 1MB stack size could already provide efficient improvement while
>> avoiding platform dependent stack size constraint (and I'll be
>> cursed for this by next generations of HotSpot developer as a
>> 1MB limitation could become completely unjustified in the future).
>>
>> Fred
>>
>>> I have thought about the test harness setting an environment variable
>>> and/or a java property to the correct
>>> minimum values for "-Xss" depending upon the architecture the test is
>>> running on.  The tests could then use
>>> the value to start each test. That way each test could be tailored for
>>> the specific architecture. After looking into
>>> this it looks the implementation would be significant and would be
>>> outside the scope of this bug fix.
>>>
>>> Thanks,
>>>
>>> Jerry
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> My fix for JDK-8081770 
>>>>> https://bugs.openjdk.java.net/browse/JDK-8081770
>>>>> moved the functionality
>>>>> from the testme.sh script to exeinvoke.c so the dependency for a 
>>>>> local
>>>>> compiler could be removed.
>>>>> Because of this the stack size change needs to be applied to 
>>>>> exeinvoke.c
>>>>> file instead of the testme.sh file.
>>>>>
>>>>> Please let me know if you have any questions or concerns.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Jerry
>>>
>