[OpenJDK 2D-Dev] RFR 8148886: SEGV in sun.java2d.marlin.Renderer._endRendering

[Laurent Bourgès]

Jim,

Here is the updated webrev:
http://cr.openjdk.java.net/~lbourges/marlin/marlin-8148886.4/

Changes:
- HardReference adopted: it simplified the code as I removed the wrapper /
factory classes but also allowed me to apply generic types: Reference<?
extends ReentrantContext> instead of Object
- ReentrantContextProviderTL: added a constructor with 2 reference types
(TL / CLQ), the default one uses weak reference for CLQ
- added javadoc in ReentrantContext* classes: please correct my english
sentences !
I am not fluent enough to write clearly such documentation and I do not
know how much details are needed or not as it is a internal API.

About the only simplifying suggestion I might have is that if you can
> subclass Reference and create HardReference<T> extends Reference<T> with an
> obvious implementation, then you wouldn't have to special case the HARD
> type and you could get rid of a lot of your use of raw "Object" references
> and replace them with generified code.  Unfortunately Reference is not
> subclassable outside the java.lang.ref package.  I looked online and found
> a number of people considering subclassing a HardReference variant off of
> WeakReference - as long as the subclass keeps a hard reference the referent
> in the super class is not relevant.  That mechanism may be a little clunky,
> but it would mean that you can always assume that the TL.get() and
> CLQ.poll() are returning a Reference<K> and just call get() on it without
> having to use a factory/accessor.
>

That's absolutely what I always needed !
Several times I wondered why no such HardReference exists in the
java.lang.reference package:
should we tell core-libs that it is really needed to improve homogeneity /
consistency ... and simplify such reference-based code ?


> public class HardReference<T> extends WeakReference<T> {
>     private final T strongRef;
>     public HardReference(T obj) {
>         super(null);
>         this.strongRef = obj;
>     }
>     public T get() { return strongRef; }
> }
>

I added such class in ReentrantContextProvider but I did not use
'super(null)'. What is the aim ?
Does it simplify the reference processing ? Of course such HardReference
does not need any reference queue...


> Another way to crack that egg would be to simply embed the following code
> where you call the resolveReference methods:
>     ctx = (obj instanceof Reference<K>) ? ((Reference<K>) obj).get() :
> ((K) obj);
> (No need for a null test there because instanceof ensures non-nullness and
> you don't care about which form of reference it is since they all share the
> same get() method.)
>

It was the case before in MarlinRenderingEngine but I wanted to avoid most
conditional statements ... .


> If you made either of those changes then the Wrapper classes would only
> exist to create the appropriate Reference object and could be left as
> factories or simply be replaced with a wrap(ctx) method in the parent with
> a switch statement in it.  Since it is only called on construction, the
> switch statement should have negligible overhead and eliminate 4 inner
> classes (parent Wrapper and 3 flavors of Wrapper).
>

Wrapped in ReentrantContextProvider.getOrCreateReference(ctx).

Thanks for your suggestions,
Laurent
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20160210/04e81114/attachment.html>