[OpenJDK Rasterizer] [OpenJDK 2D-Dev] Path2d needRoom very slow for huge paths

[Sergey Bylokhov]

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.

02.04.15 23:59, Jim Graham wrote:
> All of your comments are spot on (modulo the confusion over 
> Pisces/Marlin vs Path2D).  And you are right that 500 seems pretty 
> lean to me.  800K path segments seems to be a pretty large outlier, 
> though, do we really see paths that large other than via a test case?
>
> I think I'm good with the proposed fix, though.  RAM sizes have also 
> changed dramatically since 1998... ;)
>
> And trim() couldn't hurt.
>
> Another option for growable arrays that can deal with scalability is a 
> chain of arrays rather than a single growing array.  It can get more 
> complicated to do the bookkeeping, but you can find a decent value of 
> how many entries can hide the incremental growth without having to 
> deal with the overhead of:
>
> - GC'ing the previous array
> - needing 2x the storage for the N-1 and Nth arrays
> - copying the data constantly from array i to i+1
>
>             ...jim
>
> On 4/2/15 1:35 PM, Laurent Bourgès wrote:
>> I can understand to promote the 99% use case (small path) but the
>> performance penalty is too important to me for large paths....
>>
>> Let's have jim's point of view as he knows the complete story...
>>
>> Laurent
>>
>> Le 2 avr. 2015 19:33, "Phil Race" <philip.race at oracle.com
>> <mailto:philip.race at oracle.com>> a écrit :
>>
>>     I should have paid more attention to the context.
>>     So it appears that 500 has been the value since 1998 and
>>     it was made that under this bugid when the code was all
>>     still in GeneralPath.java
>>
>>     https://bugs.openjdk.java.net/__browse/JDK-4134316
>>     <https://bugs.openjdk.java.net/browse/JDK-4134316>
>>
>>     prior to that fix it was even smaller
>>
>>     <     static final int INIT_SIZE = 10;
>>     <     static final int EXPAND_SIZE = 10;
>>     ---
>>     >     static final int INIT_SIZE = 20;
>>     >     static final int EXPAND_MAX = 500;
>>
>>     Looks like something we should get Jim's view on since although
>>     a lot has changed since 1998 there may be some negative
>>     consequences he is aware of.
>>
>>     -phil.
>>
>>     On 04/02/2015 10:15 AM, Laurent Bourgčs wrote:
>>
>>
>>         Phil,
>>
>>         Just a short comment:
>>
>>         > pisces was originally written for the "ME" environment so I 
>> suspect that
>>         > it was very conservative in allocating new heap space. That 
>> clearly does not
>>         > matter so much on desktop so the proportional approach 
>> rather than
>>         > the fixed size approach seems better. 1/8th seems 
>> conservative and
>>         > since the results look good so I don't object.
>>
>>         I think Path2D is part of the java2d api and seems not related
>>         to pisces ?
>>
>>         > I think a new bug id would be required as this is another 
>> thing that
>>         > might be backported to pisces in 8u60.
>>
>>         Agreed.
>>
>>         Laurent
>>
>>


-- 
Best regards, Sergey.