On 6/19/13 2:05 AM, Peter Levart wrote:
It doesn't make a difference between Reference.isEnqueued() and
ReferenceQueue.poll(), since there isn't any synchronization between
the two. So isEnqueued() can still be returning true while another
thread has already de-queued the instance. I guess the real use of
isEnqueued() method is reliable detection of false -> true transition
only.
I think the isEnqueued method is useful to determine when a
referent has been GC'ed. If there are threads that remove them
from the queue, the code calling isEnqueued should prepare for
the race when isEnqueued returning true, the reference may
have been dequeued shortly. That seems to be acceptable to
return true when another thread has already dequeued it while
not costing much performance tradeoff.
On the other hand, I agree that it'd be nice if we can clean up
the synchronization logic.
I can't see why the isEnqueued() method checks for both (next != null
&& queue == ENQUEUED). Wouldn't the check for (queue == ENQUEUED) be
enough? In that case the Reference.queue field could be made volatile
and synchronization on Reference instance removed.
I also thought about that. The uncertainty I have is any performance
implication that we need to concern about. Thomas - can you also find
out from the GC team?
It looks to me that next != null is an optimization since next == null
if not pending to be enqueued or not enqueued.
While you're at it, the reallyPoll() could optimize the double
volatile read from head and only perform it once:
private Reference<? extends T> reallyPoll() { /* Must hold
lock */
Reference<? extends T> r = head;
if (r != null) {
head = (r.next == r) ? null : r.next;
r.queue = NULL;
r.next = r;
queueLength--;
if (r instanceof FinalReference) {
sun.misc.VM.addFinalRefCount(-1);
}
return r;
}
return null;
}
Good catch.
Mandy