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

Lev Priima lev.priima at oracle.com
Thu Feb 26 22:35:35 UTC 2015


Thanks a lot, David!

Lev

On 02/27/2015 01:10 AM, David Holmes wrote:
> Okay I will push this for you Lev.
>
> Thanks,
> David
>
> On 27/02/2015 6:53 AM, Lev Priima wrote:
>> 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
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>>
>>




More information about the core-libs-dev mailing list