Hi Kim,
Thanks for doing this work and rewriting the comments. It's great to
see this simplification. It looks good in general. I have a couple of
personal preferences if you are okay to update them.
I like to keep the existing indentation of the description of different
states (line 50-75) that makes them easy to read.
"normal reference" - I suggest to change it to "soft, weak, or phantom
reference". FinalReference is also a Reference object and it's an
implementation class that supports finalization. Making it explicit
would avoid confusion.
86 * FinalReference (which exists to support finalization, which was
87 * deprecated in JDK 9) differs from normal references
I suggest to drop "which was deprecated in JDK 9" since @Deprecated is
in the javadoc and no implication to the implementation.
Thanks for adding the comment of the possible transitions. I was
tempted to make an ascii flow chart. State change due to the API makes
it harder to draw. This is my first snap but it's not yet in a
satisfying state. I think this is clearer than the text description.
What do you think? If we like this, I can clean that up and send you a
revised version.
* Transitions: * clear * active/registered ------> inactive/registered *
| | enqueue [2] * | GC enqueue [2] v * | ------> inactive/enqueued * v *
pending/registered --- * | | * | enqueue [2] |-> inactive/enqueued
(pending list processing) * v | | * pending/enqueued --| | * | |
poll/remove * | poll/remove | * v v * pending/dequeued ----->
inactive/dequeued * * clear/enqueue * active/unregistered ----->
inactive/unregistered * | * | GC * v * pending/unregistered ->
inactive/unregistered - pending list processing
114 * -> inactive/unregistered - GC, clear, enqueue
Under what circumstance does GC make a reference from
active/unregistered -> inactive/unregistered?
Mandy
On 5/20/18 8:12 AM, Kim Barrett wrote:
On May 15, 2018, at 12:09 AM, Kim Barrett <kim.barr...@oracle.com> wrote:
New webrev, rebased to include JDK-8201491:
http://cr.openjdk.java.net/~kbarrett/8203028/open.01/
New webrevs with above changes:
full: http://cr.openjdk.java.net/~kbarrett/8203028/open.02/
incr: http://cr.openjdk.java.net/~kbarrett/8203028/open.02.inc/
Following up on a suggestion from Mark Reinhold in the discussion of JDK-8203327
on the core-libs list, I’ve added the following comment to the Reference.queue
field.
diff -r 98982cddbac0 -r 2429129c4d48
src/java.base/share/classes/java/lang/ref/Reference.java
--- a/src/java.base/share/classes/java/lang/ref/Reference.java Mon May 14
18:01:36 2018 -0400
+++ b/src/java.base/share/classes/java/lang/ref/Reference.java Sun May 20
11:06:48 2018 -0400
@@ -131,6 +131,14 @@
private T referent; /* Treated specially by GC */
+ /* The queue this reference gets enqueued to by GC notification or by
+ * calling enqueue().
+ *
+ * When registered: the queue with which this reference is registered.
+ * enqueued: ReferenceQueue.ENQUEUE
+ * dequeued: ReferenceQueue.NULL
+ * unregistered: ReferenceQueue.NULL
+ */
volatile ReferenceQueue<? super T> queue;
/* The link in a ReferenceQueue's list of Reference objects.
