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.laur...@gmail.com
<mailto:bourges.laur...@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.ri...@oracle.com
<mailto:roger.ri...@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
