The patch as it is will make things much better, but it may be worth
"doing it right" as long as we are revisiting this algorithm and write
it to deal better with the OOME/integer overflow cases.
I looked at the ArrayList code and found a bit of voodoo there in the
overflow code which troubled me as a potential maintainer. It's one
thing to write clever code, it's another thing to write clever code that
others can come along and maintain without having to fill a white board
with input/output ranges and 2's complement rules. It's not like this
code is going to be in an inner loop somewhere - the time for a couple
of conditional checks and min/maxes will be vastly swallowed by the
costs of allocation and copying data so it would be better to just write
straightforward code whose overflow considerations are documented for
future maintainers. (Having said that I probably wrote some pretty
obtusely clever code in the Rectangle class myself... D'oh!) My general
goal is to include comments with the incoming assumptions and then
document how I've ruled out cases and narrowed ranges with small
single-line comments as the calculations and decisions are made...
...jim
On 4/3/15 6:37 AM, Laurent Bourgès wrote:
Sergey,
2015-04-03 15:16 GMT+02:00 Sergey Bylokhov <sergey.bylok...@oracle.com
<mailto:sergey.bylok...@oracle.com>>:
Hello.
Is that a problem that with the new code we can get OOM earlier in
some cases? Should we take care of overflow more carefully now? It
seems that ArrayList.grow contains similar logic.
I advocate I did not test with a small heap (Xmx32m ...)
I agree my patch will waste more memory : 1/8th for both arrays (byte[]
and float or double[]) instead of 500 byte and 1000 float/double values !
I looked at ArrayList.grow(int) and it deals with integer overflow ie
Integer.MAX_VALUE ... I did not imagine such big paths !
But you're right: my code can be improved to deal with OOME and overflow.
In case of OOME, it may be possible to try allocating another array with
a smaller grow ?
Is it what you had in mind ?
Cheers,
Laurent
