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

[Harold Seigel]

Hi Gerard,

Thanks for approving these changes!

I had already looked into what "-eager" meant so it wasn't difficult to 
look at it again.  I think it is poorly named and worth understanding.

I agree that 15 minutes is too long to run these tests.  Normally they 
run in a few minutes or less.

Thanks, Harold

On 8/19/2020 1:17 PM, gerard ziemski wrote:
> 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
>>>
>