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.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/
>
> 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
