RFR 8251490: [TESTBUG] The Java thread stack size specified is too small for nsk/stress/stack. Specify at least 448k

[gerard ziemski]

hi Harold,


On 8/18/20 2:54 PM, Harold Seigel wrote:
>
> Hi Gerard,
>
> Thanks for looking at this change.  Please see inline comments.
>
> On 8/18/2020 2:06 PM, gerard ziemski wrote:
>> hi Harold,
>>
>> Thank you for fixing this.
>>
>>
>> On 8/18/20 12:19 PM, Harold Seigel wrote:
>>> Hi,
>>>
>>> Please review this small change to fix five 
>>> vmTestbase/nsk/stress/stack tests that are failing in tier5 on Linux 
>>> AArch64.  The change increases the stack size for three of the 
>>> tests.  Increasing the stack size of the other two tests could 
>>> result in timeouts on other platforms.  So, those tests were changed 
>>> to no longer run on AArch64.
>>>
>>> Open Webrev: 
>>> http://cr.openjdk.java.net/~hseigel/bug_8251490/webrev/index.html
>>>
>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8251490
>>>
>>> The change was tested by running the tests as part of mach5 tier5 on 
>>> Windows, Mac, Linux x64, and Linux AArch64.
>>>
>>> Thanks, Harold
>>>
>> It looks reasonable to me, but I have some questions:
>>
>> #1 Does "-eager" really make much difference here? Do you have any 
>> timing numbers you can provide with and without that flag, just so 
>> that we know how much it helps?
>
> This change does not affect the use of "-eager".  "-eager" has been 
> part of the tests at least as far back as JDK-11.  The only affect of 
> running with "-eager" is to prevent the creation of a Terminator 
> thread that waits a designated amount of time before calling 
> System.exit().  Its purpose is described here in Terminator.java:
>
>     /**
>      * Terminator is used to terminate a stress test with PASS exit status
>      * before the test is terminated as timed out (and so failed).
>      *
>      * Terminator class holds a thread which sleeps for the given amount
>      * of time, and then wakes up and executes System.exit()
>      * with the given exit status. That thread is daemon, so it doesn't
>      * prevent application from exiting once all its threads finish
>      * before it's time for termination. Appointing terminator in zero
>      * delay implies immediate exit().
>      *
>      * There is a limitation: you may appoint no more than one terminator
>      * per application.
>      */
>
Sorry Harold, I did not mean to make you investigate this option. I 
thought this was something else (i.e. compile java methods instantly).


>>
>> #2 How did you determine the the 900s timeout? How long does it take 
>> for a regular, middle of the road machine to execute each test? Do we 
>> use any slow/low end machines in our testing that could take longer 
>> than this time out? (I'm worried about that hardcoded time out value 
>> if you can tell)
>
> I did not determine the 900s timeout.  That also has been in the tests 
> for at least as far back as JDK-11.  When working on JDK-8139875, I 
> ran the tests many times with different stack sizes both locally and 
> with mach5 and looked at the run times for the tests, making sure they 
> were all significantly below the timeout.  So far, the tests have run 
> as part of tier5 for almost two weeks without timing out.
>
> The purpose of this proposed change is provide stack size values that 
> will work on AArch64.
>
That part looks fine.


> Do you think a new bug is needed to address the use of -eager and the 
> 900s timeout?
>
900s is 15 minutes. On one hand it needs to be long enough for tests not 
to timeout, but if any of these test need to run for such long, then it 
seems excessive. But no, I don't think this needs to be investigated 
further if your fix solves the issue at hand.


Thank you for taking the time to answer my questions.


cheers
>
> Thanks, Harold
>
>>
>>
>> cheers
>>