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

Thu Feb 26 11:01:42 UTC 2015

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