The change in modules.xml is fine. Mandy
> On Jan 19, 2016, at 7:32 AM, Laurent Bourgès <[email protected]> > 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 <[email protected]>: > 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
