[OpenJDK 2D-Dev] RFR 8159093: Fix coding conventions in Marlin renderer

[Jim Graham]

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