8203028: Simplify reference processing in light of JDK-8175797
authorkbarrett
Sat, 26 May 2018 03:11:50 -0400
changeset 50277 f84ae8aa5d88
parent 50276 6a5a8ed5e475
child 50278 a8e77041c69f
8203028: Simplify reference processing in light of JDK-8175797 Summary: Removed special handling of Reference.next Reviewed-by: tschatzl, sjohanss, mchung
src/hotspot/share/gc/parallel/psCompactionManager.cpp
src/hotspot/share/gc/parallel/psParallelCompact.cpp
src/hotspot/share/gc/parallel/psPromotionManager.cpp
src/hotspot/share/gc/shared/referenceProcessor.cpp
src/hotspot/share/gc/shared/referenceProcessor.hpp
src/hotspot/share/oops/instanceRefKlass.cpp
src/hotspot/share/oops/instanceRefKlass.inline.hpp
src/java.base/share/classes/java/lang/ref/Reference.java
src/java.base/share/classes/java/lang/ref/ReferenceQueue.java
--- a/src/hotspot/share/gc/parallel/psCompactionManager.cpp	Fri May 25 14:10:21 2018 -0700
+++ b/src/hotspot/share/gc/parallel/psCompactionManager.cpp	Sat May 26 03:11:50 2018 -0400
@@ -200,16 +200,9 @@
       cm->mark_and_push(referent_addr);
     }
   }
-  T* next_addr = (T*)java_lang_ref_Reference::next_addr_raw(obj);
-  // Treat discovered as normal oop, if ref is not "active",
-  // i.e. if next is non-NULL.
-  T  next_oop = RawAccess<>::oop_load(next_addr);
-  if (!CompressedOops::is_null(next_oop)) { // i.e. ref is not "active"
-    T* discovered_addr = (T*)java_lang_ref_Reference::discovered_addr_raw(obj);
-    log_develop_trace(gc, ref)("   Process discovered as normal " PTR_FORMAT, p2i(discovered_addr));
-    cm->mark_and_push(discovered_addr);
-  }
-  cm->mark_and_push(next_addr);
+  // Treat discovered as normal oop.
+  T* discovered_addr = (T*)java_lang_ref_Reference::discovered_addr_raw(obj);
+  cm->mark_and_push(discovered_addr);
   klass->InstanceKlass::oop_pc_follow_contents(obj, cm);
 }
 
--- a/src/hotspot/share/gc/parallel/psParallelCompact.cpp	Fri May 25 14:10:21 2018 -0700
+++ b/src/hotspot/share/gc/parallel/psParallelCompact.cpp	Sat May 26 03:11:50 2018 -0400
@@ -3081,13 +3081,10 @@
 #ifdef ASSERT
 template <class T> static void trace_reference_gc(const char *s, oop obj,
                                                   T* referent_addr,
-                                                  T* next_addr,
                                                   T* discovered_addr) {
   log_develop_trace(gc, ref)("%s obj " PTR_FORMAT, s, p2i(obj));
   log_develop_trace(gc, ref)("     referent_addr/* " PTR_FORMAT " / " PTR_FORMAT,
                              p2i(referent_addr), referent_addr ? p2i((oop)RawAccess<>::oop_load(referent_addr)) : NULL);
-  log_develop_trace(gc, ref)("     next_addr/* " PTR_FORMAT " / " PTR_FORMAT,
-                             p2i(next_addr), next_addr ? p2i((oop)RawAccess<>::oop_load(next_addr)) : NULL);
   log_develop_trace(gc, ref)("     discovered_addr/* " PTR_FORMAT " / " PTR_FORMAT,
                              p2i(discovered_addr), discovered_addr ? p2i((oop)RawAccess<>::oop_load(discovered_addr)) : NULL);
 }
