RFR 8073354: TimSortStackSize2.java: test cleanup: make test run with single argument

Thu Feb 26 20:53:24 UTC 2015

this cleanup is also required to fix this: 
https://bugs.openjdk.java.net/browse/JDK-8073959

Lev

On 02/26/2015 02:01 PM, Lev Priima wrote:
> Thanks David,
> Are OK with pushing this 
> http://cr.openjdk.java.net/~lpriima/8073354/webrev.01/ cleanup ?
>
> Lev
>
> On 02/26/2015 10:20 AM, David Holmes wrote:
>> On 24/02/2015 8:39 PM, Lev Priima wrote:
>>> I've updated summary and description.
>>
>> Okay - thanks.
>>
>> David
>>
>>> Lev
>>>
>>> On 02/23/2015 04:55 AM, David Holmes wrote:
>>>> On 20/02/2015 7:57 PM, Lev Priima wrote:
>>>>> Functional is pretty same, but:
>>>>>   - make it run with a single argument '67108864' by moving 
>>>>> @summary tag
>>>>> strait down to line after the @run tag
>>>>>   - '-Xmx385' ->'-Xms385' in run command. MaxHeapSize will adjust a
>>>>> accordingly.
>>>>>   - spaces and code style.
>>>>>
>>>>> Of course this issue may be closed as dup of JDK-8073354 and test 
>>>>> will
>>>>> pass after applying JDK-8073354. But I just want to use this RFE 
>>>>> ID to
>>>>> make test run with a single argument ( as you noticed previously ).
>>>>
>>>> So this is all just cleanup - which is fine in itself but seems to
>>>> have no bearing on the issue reported/described in the bug report. If
>>>> you want to use this bug ID then I think you need to change the bug
>>>> report substantially else it will just be confusing.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>
>>>>> Lev
>>>>>
>>>>> On 02/20/2015 05:26 AM, David Holmes wrote:
>>>>>> On 19/02/2015 11:23 PM, Lev Priima wrote:
>>>>>>> After Jespers comments I removed catch of OOME and added minimum 
>>>>>>> heap
>>>>>>> size required for test(-Xms385) to overcome default ergonomics for
>>>>>>> client.
>>>>>>> Please review: 
>>>>>>> http://cr.openjdk.java.net/~lpriima/8073354/webrev.01/
>>>>>>
>>>>>> Didn't see this before sending my previous reply.
>>>>>>
>>>>>> I'm confused now. What functional change is now being made here ??
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>>> Lev
>>>>>>>
>>>>>>> On 02/19/2015 03:00 PM, Lev Priima wrote:
>>>>>>>> There is also a problem, if the memory on host is highly 
>>>>>>>> fragmented,
>>>>>>>> then we can't allocate continuous amount of heap for creating such
>>>>>>>> big
>>>>>>>> array. I've added catch for OOME and treat this case as 
>>>>>>>> skip-pass to
>>>>>>>> make test more robust. Also I've removed explicit -Xmx385 from 
>>>>>>>> @run
>>>>>>>> tag and made it start with a single argument.
>>>>>>>>
>>>>>>>> Please review and push:
>>>>>>>> http://cr.openjdk.java.net/~lpriima/8073354/webrev.00/
>>>>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8073354
>>>>>>>>
>>>>>>>> Lev
>>>>>>>>
>>>>>>>> On 02/17/2015 09:55 AM, David Holmes wrote:
>>>>>>>>> On 17/02/2015 3:43 PM, Lev Priima wrote:
>>>>>>>>>> Thanks David!
>>>>>>>>>> Is this expected behavior of this annotation ?
>>>>>>>>>
>>>>>>>>> Yes that is the way jtreg defines tags:
>>>>>>>>>
>>>>>>>>> http://openjdk.java.net/jtreg/tag-spec.html
>>>>>>>>>
>>>>>>>>> "The argument tokens of a tag extend from the first token 
>>>>>>>>> after the
>>>>>>>>> tag token to the end of the comment, the end of the file, or the
>>>>>>>>> next
>>>>>>>>> tag token, whichever comes first."
>>>>>>>>>
>>>>>>>>> So everything between @run and @summary are taken to be the @run
>>>>>>>>> commands. And there's no @comment tag unfortunately.
>>>>>>>>>
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>>> Lev
>>>>>>>>>>
>>>>>>>>>> On 02/17/2015 03:20 AM, David Holmes wrote:
>>>>>>>>>>> On 16/02/2015 9:20 PM, David Holmes wrote:
>>>>>>>>>>>> On 16/02/2015 6:59 PM, Lev Priima wrote:
>>>>>>>>>>>>> Thanks, David,
>>>>>>>>>>>>> Could you please push it ?
>>>>>>>>>>>>
>>>>>>>>>>>> I will if Roger doesn't get to it first. It'll be 11 hours 
>>>>>>>>>>>> before
>>>>>>>>>>>> I can
>>>>>>>>>>>> push it.
>>>>>>>>>>>
>>>>>>>>>>> This has been pushed but note there is a minor issue with the
>>>>>>>>>>> test.
>>>>>>>>>>> The jtreg tag specification doesn't terminate tags on newlines,
>>>>>>>>>>> they
>>>>>>>>>>> continue until the next tag is encountered or the end of the
>>>>>>>>>>> comment.
>>>>>>>>>>> Consequently this:
>>>>>>>>>>>
>>>>>>>>>>>  * @run main/othervm -Xmx385m TimSortStackSize2 67108864
>>>>>>>>>>>  * not for regular execution on all platforms:
>>>>>>>>>>>  * run main/othervm -Xmx8g TimSortStackSize2 1073741824
>>>>>>>>>>>  * run main/othervm -Xmx16g TimSortStackSize2 2147483644
>>>>>>>>>>>
>>>>>>>>>>> is processed as:
>>>>>>>>>>>
>>>>>>>>>>> @run main/othervm -Xmx385m TimSortStackSize2 67108864 not for
>>>>>>>>>>> regular
>>>>>>>>>>> execution on all platforms: run main/othervm -Xmx8g
>>>>>>>>>>> TimSortStackSize2
>>>>>>>>>>> 1073741824 run main/othervm -Xmx16g TimSortStackSize2 
>>>>>>>>>>> 2147483644
>>>>>>>>>>>
>>>>>>>>>>> and so TimSortStackSize2 is invoked with 18 arguments.
>>>>>>>>>>>
>>>>>>>>>>> David
>>>>>>>>>>> -----
>>>>>>>>>>>
>>>>>>>>>>>> David
>>>>>>>>>>>>
>>>>>>>>>>>>> Lev
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 02/16/2015 08:55 AM, David Holmes wrote:
>>>>>>>>>>>>>> On 14/02/2015 12:03 AM, Lev Priima wrote:
>>>>>>>>>>>>>>> Please review and push:
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~lpriima/8073124/webrev.00/
>>>>>>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8073124
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I hadn't realized 8072909 had been pushed without final
>>>>>>>>>>>>>> reviewer
>>>>>>>>>>>>>> comments being addressed. :(
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> These changes seem okay. I hope they get promoted at the 
>>>>>>>>>>>>>> same
>>>>>>>>>>>>>> time as
>>>>>>>>>>>>>> the original changeset so we don't get test failures.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Lev
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 02/13/2015 05:20 AM, David Holmes wrote:
>>>>>>>>>>>>>>>> Hi Lev,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 13/02/2015 2:56 AM, Lev Priima wrote:
>>>>>>>>>>>>>>>>> Christos,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Test may fail on shorter arrays(page 8 of paper). For
>>>>>>>>>>>>>>>>> instance, on
>>>>>>>>>>>>>>>>> worst
>>>>>>>>>>>>>>>>> case, generated by test, it starts to fail on length
>>>>>>>>>>>>>>>>> 67108864.
>>>>>>>>>>>>>>>>> After increasing stack size of runs to merge,
>>>>>>>>>>>>>>>>> Arrays.sort(T[])
>>>>>>>>>>>>>>>>> works
>>>>>>>>>>>>>>>>> also on maximum possible array for HotSpot JVM.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I'd also like to see this documented somewhere in the 
>>>>>>>>>>>>>>>> code.
>>>>>>>>>>>>>>>> Presently
>>>>>>>>>>>>>>>> there is a reference to listsort.txt but then you have 
>>>>>>>>>>>>>>>> to go
>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>> find
>>>>>>>>>>>>>>>> it on the web. :( At a minimum could we please add:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>  175          * computation below must be changed if
>>>>>>>>>>>>>>>> MIN_MERGE is
>>>>>>>>>>>>>>>> decreased.  See
>>>>>>>>>>>>>>>>  176          * the MIN_MERGE declaration above for more
>>>>>>>>>>>>>>>> information.
>>>>>>>>>>>>>>>> +             * The maximum value of 49 allows for an 
>>>>>>>>>>>>>>>> array
>>>>>>>>>>>>>>>> up to
>>>>>>>>>>>>>>>> length
>>>>>>>>>>>>>>>> +             * Integer.MAX_VALUE-4.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Roger, David,
>>>>>>>>>>>>>>>>> I've updated the test (
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~lpriima/8072909/webrev.01/test/java/util/Arrays/TimSortStackSize2.java.html 
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> ) to make it more suitable for regular execution:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>    27  * @run main/othervm TimSortStackSize2 67108864
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> This will still fail on small memory devices:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> :~> java TimSortStackSize2 67108864
>>>>>>>>>>>>>>>> Exception in thread "main" java.lang.OutOfMemoryError: 
>>>>>>>>>>>>>>>> Java
>>>>>>>>>>>>>>>> heap
>>>>>>>>>>>>>>>> space
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> as the default heap ergonomics may not be large enough. I
>>>>>>>>>>>>>>>> had to
>>>>>>>>>>>>>>>> add a
>>>>>>>>>>>>>>>> minimum heap of -Xmx385M to get it to run.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>> David
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>    28  * not for regular execution on all platforms:
>>>>>>>>>>>>>>>>>    29  * run main/othervm -Xmx8g TimSortStackSize2
>>>>>>>>>>>>>>>>> 1073741824
>>>>>>>>>>>>>>>>>    30  * run main/othervm -Xmx32g TimSortStackSize2
>>>>>>>>>>>>>>>>> 2147483644
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Could you please push this:
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~lpriima/8072909/webrev.01/
>>>>>>>>>>>>>>>>> ?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Lev
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 02/12/2015 12:54 PM, christos at zoulas.com wrote:
>>>>>>>>>>>>>>>>>> On Feb 12, 9:57pm,david.holmes at oracle.com (David Holmes)
>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>> -- Subject: Re: 8072909: TimSort fails with
>>>>>>>>>>>>>>>>>> ArrayIndexOutOfBoundsException on
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> | Ok - thanks Lev!
>>>>>>>>>>>>>>>>>> |
>>>>>>>>>>>>>>>>>> | David
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> For posterity can someone document this, and also the
>>>>>>>>>>>>>>>>>> value for
>>>>>>>>>>>>>>>>>> which
>>>>>>>>>>>>>>>>>> Integer.MAX_VALUE-4 fails?
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> christos
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>
>