Phil, Just a reminder: do you approve the last webrev ?
Laurent Le 19 janv. 2016 16:32, "Laurent Bourgès" <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/ > > 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
