RFR(S) : 8199265 : java/util/Arrays/TimSortStackSize2.java fails with OOM

[David Holmes]

On 27/06/2018 4:42 PM, Igor Ignatyev wrote:
> Hi David,
> 
> thanks for reviewing.
> 
> please see my answers inline.
> 
> -- Igor
> 
>> On Jun 26, 2018, at 11:03 PM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> Hi Igor,
>>
>> Using 2x max heap seems a reasonable approach.
>>
>> AFAICT the other changes are just cleanup and don't make any difference to the way the test is run - right?
> right.
>>
>> Only nit: ProcessTools.executeTestJvm is deprecated in favor of ProcessTools.executeTestJava.
> if you look at their javadocs it looks over way around, executeTestJvm has a proper javadoc, while executeTestJava just says see #executeTestJvm; at least it's so in /test/lib testlibrary. the library in /test/jdk/test/lib is(should be) deprecated in favor of one in /test/lib. I don't have a strong preference b/w ProcessTools::executeTestJava and executeTestJvm though, so if executeTestJava sounds better for more people, I'm fine w/ totally fine w/ using it instead of executeTestJvm.

Okay I didn't know we had two versions of the test library still. That's 
not very good. I was looking at the test/jdk/lib one which has:

    /**
      * @deprecated Use executeTestJava instead
      */
     public static OutputAnalyzer executeTestJvm(String... options) 
throws Exception {

If you're using the other one then I guess it doesn't matter. Having 
both methods seems pointless regardless.

Cheers,
David

>>
>>
>> Thanks,
>> David
>>
>> On 27/06/2018 11:14 AM, Igor Ignatyev wrote:
>>> http://cr.openjdk.java.net/~iignatyev/8199265/webrev.00/
>>>> 10 lines changed: 0 ins; 2 del; 8 mod;
>>> Hi all
>>> could you please review another attempt to make TimSortStackSize2 more robust?
>>> 8190679 changed the test to set both Xmx and Xms to avoid failures caused by Xmx being smaller than Xms (when Xmx is specified as external vm flag). it appears that current values of heap size might not be enough in some cases, therefore setting Xmx equal to Xms leads to OOM.
>>> The proposed fix sets Xmx two times more than Xms, this should be enough for the test to pass and also ensures that Xmx is always bigger than Xms (since the test specific flags are appended, external Xms/Xmx flags will be ignored).
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8199265
>>> webrev: http://cr.openjdk.java.net/~iignatyev/8199265/webrev.00/
>>> testing:  java/util/Arrays/TimSortStackSize2.java multiple times on different platforms
>>> Thanks,
>>> -- Igor
>