RFR (XS): 8014890 : Reference queues may return more entries than expected (original) (raw)
Mandy Chung mandy.chung at oracle.com
Wed Jun 19 22:49:06 UTC 2013
- Previous message: RFR (XS): 8014890 : Reference queues may return more entries than expected
- Next message: hg: jdk8/tl/jdk: 8016446: Improve forEach/replaceAll for Map, HashMap, Hashtable, IdentityHashMap, WeakHashMap, TreeMap, ConcurrentMap
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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
- Previous message: RFR (XS): 8014890 : Reference queues may return more entries than expected
- Next message: hg: jdk8/tl/jdk: 8016446: Improve forEach/replaceAll for Map, HashMap, Hashtable, IdentityHashMap, WeakHashMap, TreeMap, ConcurrentMap
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]