RFR: JDK-8149925 We don't need jdk.internal.ref.Cleaner any more

Peter Levart peter.levart at gmail.com
Wed Mar 9 21:16:09 UTC 2016


Hi Mandy, Chris, Kim, Roger and others,

Hearing no objections for a day, two Reviewers saying it looks ok and 
successfully re-running the tests, I pushed webrev.07.part1 to jdk9-dev.

Thanks for reviews and comments.

Now to the 2nd part...

On 03/07/2016 07:35 PM, Mandy Chung wrote:
> ...
>> And here's the 2nd part that applies on top of part 1:
>>
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.07.part2/
>>
>>
>> Together they form functionally equivalent change as in webrev.06priv with only two additional cosmetic changes to part 2 (renaming of method Cleaner.cleanNextPending -> Cleaner.cleanNextEnqueued and removal of an obsolete comment in nio Bits).
>>
> 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:

- the thread could have lower priority than RaferenceHandler thread, so 
stealing a chunk of references from the pending list and enqueueing them 
in low-priority thread might lead to reduced throughput.
- the thread could have used almost all of it's stack before calling the 
ByteBuffer.allocateDirect() so there's a danger of StackOverflowError(s) 
when executing the sections of Reference.enqueuePendingReferences() 
method, loosing in effect a chunk of pending References that have been 
unhooked from the pending list.

If this is what you are concerned about then maybe we just need a way to 
synchronize with ReferenceHandler thread to wait until it enqueues all 
pending references discovered by the time the synchronization was 
requested, but otherwise not do any enqueuing... I'll think about that 
approach.

>   I like your proposed change to take over handling the whole chain of pending references at once.  The unhookPhase and enqueuePhase add the complexity that I think we can avoid.

That's necessary in my approach for the synchronization with threads 
that do the concurrent enqueueing (ReferenceHandler thread, for 
example). For example, a thread that comes before us and unhooks a chunk 
of pending references might still be enqueuing them while we discover 
that the pending list is empty and think that all references discovered 
so far have already been enqueued. We must wait for them to be enqueued 
before continuing. Reference.enqueuePendingReferences() is meant to be 
called in pair right after System.gc():

System.gc(); // discover Reference(s)
Reference.enqueuePendingReferences(); // enqueue Reference(s) discovered 
by System.gc() above
// if ReferenceHandler thread steals a chunk of pending references 
before us,
// we must wait for ReferenceHandler thread to enqueue them before 
continuing...

// ... now see what is enqueued...

> I’m okay for only system's cleaner thread to help the reference handler thread doing its job.  Would you consider having the special cleaner thread to help the enqueuing before waiting on the cleaner's ReferenceQueue?

As I explained to Kim, the trick is not as much about helping than it is 
about synchronizing with ReferenceHandler thread. The allocating thread 
must know when all References discovered up to a certain point in time 
are processed before giving up with OOME. If it helps processing or not 
is of 2nd importance. It can help if that improves throughput but needs not.

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

So let me think about these things for a little more. I'll try to 
address above concerns.

Regards, Peter





More information about the core-libs-dev mailing list