RFR 8252249: nsk/stress/stack/stack016.java fails with "Error: TEST_BUG: trickyRecursion() must throw an error anyway!"

[Harold Seigel]

Thanks!

Harold

On 9/2/2020 9:02 AM, David Holmes wrote:
> Hi Harold,
>
> No need for updated webrev.
>
> David
>
> On 2/09/2020 10:20 pm, Harold Seigel wrote:
>> Hi David,
>>
>> Thanks for reviewing this.  Please see comments inline.
>>
>> Let me know if you need to see a new webrev.
>>
>> Thanks, Harold
>>
>> On 9/2/2020 2:26 AM, David Holmes wrote:
>>> Hi Harold,
>>>
>>> On 2/09/2020 5:02 am, Harold Seigel wrote:
>>>> Hi,
>>>>
>>>> Please review this change to hotspot test 
>>>> vmTestbase/nsk/stress/stack/stack016.java.  The test calls a 
>>>> recursive method and keeps track of the number of repetitions 
>>>> needed to cause an exception.  It then runs a bunch of threads that 
>>>> call the recursive method for a multiple of the repetition number, 
>>>> expecting each of them to get a StackOverflowError or 
>>>> OutOfMemoryError exception. Occasionally, the test fails because 
>>>> one of the threads does not throw an exception.
>>>>
>>>> This change tries to fix this in two ways.  One, by making sure 
>>>> that the thread used to determine the number of repetitions gets a 
>>>> StackOverflowError or OutOfMemoryError exception, and not some 
>>>> other unexpected exception. 
>>>
>>> That's a good improvement, though other exceptions seem unlikely - 
>>> have you actually observed any unexpected exceptions?
>> No, but they would be hard to see since the test was eating them.
>>>
>>>> The other way is to run the test twice, once with -Xcomp and once 
>>>> with -Xint, to ensure that thread stack consumption doesn't vary 
>>>> because the original thread called an interpreted method and a 
>>>> subsequent thread called a compiled method.
>>>
>>> Right - running interpreted would give a different maximum recursion 
>>> depth from running under the JIT, and when we bumped up the stack 
>>> depth we would have run far more iterations and so JIT'd more code - 
>>> thus breaking the test. So forcing fully interpreted or fully 
>>> compiled seems a good way to stabilise things.
>>>
>>> But I would suggest that you keep the required condition
>>>
>>> vm.compMode != "Xcomp"
>>>
>>> to minimise the runs of this test. When executed as part of an Xcomp 
>>> run both @run's will behave exactly the same way (Xcomp) - and that 
>>> is the same as the second @run in a non-Xcomp run. So no point 
>>> running the Xcomp version three times.
>> Thanks for pointing this out.  I'll restore 'vm-compMode != "Xcomp"' 
>> before pushing the change.
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>>>
>>>> Open Webrev: 
>>>> http://cr.openjdk.java.net/~hseigel/bug_8252249.stack/webrev/index.html 
>>>>
>>>>
>>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8252249
>>>>
>>>> The modified test was tested on Mac OS, Linux x64, and Windows.
>>>>
>>>> Thanks, Harold
>>>>