[icedtea-web] RFC: Handle a requested heap size of '1g'

[Jiri Vanek]

On 03/31/2014 03:48 PM, Omair Majid wrote:
> * Jiri Vanek <jvanek at redhat.com> [2014-03-31 09:33]:
>> On 03/28/2014 10:44 PM, Omair Majid wrote:
>>> I just ran across this jnlp file: http://atlaslivecasino.net/casino_game.jnlp
>>>
>>> This fails to work with HEAD because the application asks for a heap
>>> size of '1G'. This is not a regression.
>>
>> What about t and T ? O:)
>
> The man page for java suggests the valid options are: k/K/m/M/g/G.
>
>> I do not wont to be nitpicker, but I think this code desreves a bit more refactoring>
>>
>> Eg the one attached?
>
> Sure, this looks even better!
>
>> Feel free to push anyone you like more.
>
> I don't feel appropriate taking credit for the much nicer patch you
> wrote. Unless there is a strong reason to, please call yourself the
> author :)
>
>> ps: please jsut keep the  JREDesc a =  in tests. It removed NB  warning.
>
> Ugh. Really?
>
>> +++ b/netx/net/sourceforge/jnlp/JREDesc.java	Mon Mar 31 15:32:48 2014 +0200
>
>> +        //0 or 0k/m/g is still valid as it was
>
> Please remove this line/comment ^
>
>> +        String heapSizeLower = heapSize.toLowerCase();
>> +        boolean match = heapSizeLower.matches("\\s*\\d+[kmgtp]{0,1}\\s*");
>
> '?' is the same as '{0,1}', right? Would that be cleaner to use?
>
> If we are accepting leading or trailing spaces here, are we passing
> those to the JVM too? Maybe we should trim leading/trailing spaces
> before storing the sizes?
>
>> +++ b/tests/netx/unit/net/sourceforge/jnlp/JREDescTest.java	Mon Mar 31 15:32:48 2014 +0200
>
>> +        JREDesc a = new JREDesc(null, null, null, "1", null, null);
>
>> +        JREDesc.checkHeapSize("10");
>
> These two lines are doing pretty much exactly the same thing, but two
> different ways. I think the tests should check the heap size in the same
> way everywhere. Either through JREDesc.checkHeapSize everywhere or
> through the constructor everywhere. Is there a benefit to mixing both?


I think you will see the benefit in this a bitmore refactored version.

Rest fixed.

Ok to go?
>
> Thanks,
> Omair
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: heap-size-bug3.patch
Type: text/x-patch
Size: 9121 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140331/e280bfe0/heap-size-bug3-0001.patch>