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
