Hi Laurent,
The new doc comment and method looks good.
Arrays.copyOf is often faster than allocation+arraycopy (typically
because it doesn't need to clear the entire array first and then fill it
with data in 2 separate passes - Hotspot optimizes that method into a
single operation). The new algorithm could simply use:
return Arrays.copyOf(..., newSize);
inside the try/catch where it currently has:
... = new <type>[newSize];
and then it wouldn't need to use "break" or have the arraycopy after the
loop. It might also benefit from the faster speed of arrayCopy.
In the needRoom() methods, there is one more potential overflow path.
If the array grows to just under MAX_INT, then "numCoords + newCoords"
may overflow to a negative number and the array would not be grown
because the test in needRoom() would fail to trigger a call to the grow
method. We would still get an AIOOBE in the moveTo/lineTo/etc. routine,
but we would insert the new path type into the types array (and
increment numTypes) before we overflowed the coords array. We'd also
bump the numCoords by a couple of entries before we hit the true end of
the array. After the exception, we'd leave the path in a bad state
where it had a recorded segment type that had no associated data (or
partial data) in the coords array. It would be cleaner to test the
"numCoords + newCoords" for overflow in needRoom(). A simple test of
the sum being < 0 should do since both numCoords and newCoords should
always be positive numbers and all pos+pos overflows always result in
negative numbers...
...jim
On 4/10/15 8:07 AM, Laurent Bourgès wrote:
Jim,
Here is the new webrev:
http://cr.openjdk.java.net/~lbourges/path2D/Path2D_needRoom.1/
Changes:
- needRoom() applies your pseudo-code; see expandPointTypes() and
expandCoords()
- added a new public trimToSize() method to Path2D implemented by both
Path2D.Float and Path2D.Double classes
Cheers,
Laurent
2015-04-08 22:53 GMT+02:00 Jim Graham <james.gra...@oracle.com
<mailto:james.gra...@oracle.com>>:
Hi Laurent,
I'd probably do:
int newsizemin = oldcount + newitems;
if (newsizemin < oldcount) {
// hard overflow failure - we can't even accommodate
// new items without overflowing
return failure, throw exception?
}
int newsize = <growth algorithm computation>;
if (newsize < newsizemin) {
// overflow in growth algorithm computation
newsize = newsizemin;
... OR ...
newsize = MAX_INT;
}
while (true) {
try {
allocate newsize;
break; (or return?)
} catch (OOME e) {
if (newsize == newsizemin) {
throw e;
}
}
newsize = newsizemin + (newsize - newsizemin) / 2;
}
