RFR: 8203028: Simplify reference processing in light of JDK-8175797

[Thomas Schatzl]

Hi Kim,

  some initial comments...

On Sun, 2018-05-13 at 21:32 -0400, Kim Barrett wrote:
> Please review this simplification of reference processing, taking
> advantage of the changes made by JDK-8175797, which made
> Reference.enqueue() ensure the referent was cleared before adding the
> reference to the queue.
> 
> GC scanning of Reference objects no longer treats the next field as
> special; it is just an ordinary object field. It didn't need to be
> treated specially before, but because it was often examined for other
> reasons it was potentially more efficient or convenient to do so.
> That rationale no longer holds, because it is now only involved in
> determining whether FinalReferences are active.
> 
> Various additional simplifications in the core of reference
> processing, eliminating the use of the next field except when dealing
> with FinalReferences.  In particular, most of the phase2 difference
> between STW and concurrent collectors has been eliminated, since we
> no longer need to deal with a concurrently enqueued reference that
> retains its (live) referent.
> 
> Rewrote internal comment describing states for
> java.lang.ref.Reference.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8203028
> 
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8203028/open.00/
> 

- please update to latest jdk/jdk, there are some merge conflicts with
the precleaning for g1 change.

- please remove the unused complete_gc parameter from
ReferenceProcessor::process_phase2.

- in ReferenceProcessor::process_phase2, the comment about the
iter.referent() == NULL path only being possible when discovery is not
atomic should be reinforced with an assert.

- Reference.java, comments:

1) "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."

Could this be more specific than "appropriate state" - wouldn't this be
always "unreachable"?

Also, referent may be NULL in case of mutators clear()'ing (or
enqeue()'ing) the Reference in case of concurrent reference processing.
I would prefer to make this more accurate than misleading.

2) "The collector only needs to examine the referent field and the
discovered field to determine whether a normal 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 either (1)
it has already been discovered, or (2) it is in the pending list."

What is a "normal" Reference object (and what is a not-normal one?)

3) "FinalReference 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 the object when it is added to the pending list, and 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."

- instead of "is set to the object" "is set to "this"" would be better
to avoid the question what "set to the object" means.

- " of a FinalReference object is set to the object when it is added to
the pending list, and the use of this as the value of next in the
enqueued and dequeued states maintains the non-active state."

Please split the sentence between "list" and "and". Also I would prefer
if "this" were quoted to make sure the value "this" is meant and not
referring to something else (I know the entire file does not do that,
so ignore if you think this is a bad idea)


Thanks,
  Thomas