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
