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

[David Holmes]

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
>>>