[OpenJDK 2D-Dev] RFR 8159093: Fix coding conventions in Marlin renderer
Jim Graham
james.graham at oracle.com
Fri Jun 10 21:54:02 UTC 2016
Thanks Laurent,
Eeek, I hate the type[] syntax for declaring arrays, but I guess I have
to grow with the times.
One last thing I just noticed, RendererState.getInstance doesn't protect
against MT access if multiple threads encounter the null instance case
and all decide to make their own...
...jim
On 6/10/2016 4:59 AM, Laurent Bourgès wrote:
> Jim,
>
> I fixed the issues you mentioned, see below.
>
> Here is the new webrev:
> http://cr.openjdk.java.net/~lbourges/marlin/marlin-8159093.1/
>
> I also fixed the bracket position (int val[] => int[] val) in Helpers,
> MarlinRenderingEngine, MarlinTileGenerator classes.
>
> My comments:
>
> 2016-06-10 1:48 GMT+02:00 Jim Graham <james.graham at oracle.com
> <mailto:james.graham at oracle.com>>:
>
>
> In RendererStats, lines 276,277 - is it better to convert to an
> array (which is an inherently risky situation for a concurrent
> collection due to the potential for the size changing between the
> array allocation and the toArray), or to iterate the concurrent
> collection directly? I realize that the toArray() method protects
> against a short array, but is it any better than just directly
> iterating which would deal with the concurrency automatically anyway
> without having to allocate an array. One thing to note, if you
> convert to an array and there is a concurrency issue then the array
> may have a null entry to indicate "this is the end of the list", but
> you don't look for that null entry. A simple "if rdrCtx==null
> break;" statement would be enough to deal with that case.
>
>
> I agree and adopted the simplest solution: iterate directly on the
> concurrent queue.
>
>
> MarlinConst.java - you added DO_FLUSH_STATS, but I don't see it
> getting used anywhere...?
>
>
> Exact; I removed it as it will be only used in the next patch.
>
>
> MarlinRenderingEngine.java - it looks like you eliminated all uses
> of mon_npi_currentSegment, but it is still created in RendererStats...?
>
>
> mon_npi_currentSegment removed in RendererStats.
>
>
> Histogram.java - 2016 copyright
>
>
> Fixed.
>
> Regards,
> Laurent
More information about the 2d-dev
mailing list