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

[Jim Graham]

The getInstance MT issue could be filed as a separate follow-on bug if 
you want, in which case webrev.1 is good to go...

		...jim

On 6/10/2016 2:54 PM, Jim Graham wrote:
> 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