[OpenJDK Rasterizer] Marlin renderer contribution for review

Laurent Bourgès bourges.laurent at gmail.com
Wed Mar 25 21:39:59 UTC 2015


Hi Jim,

> Briefly I checked out the line lengths only in Renderer.java and they
seem good now.
Ok, I fixed also MarlinConst / RendererStats you pointed out.

> But you changed a number of other things that I'm now going to have to
review.  Can you summarize not just the "what changed", but also the "where
it changed" so I don't have to look over all parts of all files trying to
find the changes?
Ok,  sorry. I pushed also my changes onto github that has a nice diff
tool... you can not use.

You could maybe compare both marlin.x/jdk.patch files.

> Also, if we can hold off on further development work until we get a stake
in the ground it would avoid this "having to review a huge changeset over
again from scratch" due to ancillary fixes.

Agreed. It was minor changes compared to the huge marlin patch: I will give
you hints below.

> Also, the very first file I brought up still had /** */ single line
faux-doc comments (AAShapePipe, on private fields no less).  That was the
second thing you said that you fixed below...?

I disagree: this is a typical javadoc comment for a field (even private).
Maybe you dislike javadoc for fields.
What's clearly the problem ? Do you want me to remove all new comments /
javadoc ?

I said I removed many of them that were redundant or not appropriate
javadoc:
See RendererContext.

I do my best to make it right and spent many hours on preparing & testing
the latest webrev.

I agree it can be painful on your side.

>>     - Fixed Unsafe access

See Renderer lines 52 to 75.

>>     - Use PhantomReference / ReferenceQueue and the new OffHeapDisposer
>>     thread to free off-heap memory to avoid finalization in Renderer

See Renderer lines 52 to 75 and lines 1379 to 1415.

I forgot to remove lines 530 to 537 = deprecated finalize method.

>>     - added the ArrayCachesHolder class in RendererContext to gather all
>>     ArrayCache instances: use it wrapped using a WeakReference to reduce
>>     the memory footprint of large array caches
In RendererContext lines 70 and 140-170 and 340 to eof.

>>     - Use AccessController.doPrivileged(new GetPropertyAction(key)) to
>>     properly get System properties in a secure environment in
>>     MarlinRenderingEngine

MarlinRenderingEngine:  Line 830 to eof.

If you prefer, I can send a diff between webrevs or do more cleanup ?

Laurent
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/graphics-rasterizer-dev/attachments/20150325/3947e708/attachment-0001.html>


More information about the graphics-rasterizer-dev mailing list