Hi Aleksey,
This one is even better:
http://cr.openjdk.java.net/~plevart/jdk8-tl/Cleaners/webrev.02/
Instead of hooking up on the ReferenceQueue.poll/remove methods, there
are two new methods in ReferenceQueue: drain/drainLoop - only the 1st
one helps in enqueue-ing while holding the queue's lock. This guarantees
that no pending references are left behind - well only one single
Reference can be skipped - the one in-transit in the ReferenceHandler
thread. The other, drainLoop, is used in CleanerHandler thread to
process Cleaners in background. This one does not do any helping. The
only reason for using:
cleanersQueue.drainLoop(c -> ((Cleaner)c).clean());
instead of external looping:
for (;;) {
Cleaner c = (Cleaner) cleanersQueue.remove();
c.clean();
}
is that with drainLoop, the queue's lock is not relinquished between
de-queue-ing the Cleaner and invoking it's clean() method. This
prevents another Cleaner to be left behind when drain() is called.
With this variant, using no Thread.sleep(), I can not make
DirectBufferTest to fail, no matter how many allocating threads I start
(1, 2, 4, 8, 16, ..., 256).
Regards, Peter
On 10/04/2013 09:54 AM, Peter Levart wrote:
Hi Aleksey,
I played with reference handling code and got the following idea:
Instead of iterating over the set of active Cleaners looking for those
that were cleared by the VM, make ReferenceQueue.poll/remove help
ReferenceHandler thread in enqueue-ing the references. This assumes VM
links the References into a discovered list at the same time as
clearing them. Here's a prototype of this approach:
http://cr.openjdk.java.net/~plevart/jdk8-tl/Cleaners/webrev.01/
It is maybe to aggressive to hook helping enqueue references on the
public ReferenceQueue.poll/remove methods which affects other code
too, but that could be changed (use package-private API between
ReferenceQueue and Cleaner). With this variant, I was not able to fail
the DirectBufferTest on my machine (4 cores i7) with 1,2,4,8,16,32,64
threads and -XX:MaxDirectMemorySize=100m. It sometimes fails quickly
at 128 threads and sometimes passes 60 seconds without failure.
There's certainly room for improvement. Without the patch it fails
after ~500 iterations as soon as 2 threads are used.
So what do you think of the approach in general? You see, I tried to
avoid Thread.sleep() calls to prove the approach is very predictable
even without them. The help-enqueue-references code is executed out of
ReferenceQueue.poll/remove synchronized blocks, so there is no
guarantee that all pending Cleaners have been processed before
giving-up with OOME. Adding a short Thread.sleep() in the Bits loop:
System.gc();
try {
Thread.sleep(100L);
}
catch (InterruptedException x) {}
cleans = Cleaner.assistCleanup();
Might help. It could even be exponential backoff.
Regards, Peter
On 10/03/2013 02:40 PM, Aleksey Shipilev wrote:
On 10/03/2013 04:32 PM, Paul Sandoz wrote:
Alexsey, what do you observe if you revert back Cleaner to a
PhantomReference and retain QUEUE/CLEANERS but not
assistCleanupSlow?
I observed the minuscule probability (my estimate is <0.1%) we hit the
OOME with the original test. This is literally the very aggressive
fallback strategy.
-Aleksey.
