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.gra...@oracle.com
<mailto:james.gra...@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