@@ -3097,12 +3094,10 @@
 static void oop_pc_update_pointers_specialized(oop obj, ParCompactionManager* cm) {
   T* referent_addr = (T*)java_lang_ref_Reference::referent_addr_raw(obj);
   PSParallelCompact::adjust_pointer(referent_addr, cm);
-  T* next_addr = (T*)java_lang_ref_Reference::next_addr_raw(obj);
-  PSParallelCompact::adjust_pointer(next_addr, cm);
   T* discovered_addr = (T*)java_lang_ref_Reference::discovered_addr_raw(obj);
   PSParallelCompact::adjust_pointer(discovered_addr, cm);
   debug_only(trace_reference_gc("InstanceRefKlass::oop_update_ptrs", obj,
-                                referent_addr, next_addr, discovered_addr);)
+                                referent_addr, discovered_addr);)
 }
 
 void InstanceRefKlass::oop_pc_update_pointers(oop obj, ParCompactionManager* cm) {
--- a/src/hotspot/share/gc/parallel/psPromotionManager.cpp	Fri May 25 14:10:21 2018 -0700
+++ b/src/hotspot/share/gc/parallel/psPromotionManager.cpp	Sat May 26 03:11:50 2018 -0400
@@ -442,7 +442,7 @@
   if (PSScavenge::should_scavenge(referent_addr)) {
     ReferenceProcessor* rp = PSScavenge::reference_processor();
     if (rp->discover_reference(obj, klass->reference_type())) {
-      // reference already enqueued, referent and next will be traversed later
+      // reference discovered, referent will be traversed later.
       klass->InstanceKlass::oop_ps_push_contents(obj, pm);
       return;
     } else {
@@ -450,20 +450,10 @@
       pm->claim_or_forward_depth(referent_addr);
     }
   }
-  // Treat discovered as normal oop, if ref is not "active",
-  // i.e. if next is non-NULL.
-  T* next_addr = (T*)java_lang_ref_Reference::next_addr_raw(obj);
-  T  next_oop = RawAccess<>::oop_load(next_addr);
-  if (!CompressedOops::is_null(next_oop)) { // i.e. ref is not "active"
-    T* discovered_addr = (T*)java_lang_ref_Reference::discovered_addr_raw(obj);
-    log_develop_trace(gc, ref)("   Process discovered as normal " PTR_FORMAT, p2i(discovered_addr));
-    if (PSScavenge::should_scavenge(discovered_addr)) {
-      pm->claim_or_forward_depth(discovered_addr);
-    }
-  }
-  // Treat next as normal oop;  next is a link in the reference queue.
-  if (PSScavenge::should_scavenge(next_addr)) {
-    pm->claim_or_forward_depth(next_addr);
+  // Treat discovered as normal oop
+  T* discovered_addr = (T*)java_lang_ref_Reference::discovered_addr_raw(obj);
+  if (PSScavenge::should_scavenge(discovered_addr)) {
+    pm->claim_or_forward_depth(discovered_addr);
   }
   klass->InstanceKlass::oop_ps_push_contents(obj, pm);
 }
--- a/src/hotspot/share/gc/shared/referenceProcessor.cpp	Fri May 25 14:10:21 2018 -0700
+++ b/src/hotspot/share/gc/shared/referenceProcessor.cpp	Sat May 26 03:11:50 2018 -0400
@@ -303,9 +303,6 @@
 }
 
 void DiscoveredListIterator::enqueue() {
-  // Self-loop next, so as to make Ref not active.
-  java_lang_ref_Reference::set_next_raw(_current_discovered, _current_discovered);
-
   HeapAccess<AS_NO_KEEPALIVE>::oop_store_at(_current_discovered,
                                             java_lang_ref_Reference::discovered_offset,
                                             _next_discovered);
@@ -364,38 +361,35 @@
                              iter.removed(), iter.processed(), p2i(&refs_list));
 }
 
