> On Mar 19, 2016, at 7:00 AM, Peter Levart <peter.lev...@gmail.com> wrote: > > Here's the webrev: > > > http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.08.part2/ > >> >> On 03/07/2016 07:35 PM, Mandy Chung wrote: >>> >>> I studied webrev.06priv and the history of JDK-6857566. >>> >>> I’m not comfortable for any arbitrary thread to handle the enqueuing of the >>> pending references (this change is more about the fix for JDK-6857566). >>> >> >> Why? A Thread is a Thread is a Thread... When legacy Cleaner is removed, >> ReferenceHandler thread will be left with swapping pointers only - no custom >> code will be involved. The only things I can think of against using >> arbitrary thread are: >> :
My uncomfort was the fix for JDK-6857566 - both enqueuing pending ref and invoking the cleaning code in an arbitrary thread. Looking at it again - enqueuing the pending reference is not so much of a concern (simply updating the link) but the common cleaner could be used by other code that may only expect to be invoked in system thread that’s still my concern (thinking of thread locals). On the other hand, invoking Deallocator::run (deallocating the native memory) in arbitrary threads has no problem. Consider me being paranoid of the fix for JDK-6857566. The current list of clients using CleanerFactory::cleaner may be safe being called from arbitrary threads but I can’t say what will be added in the future. >> >>> The allocating thread may do a System.gc() that may discover phantom >>> reachable references. All it’s interested is only the direct byte buffer >>> ones so that it can deallocate the native memory. What is the downside of >>> having a dedicated Cleaner for direct byte buffer that could special case >>> for it? >> >> A dedicated Cleaner for direct buffers might be a good idea if other uses of >> shared Cleaner in JDK become heavy. So that helping process Cleanable(s) >> does not involve other unrelated Cleanable(s). But it comes with a price of >> another dedicated background thread. >> Perhaps provide one Cleaner specific for native memory deallocation or anything safe to be called in arbitrary thread. It could provide the entry point for the allocating thread to assist the cleaning (i.e. Bits::reserveMemory could call it). That will make it explicit that this cleaner provides explicit control for other threads to assist the cleaning action (and JavaLangRefAccess would only be used by this special cleaner and not in NIO). All clients of Unsafe.freeMemory could use that special cleaner for native memory deallocation use such as IOVecWrapper, DirectByteBuffer, Marlin’s OffHeapArray. The common cleaner would be kept for other things to use and it should be lazily created to avoid another thread. Does this sound reasonable? Mandy
