RFR 8073354: TimSortStackSize2.java: test cleanup: make test run with single argument
David Holmes
david.holmes at oracle.com
Thu Feb 26 22:10:46 UTC 2015
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