+inline void log_dropped_ref(const DiscoveredListIterator& iter, const char* reason) {
+  log_develop_trace(gc, ref)("Dropping %s reference " PTR_FORMAT ": %s",
+                             reason, p2i(iter.obj()),
+                             iter.obj()->klass()->internal_name());
+}
+
+// Traverse the list and remove any Refs whose referents are alive,
+// or NULL if discovery is not atomic.
 void ReferenceProcessor::process_phase2(DiscoveredList&    refs_list,
                                         BoolObjectClosure* is_alive,
                                         OopClosure*        keep_alive,
                                         VoidClosure*       complete_gc) {
-  if (discovery_is_atomic()) {
-    // complete_gc is ignored in this case for this phase
-    pp2_work(refs_list, is_alive, keep_alive);
-  } else {
-    assert(complete_gc != NULL, "Error");
-    pp2_work_concurrent_discovery(refs_list, is_alive,
-                                  keep_alive, complete_gc);
-  }
-}
-// Traverse the list and remove any Refs that are not active, or
-// whose referents are either alive or NULL.
-void
-ReferenceProcessor::pp2_work(DiscoveredList&    refs_list,
-                             BoolObjectClosure* is_alive,
-                             OopClosure*        keep_alive) {
-  assert(discovery_is_atomic(), "Error");
+  // complete_gc is unused.
   DiscoveredListIterator iter(refs_list, keep_alive, is_alive);
   while (iter.has_next()) {
-    iter.load_ptrs(DEBUG_ONLY(false /* allow_null_referent */));
-    DEBUG_ONLY(oop next = java_lang_ref_Reference::next(iter.obj());)
-    assert(next == NULL, "Should not discover inactive Reference");
-    if (iter.is_referent_alive()) {
-      log_develop_trace(gc, ref)("Dropping strongly reachable reference (" INTPTR_FORMAT ": %s)",
-                                 p2i(iter.obj()), iter.obj()->klass()->internal_name());
+    iter.load_ptrs(DEBUG_ONLY(!discovery_is_atomic() /* allow_null_referent */));
+    if (iter.referent() == NULL) {
+      // Reference has been cleared since discovery; only possible if
+      // discovery is not atomic (checked by load_ptrs).  Remove
+      // reference from list.
+      log_dropped_ref(iter, "cleared");
+      iter.remove();
+      iter.move_to_next();
+    } else if (iter.is_referent_alive()) {
       // The referent is reachable after all.
-      // Remove Reference object from list.
+      // Remove reference from list.
+      log_dropped_ref(iter, "reachable");
       iter.remove();
-      // Update the referent pointer as necessary: Note that this
+      // Update the referent pointer as necessary.  Note that this
       // should not entail any recursive marking because the
       // referent must already have been traversed.
       iter.make_referent_alive();
@@ -413,45 +407,6 @@
   )
 }
 
-void
-ReferenceProcessor::pp2_work_concurrent_discovery(DiscoveredList&    refs_list,
-                                                  BoolObjectClosure* is_alive,
-                                                  OopClosure*        keep_alive,
-                                                  VoidClosure*       complete_gc) {
-  assert(!discovery_is_atomic(), "Error");
-  DiscoveredListIterator iter(refs_list, keep_alive, is_alive);
-  while (iter.has_next()) {
-    iter.load_ptrs(DEBUG_ONLY(true /* allow_null_referent */));
-    HeapWord* next_addr = java_lang_ref_Reference::next_addr_raw(iter.obj());
-    oop next = java_lang_ref_Reference::next(iter.obj());
-    if ((iter.referent() == NULL || iter.is_referent_alive() ||
-         next != NULL)) {
-      assert(oopDesc::is_oop_or_null(next), "Expected an oop or NULL for next field at " PTR_FORMAT, p2i(next));
-      // Remove Reference object from list
-      iter.remove();
-      // Trace the cohorts
-      iter.make_referent_alive();
-      if (UseCompressedOops) {
-        keep_alive->do_oop((narrowOop*)next_addr);
-      } else {
-        keep_alive->do_oop((oop*)next_addr);
-      }
-      iter.move_to_next();
-    } else {
-      iter.next();
-    }
-  }
-  // Now close the newly reachable set
-  complete_gc->do_void();
-  NOT_PRODUCT(
-    if (iter.processed() > 0) {
-      log_develop_trace(gc, ref)(" Dropped " SIZE_FORMAT " active Refs out of " SIZE_FORMAT
-        " Refs in discovered list " INTPTR_FORMAT,
-        iter.removed(), iter.processed(), p2i(&refs_list));
-    }
-  )
-}
-
 void ReferenceProcessor::process_phase3(DiscoveredList&    refs_list,
                                         bool               clear_referent,
                                         BoolObjectClosure* is_alive,
@@ -465,8 +420,12 @@
       // NULL out referent pointer
       iter.clear_referent();
     } else {
-      // keep the referent around
+      // Current reference is a FinalReference; that's the only kind we
+      // don't clear the referent, instead keeping it for calling finalize.
       iter.make_referent_alive();
+      // Self-loop next, to mark it not active.
+      assert(java_lang_ref_Reference::next(iter.obj()) == NULL, "enqueued FinalReference");
+      java_lang_ref_Reference::set_next_raw(iter.obj(), iter.obj());
     }
     iter.enqueue();
     log_develop_trace(gc, ref)("Adding %sreference (" INTPTR_FORMAT ": %s) as pending",
@@ -913,9 +872,9 @@
   if (!_discovering_refs || !RegisterReferences) {
     return false;
   }
-  // We only discover active references.
-  oop next = java_lang_ref_Reference::next(obj);
-  if (next != NULL) {   // Ref is no longer active
+
+  if ((rt == REF_FINAL) && (java_lang_ref_Reference::next(obj) != NULL)) {
+    // Don't rediscover non-active FinalReferences.
     return false;
   }
 
@@ -1121,24 +1080,15 @@
       return true;
     }
     iter.load_ptrs(DEBUG_ONLY(true /* allow_null_referent */));
-    oop obj = iter.obj();
-    oop next = java_lang_ref_Reference::next(obj);
-    if (iter.referent() == NULL || iter.is_referent_alive() || next != NULL) {
-      // The referent has been cleared, or is alive, or the Reference is not
-      // active; we need to trace and mark its cohort.
+    if (iter.referent() == NULL || iter.is_referent_alive()) {
+      // The referent has been cleared, or is alive; we need to trace
+      // and mark its cohort.
       log_develop_trace(gc, ref)("Precleaning Reference (" INTPTR_FORMAT ": %s)",
                                  p2i(iter.obj()), iter.obj()->klass()->internal_name());
       // Remove Reference object from list
       iter.remove();
       // Keep alive its cohort.
       iter.make_referent_alive();
-      if (UseCompressedOops) {
-        narrowOop* next_addr = (narrowOop*)java_lang_ref_Reference::next_addr_raw(obj);
-        keep_alive->do_oop(next_addr);
-      } else {
-        oop* next_addr = (oop*)java_lang_ref_Reference::next_addr_raw(obj);
-        keep_alive->do_oop(next_addr);
-      }
       iter.move_to_next();
     } else {
       iter.next();
--- a/src/hotspot/share/gc/shared/referenceProcessor.hpp	Fri May 25 14:10:21 2018 -0700
+++ b/src/hotspot/share/gc/shared/referenceProcessor.hpp	Sat May 26 03:11:50 2018 -0400
@@ -262,15 +262,6 @@
                       BoolObjectClosure* is_alive,
                       OopClosure*        keep_alive,
                       VoidClosure*       complete_gc);
-  // Work methods in support of process_phase2
-  void pp2_work(DiscoveredList&    refs_list,
-                BoolObjectClosure* is_alive,
-                OopClosure*        keep_alive);
-  void pp2_work_concurrent_discovery(
-                DiscoveredList&    refs_list,
-                BoolObjectClosure* is_alive,
-                OopClosure*        keep_alive,
-                VoidClosure*       complete_gc);
   // Phase3: process the referents by either clearing them
   // or keeping them alive (and their closure), and enqueuing them.
   void process_phase3(DiscoveredList&    refs_list,
--- a/src/hotspot/share/oops/instanceRefKlass.cpp	Fri May 25 14:10:21 2018 -0700
+++ b/src/hotspot/share/oops/instanceRefKlass.cpp	Sat May 26 03:11:50 2018 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -30,10 +30,8 @@
 
 void InstanceRefKlass::update_nonstatic_oop_maps(Klass* k) {
   // Clear the nonstatic oop-map entries corresponding to referent
-  // and nextPending field.  They are treated specially by the
+  // and discovered fields.  They are treated specially by the
   // garbage collector.
-  // The discovered field is used only by the garbage collector
-  // and is also treated specially.
   InstanceKlass* ik = InstanceKlass::cast(k);
 
   // Check that we have the right class
@@ -45,22 +43,33 @@
 
   OopMapBlock* map = ik->start_of_nonstatic_oop_maps();
 
-  // Check that the current map is (2,4) - currently points at field with
-  // offset 2 (words) and has 4 map entries.
-  debug_only(int offset = java_lang_ref_Reference::referent_offset);
-  debug_only(unsigned int count = ((java_lang_ref_Reference::discovered_offset -
-    java_lang_ref_Reference::referent_offset)/heapOopSize) + 1);
+#ifdef ASSERT
+  // Verify fields are in the expected places.
+  int referent_offset = java_lang_ref_Reference::referent_offset;
+  int queue_offset = java_lang_ref_Reference::queue_offset;
+  int next_offset = java_lang_ref_Reference::next_offset;
+  int discovered_offset = java_lang_ref_Reference::discovered_offset;
+  assert(referent_offset < queue_offset, "just checking");
+  assert(queue_offset < next_offset, "just checking");
+  assert(next_offset < discovered_offset, "just checking");
+  const unsigned int count =
+    1 + ((discovered_offset - referent_offset) / heapOopSize);
+  assert(count == 4, "just checking");
+#endif // ASSERT
 
+  // Updated map starts at "queue", covers "queue" and "next".
+  const int new_offset = java_lang_ref_Reference::queue_offset;
+  const unsigned int new_count = 2; // queue and next
+
+  // Verify existing map is as expected, and update if needed.
   if (UseSharedSpaces) {
-    assert(map->offset() == java_lang_ref_Reference::queue_offset &&
-           map->count() == 1, "just checking");
+    assert(map->offset() == new_offset, "just checking");
+    assert(map->count() == new_count, "just checking");
   } else {
-    assert(map->offset() == offset && map->count() == count,
-           "just checking");
-
-    // Update map to (3,1) - point to offset of 3 (words) with 1 map entry.
-    map->set_offset(java_lang_ref_Reference::queue_offset);
-    map->set_count(1);
+    assert(map->offset() == referent_offset, "just checking");
+    assert(map->count() == count, "just checking");
+    map->set_offset(new_offset);
+    map->set_count(new_count);
   }
 }
 
@@ -74,7 +83,7 @@
   if (referent != NULL) {
     guarantee(oopDesc::is_oop(referent), "referent field heap failed");
   }
-  // Verify next field
+  // Additional verification for next field, which must be a Reference or null
   oop next = java_lang_ref_Reference::next(obj);
   if (next != NULL) {
     guarantee(oopDesc::is_oop(next), "next field should be an oop");
--- a/src/hotspot/share/oops/instanceRefKlass.inline.hpp	Fri May 25 14:10:21 2018 -0700
+++ b/src/hotspot/share/oops/instanceRefKlass.inline.hpp	Sat May 26 03:11:50 2018 -0400
@@ -46,14 +46,6 @@
 }
 
 template <bool nv, typename T, class OopClosureType, class Contains>
-void InstanceRefKlass::do_next(oop obj, OopClosureType* closure, Contains& contains) {
-  T* next_addr = (T*)java_lang_ref_Reference::next_addr_raw(obj);
-  if (contains(next_addr)) {
-    Devirtualizer<nv>::do_oop(closure, next_addr);
-  }
-}
-
-template <bool nv, typename T, class OopClosureType, class Contains>
 void InstanceRefKlass::do_discovered(oop obj, OopClosureType* closure, Contains& contains) {
   T* discovered_addr = (T*)java_lang_ref_Reference::discovered_addr_raw(obj);
   if (contains(discovered_addr)) {
@@ -84,24 +76,15 @@
     return;
   }
 
-  // Treat referent as normal oop.
+  // Treat referent and discovered as normal oops.
   do_referent<nv, T>(obj, closure, contains);
-
-  // Treat discovered as normal oop, if ref is not "active" (next non-NULL).
-  T next_oop  = RawAccess<>::oop_load((T*)java_lang_ref_Reference::next_addr_raw(obj));
-  if (!CompressedOops::is_null(next_oop)) {
-    do_discovered<nv, T>(obj, closure, contains);
-  }
-
-  // Treat next as normal oop.
-  do_next<nv, T>(obj, closure, contains);
+  do_discovered<nv, T>(obj, closure, contains);
 }
 
 template <bool nv, typename T, class OopClosureType, class Contains>
 void InstanceRefKlass::oop_oop_iterate_fields(oop obj, OopClosureType* closure, Contains& contains) {
   do_referent<nv, T>(obj, closure, contains);
   do_discovered<nv, T>(obj, closure, contains);
-  do_next<nv, T>(obj, closure, contains);
 }
 
 template <bool nv, typename T, class OopClosureType, class Contains>
@@ -192,14 +175,11 @@
 template <typename T>
 void InstanceRefKlass::trace_reference_gc(const char *s, oop obj) {
   T* referent_addr   = (T*) java_lang_ref_Reference::referent_addr_raw(obj);
-  T* next_addr       = (T*) java_lang_ref_Reference::next_addr_raw(obj);
   T* discovered_addr = (T*) java_lang_ref_Reference::discovered_addr_raw(obj);
 
   log_develop_trace(gc, ref)("InstanceRefKlass %s for obj " PTR_FORMAT, s, p2i(obj));
   log_develop_trace(gc, ref)("     referent_addr/* " PTR_FORMAT " / " PTR_FORMAT,
       p2i(referent_addr), p2i(referent_addr ? RawAccess<>::oop_load(referent_addr) : (oop)NULL));
-  log_develop_trace(gc, ref)("     next_addr/* " PTR_FORMAT " / " PTR_FORMAT,
-      p2i(next_addr), p2i(next_addr ? RawAccess<>::oop_load(next_addr) : (oop)NULL));
   log_develop_trace(gc, ref)("     discovered_addr/* " PTR_FORMAT " / " PTR_FORMAT,
       p2i(discovered_addr), p2i(discovered_addr ? RawAccess<>::oop_load(discovered_addr) : (oop)NULL));
 }
--- a/src/java.base/share/classes/java/lang/ref/Reference.java	Fri May 25 14:10:21 2018 -0700
+++ b/src/java.base/share/classes/java/lang/ref/Reference.java	Sat May 26 03:11:50 2018 -0400
@@ -43,71 +43,146 @@
 
 public abstract class Reference<T> {
 
-    /* A Reference instance is in one of four possible internal states:
+    /* The state of a Reference object is characterized by two attributes.  It
+     * may be either "active", "pending", or "inactive".  It may also be
+     * either "registered", "enqueued", "dequeued", or "unregistered".
+     *
+     *   Active: Subject to special treatment by the garbage collector.  Some
+     *   time after the collector detects that the reachability of the
+     *   referent has changed to the appropriate state, the collector
+     *   "notifies" the reference, changing the state to either "pending" or
+     *   "inactive".
+     *   referent != null; discovered = null, or in GC discovered list.
      *
-     *     Active: Subject to special treatment by the garbage collector.  Some
-     *     time after the collector detects that the reachability of the
-     *     referent has changed to the appropriate state, it changes the
-     *     instance's state to either Pending or Inactive, depending upon
-     *     whether or not the instance was registered with a queue when it was
-     *     created.  In the former case it also adds the instance to the
-     *     pending-Reference list.  Newly-created instances are Active.
+     *   Pending: An element of the pending-Reference list, waiting to be
+     *   processed by the ReferenceHandler thread.  The pending-Reference
+     *   list is linked through the discovered fields of references in the
+     *   list.
+     *   referent = null; discovered = next element in pending-Reference list.
+     *
+     *   Inactive: Neither Active nor Pending.
+     *   referent = null.
+     *
+     *   Registered: Associated with a queue when created, and not yet added
+     *   to the queue.
+     *   queue = the associated queue.
      *
-     *     Pending: An element of the pending-Reference list, waiting to be
-     *     enqueued by the Reference-handler thread.  Unregistered instances
-     *     are never in this state.
+     *   Enqueued: Added to the associated queue, and not yet removed.
+     *   queue = ReferenceQueue.ENQUEUE; next = next entry in list, or this to
+     *   indicate end of list.
+     *
+     *   Dequeued: Added to the associated queue and then removed.
+     *   queue = ReferenceQueue.NULL; next = this.
+     *
+     *   Unregistered: Not associated with a queue when created.
+     *   queue = ReferenceQueue.NULL.
      *
-     *     Enqueued: An element of the queue with which the instance was
-     *     registered when it was created.  When an instance is removed from
-     *     its ReferenceQueue, it is made Inactive.  Unregistered instances are
-     *     never in this state.
+     * The collector only needs to examine the referent field and the
+     * discovered field to determine whether a (non-FinalReference) Reference
+     * object needs special treatment.  If the referent is non-null and not
+     * known to be live, then it may need to be discovered for possible later
+     * notification.  But if the discovered field is non-null, then it has
+     * already been discovered.
      *
-     *     Inactive: Nothing more to do.  Once an instance becomes Inactive its
-     *     state will never change again.
+     * FinalReference (which exists to support finalization) differs from
+     * other references, because a FinalReference is not cleared when
+     * notified.  The referent being null or not cannot be used to distinguish
+     * between the active state and pending or inactive states.  However,
+     * FinalReferences do not support enqueue().  Instead, the next field of a
+     * FinalReference object is set to "this" when it is added to the
+     * pending-Reference list.  The use of "this" as the value of next in the
+     * enqueued and dequeued states maintains the non-active state.  An
+     * additional check that the next field is null is required to determine
+     * that a FinalReference object is active.
      *
-     * The state is encoded in the queue and next fields as follows:
-     *
-     *     Active: queue = ReferenceQueue with which instance is registered, or
-     *     ReferenceQueue.NULL if it was not registered with a queue; next =
-     *     null.
+     * Initial states:
+     *   [active/registered]
+     *   [active/unregistered] [1]
      *
-     *     Pending: queue = ReferenceQueue with which instance is registered;
-     *     next = this
+     * Transitions:
+     *                            clear
+     *   [active/registered]     ------->   [inactive/registered]
+     *          |                                 |
+     *          |                                 | enqueue [2]
+     *          | GC              enqueue [2]     |
+     *          |                -----------------|
+     *          |                                 |
+     *          v                                 |
+     *   [pending/registered]    ---              v
+     *          |                   | ReferenceHandler
+     *          | enqueue [2]       |--->   [inactive/enqueued]
+     *          v                   |             |
+     *   [pending/enqueued]      ---              |
+     *          |                                 | poll/remove
+     *          | poll/remove                     |
+     *          |                                 |
+     *          v            ReferenceHandler     v
+     *   [pending/dequeued]      ------>    [inactive/dequeued]
      *
-     *     Enqueued: queue = ReferenceQueue.ENQUEUED; next = Following instance
-     *     in queue, or this if at end of list.
      *
-     *     Inactive: queue = ReferenceQueue.NULL; next = this.
+     *                           clear/enqueue/GC [3]
+     *   [active/unregistered]   ------
+     *          |                      |
+     *          | GC                   |
+     *          |                      |--> [inactive/unregistered]
+     *          v                      |
+     *   [pending/unregistered]  ------
+     *                           ReferenceHandler
+     *
+     * Terminal states:
+     *   [inactive/dequeued]
+     *   [inactive/unregistered]
      *
-     * With this scheme the collector need only examine the next field in order
-     * to determine whether a Reference instance requires special treatment: If
-     * the next field is null then the instance is active; if it is non-null,
-     * then the collector should treat the instance normally.
+     * Unreachable states (because enqueue also clears):
+     *   [active/enqeued]
+     *   [active/dequeued]
+     *
+     * [1] Unregistered is not permitted for FinalReferences.
      *
-     * To ensure that a concurrent collector can discover active Reference
-     * objects without interfering with application threads that may apply
-     * the enqueue() method to those objects, collectors should link
-     * discovered objects through the discovered field. The discovered
-     * field is also used for linking Reference objects in the pending list.
+     * [2] These transitions are not possible for FinalReferences, making
+     * [pending/enqueued] and [pending/dequeued] unreachable, and
+     * [inactive/registered] terminal.
+     *
+     * [3] The garbage collector may directly transition a Reference
+     * from [active/unregistered] to [inactive/unregistered],
+     * bypassing the pending-Reference list.
      */
 
     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;
 
-    /* When active:   NULL
-     *     pending:   this
-     *    Enqueued:   next reference in queue (or this if last)
-     *    Inactive:   this
+    /* The link in a ReferenceQueue's list of Reference objects.
+     *
+     * When registered: null
+     *        enqueued: next element in queue (or this if last)
+     *        dequeued: this (marking FinalReferences as inactive)
+     *    unregistered: null
      */
     @SuppressWarnings("rawtypes")
     volatile Reference next;
 
-    /* When active:   next element in a discovered reference list maintained by GC (or this if last)
-     *     pending:   next element in the pending list (or null if last)
-     *   otherwise:   NULL
+    /* Used by the garbage collector to accumulate Reference objects that need
+     * to be revisited in order to decide whether they should be notified.
+     * Also used as the link in the pending-Reference list.  The discovered
+     * field and the next field are distinct to allow the enqueue() method to
+     * be applied to a Reference object while it is either in the
+     * pending-Reference list or in the garbage collector's discovered set.
+     *
+     * When active: null or next element in a discovered reference list
+     *              maintained by the GC (or this if last)
+     *     pending: next element in the pending-Reference list (null if last)
+     *    inactive: null
      */
-    private transient Reference<T> discovered;  /* used by VM */
+    private transient Reference<T> discovered;
 
 
     /* High-priority thread to enqueue pending References
@@ -141,17 +216,17 @@
     }
 
     /*
-     * Atomically get and clear (set to null) the VM's pending list.
+     * Atomically get and clear (set to null) the VM's pending-Reference list.
      */
     private static native Reference<Object> getAndClearReferencePendingList();
 
     /*
-     * Test whether the VM's pending list contains any entries.
+     * Test whether the VM's pending-Reference list contains any entries.
      */
     private static native boolean hasReferencePendingList();
 
     /*
-     * Wait until the VM's pending list may be non-null.
+     * Wait until the VM's pending-Reference list may be non-null.
      */
     private static native void waitForReferencePendingList();
 
--- a/src/java.base/share/classes/java/lang/ref/ReferenceQueue.java	Fri May 25 14:10:21 2018 -0700
+++ b/src/java.base/share/classes/java/lang/ref/ReferenceQueue.java	Sat May 26 03:11:50 2018 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -66,6 +66,7 @@
                 return false;
             }
             assert queue == this;
+            // Self-loop end, so if a FinalReference it remains inactive.
             r.next = (head == null) ? r : head;
             head = r;
             queueLength++;
@@ -90,7 +91,10 @@
             // poll().  Volatiles ensure ordering.
             @SuppressWarnings("unchecked")
             Reference<? extends T> rn = r.next;
+            // Handle self-looped next as end of list designator.
             head = (rn == r) ? null : rn;
+            // Self-loop next rather than setting to null, so if a
+            // FinalReference it remains inactive.
             r.next = r;
             queueLength--;
             if (r instanceof FinalReference) {