<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hi Kim,<br>
<br>
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.<br>
<br>
I like to keep the existing indentation of the description of
different states (line 50-75) that makes them easy to read.<br>
<br>
"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.<br>
<span class="changed"><br>
86 * FinalReference (which exists to support finalization, which
was</span><span class="changed"><br>
87 * deprecated in JDK 9) differs from normal references</span><br>
<span class="changed"><br>
I suggest to drop "which was deprecated in JDK 9" since
@Deprecated is in the javadoc and no implication to the
implementation.<br>
<br>
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.<br>
</span>
<pre style="background-color:#ffffff;color:#000000;font-family:'Menlo';font-size:9.0pt;"><span style="color:#808080;font-style:italic;">* Transitions:
</span><span style="color:#808080;font-style:italic;">* clear
</span><span style="color:#808080;font-style:italic;">* active/registered ------> inactive/registered
</span><span style="color:#808080;font-style:italic;">* | | enqueue [2]
</span><span style="color:#808080;font-style:italic;">* | GC enqueue [2] v
</span><span style="color:#808080;font-style:italic;">* | ------> inactive/enqueued
</span><span style="color:#808080;font-style:italic;">* v
</span><span style="color:#808080;font-style:italic;">* pending/registered ---
</span><span style="color:#808080;font-style:italic;">* | |
</span><span style="color:#808080;font-style:italic;">* | enqueue [2] |-> inactive/enqueued (pending list processing)
</span><span style="color:#808080;font-style:italic;">* v | |
</span><span style="color:#808080;font-style:italic;">* pending/enqueued --| |
</span><span style="color:#808080;font-style:italic;">* | | poll/remove
</span><span style="color:#808080;font-style:italic;">* | poll/remove |
</span><span style="color:#808080;font-style:italic;">* v v
</span><span style="color:#808080;font-style:italic;">* pending/dequeued -----> inactive/dequeued
</span><span style="color:#808080;font-style:italic;">*
</span><span style="color:#808080;font-style:italic;">* clear/enqueue
</span><span style="color:#808080;font-style:italic;">* active/unregistered -----> inactive/unregistered
</span><span style="color:#808080;font-style:italic;">* |
</span><span style="color:#808080;font-style:italic;">* | GC
</span><span style="color:#808080;font-style:italic;">* v
</span><span style="color:#808080;font-style:italic;">* pending/unregistered -> inactive/unregistered - pending list processing</span></pre>
<span class="changed"></span><span class="changed"><br>
114 * -> inactive/unregistered - GC, clear, enqueue</span><span
class="changed"></span><span class="changed"><br>
<br>
Under what circumstance does GC make a reference from
active/unregistered -> inactive/unregistered?<br>
<br>
</span>Mandy<br>
<br>
<div class="moz-cite-prefix">On 5/20/18 8:12 AM, Kim Barrett wrote:<br>
</div>
<blockquote type="cite"
cite="mid:01A04562-547C-4CC3-B0B5-88C8770DFD70@oracle.com">
<blockquote type="cite">
<pre wrap="">On May 15, 2018, at 12:09 AM, Kim Barrett <a class="moz-txt-link-rfc2396E" href="mailto:kim.barrett@oracle.com"><kim.barrett@oracle.com></a> wrote:
New webrev, rebased to include JDK-8201491:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~kbarrett/8203028/open.01/">http://cr.openjdk.java.net/~kbarrett/8203028/open.01/</a>
New webrevs with above changes:
full: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~kbarrett/8203028/open.02/">http://cr.openjdk.java.net/~kbarrett/8203028/open.02/</a>
incr: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~kbarrett/8203028/open.02.inc/">http://cr.openjdk.java.net/~kbarrett/8203028/open.02.inc/</a>
</pre>
</blockquote>
<pre wrap="">
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.
</pre>
</blockquote>
<br>
</body>
</html>