[OpenJDK 2D-Dev] RFR: 8147443: Use Common Cleaner in Marlin OffHeapArray

[Phil Race]

Yes, sorry .. Monday was a holiday here so I am behind.

-phil.

On 01/20/2016 11:59 AM, Laurent Bourgès wrote:
>
> Phil,
>
> Just a reminder: do you approve the last webrev ?
>
> Laurent
>
> Le 19 janv. 2016 16:32, "Laurent Bourgès" <bourges.laurent at gmail.com 
> <mailto:bourges.laurent at gmail.com>> a écrit :
> >
> > Phil,
> >
> > I adopted your approach and please review the updated webrev:
> > http://cr.openjdk.java.net/~lbourges/marlin/marlin-Cleaner.2/ 
> <http://cr.openjdk.java.net/%7Elbourges/marlin/marlin-Cleaner.2/>
> >
> > Changes:
> > - RendererContext: added cleanerObj used as Cleaner's parent 
> reference + avoid updating the weak reference when using an hard reference
> > - Renderer & MarlinCache: use new OffHeapArray(rdrCtx.cleanerObj,...)
> > - updated Version to 'marlin-0.7.3-Unsafe-OpenJDK'
> >
> > Hope it will be good,
> >
> > Laurent
> >
> >
> > 2016-01-19 16:06 GMT+01:00 Roger Riggs <roger.riggs at oracle.com 
> <mailto:roger.riggs at oracle.com>>:
> >>
> >> Hi,
> >>
> >> This change looks ok to me (from the Cleaner perspective).
> >>
> >> (Phil, I'm assuming its just a typo that cleanerRef and cleanerObj 
> should be the same.)
> >>
> >> Roger
> >>
> >>
> >>
> >> On 1/15/16 6:55 PM, Phil Race wrote:
> >>>
> >>> This looks fine to me. One comment that I am not sure applies here
> >>> is that when using the equivalent '2d disposer' mechanism, the object
> >>> we pass to the disposer to track when to dispose the resources is kept
> >>> as small as possible. For example you have :
> >>>     Renderer(final RendererContext rdrCtx) {
> >>>         this.rdrCtx = rdrCtx;
> >>>
> >>>         this.edges = new OffHeapArray(rdrCtx, 
> INITIAL_EDGES_CAPACITY); // 96K
> >>>
> >>> Now if we add one new field  to RendererContext :
> >>>   Object cleanerObj = new Object();
> >>>
> >>> and then change your code to
> >>>         this.edges = new OffHeapArray(rdrCtx.cleanerRef, 
> INITIAL_EDGES_CAPACITY); // 96K
> >>>
> >>> then GC should be able to free up the rest of rdrCtx more promptly.
> >>>
> >>> -phil
> >>>
> >>> On 01/15/2016 02:04 PM, Laurent Bourgès wrote:
> >>>>
> >>>> Dear all,
> >>>>
> >>>> Sorry, I need to restart properly the review thread that occured 
> in another topic:
> >>>> 
> http://mail.openjdk.java.net/pipermail/2d-dev/2016-January/006145.html
> >>>>
> >>>>
> >>>> Please review this webrev refactoring Marlin OffHeapArray to use 
> the new jdk.internal.ref.CleanerFactory (new Cleaner API) instead of 
> my own solution (ReferenceQueue + PhantomReference + Thread):
> >>>> http://cr.openjdk.java.net/~lbourges/marlin/marlin-Cleaner.1/ 
> <http://cr.openjdk.java.net/%7Elbourges/marlin/marlin-Cleaner.1/>
> >>>>
> >>>> bug: https://bugs.openjdk.java.net/browse/JDK-8147443
> >>>>
> >>>> The new API is really great as it just needs 1 line:
> >>>>
> >>>> +        // Register a cleaning function to ensure freeing 
> off-heap memory:
> >>>> + CleanerFactory.cleaner().register(parent, () -> this.free());
> >>>>
> >>>>
> >>>> I added a qualified export in modules.xml:
> >>>>
> >>>>      <export>
> >>>> + <name>jdk.internal.ref</name>
> >>>> +      <to>java.desktop</to>
> >>>> +    </export>
> >>>> +    <export>
> >>>>
> >>>>
> >>>> FYI there is no performance issue as all off-heap arrays are 
> belonging to a Marlin's RendererContext (parent reference).
> >>>> The RendererContext instances are stored as thread-local 
> variables using a SoftReference (or a WeakReference) to maximize the 
> potential reuse (like a  buffer) but avoid OOME.
> >>>>
> >>>>
> >>>> Finally I tested the patch using the following settings to use 
> weak references to
> >>>> RendererContexts and enable logging all allocations / free 
> operations in
> >>>> OffHeapArray:
> >>>>
> >>>> -Dsun.java2d.renderer.log=true
> >>>> -Dsun.java2d.renderer.logUnsafeMalloc=true
> >>>> -Dsun.java2d.renderer.useRef=weak
> >>>>
> >>>> INFO: 1452703097931: OffHeapArray.allocateMemory = 65536 to addr 
> = 140642864899472
> >>>> INFO: 1452703097931: OffHeapArray.allocateMemory = 98304 to addr 
> = 140642864965024
> >>>> ...
> >>>> INFO: 1452703103321: OffHeapEdgeArray.free = 98304 at addr = 
> 140642864965024
> >>>> INFO: 1452703103321: OffHeapEdgeArray.free = 65536 at addr = 
> 140642864899472
> >>>> ...
> >>>> INFO: 1452703103827: OffHeapArray.allocateMemory = 65536 to addr 
> = 140642864910240
> >>>> INFO: 1452703103827: OffHeapArray.allocateMemory = 98304 to addr 
> = 140642864975792
> >>>> ...
> >>>> INFO: 1452703113822: OffHeapEdgeArray.free = 98304 at addr = 
> 140642864975792
> >>>> INFO: 1452703113822: OffHeapEdgeArray.free = 65536 at addr = 
> 140642864910240
> >>>> ...
> >>>>
> >>>> As mentioned by Phil, other Java2D classes (like Disposer) may 
> use the new Cleaner approach in the future and potentially need a 
> dedicated Cleaner instance (Thread) for performance sensitive code.
> >>>>
> >>>> Best regards,
> >>>> Laurent
> >>>
> >>>
> >>
> >
> >
> >
> > --
> > --
> > Laurent Bourgès
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20160120/0a5e8ed3/attachment.html>