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

[Mandy Chung]

The change in modules.xml is fine.

Mandy

> On Jan 19, 2016, at 7:32 AM, Laurent Bourgès <bourges.laurent at gmail.com> wrote:
> 
> Phil,
> 
> I adopted your approach and please review the updated webrev:
> http://cr.openjdk.java.net/~lbourges/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>:
> 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/
>>> 
>>> 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