> On May 23, 2018, at 12:09 AM, mandy chung <mandy.ch...@oracle.com> wrote: > > 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.
Thanks for looking at this. > I like to keep the existing indentation of the description of different > states (line 50-75) that makes them easy to read. Done > "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. I’ve eliminated the use of “normal”. > 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. Done. > 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. I've adopted your suggestion of ASCII-art to describe the state transitions. I've fixed a couple of errors and made some other changes to (I hope) improve readability. I've also made some naming changes for consistency and clarity, including: - "pending list" => "pending-Reference list" (both were being used) - "pending list process(or|ing)" => ReferenceHandler [thread] > 114 * -> inactive/unregistered - GC, clear, enqueue > > Under what circumstance does GC make a reference from active/unregistered -> > inactive/unregistered? The GC may bypass the [active/unregistered] -> [pending/unregistered] transition, as an optimization. New webrevs (only comment updates in Reference.java): full: http://cr.openjdk.java.net/~kbarrett/8203028/open.04/ incr: http://cr.openjdk.java.net/~kbarrett/8203028/open.04.inc/
