RFR (XL): 8202845: Refactor reference processing for improved parallelism

[Kim Barrett]

> On Jun 6, 2018, at 1:13 PM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> I thought marks_oops_alive is an unnecessary quirk of the collectors
> that found its way into the reference processing interface; its purpose
> is to indicate whether the gc should perform stealing at the end of a
> phase, as the complete closures drain the stacks completely by
> themselves anyway.
> 
> Stealing or not should be part of the complete closure (i.e. the gc,
> and attempted by default by the gc) imho (it "completes" the phase),
> like G1 does.
> 
> I fixed it though.

marks_oops_alive is another piece (really an entirely different
mechanism) of the avoidance of stealing and the termination protocol
when the keep_alive closure won't be marking any previously unmarked
objects, so (maybe) won't be generating any potentially stealable
work.  It seems to me to be a better approach than sometimes not
calling the complete_gc closure (the other mechanism, used by
process_phase2), since it puts the decision on the specific collector
that knows how its keep_alive and complete_gc closures interact.  See
further discussion below.

> New webrevs:
> http://cr.openjdk.java.net/~tschatzl/8202845/webrev.0_to_1 (diff)
> http://cr.openjdk.java.net/~tschatzl/8202845/webrev.1 (full)
> Testing:
> hs-tier1-5,jdk-tier-1-3 (with +/-ParallelRefProcEnabled)
> 
> Thanks for your quick review,
>  Thomas

Sorry this one took longer.

------------------------------------------------------------------------------
src/hotspot/share/gc/shared/referenceProcessor.cpp
 315   if (log_is_enabled(Trace, gc, ref)) {
 324   if (log_is_enabled(Trace, gc, ref)) {

s/log_is_enabled/log_develop_is_enabled/

------------------------------------------------------------------------------
src/hotspot/share/gc/shared/referenceProcessor.cpp
 571     complete_gc.do_void();

I think this is unavoidable (at least for now), but I'm wondering if
this is a performance issue for other than G1-young collections.

In the phases where marks_oops_alive is false (like this one), for the
other collectors (and G1 concurrent and full GC, I think), there won't
be anything to steal. Starting the stealing process (and its
associated termination protocol) is a waste of time in those cases.

ParallelScavange and ParallelCompaction both avoid creating the
stealing tasks if marks_oops_alive is false.  CMS avoids calling
do_work_stealing when marks_oops_alive is false.  ParNew doesn't seem
to look at that bit though.

G1 concurrent and full GCs also don't seem to look at that bit, and so
I think will be negatively affected.  Maybe this is a job for another
RFE.

Some comment revision might be called for though.  It would be nice to
not have to re-discover (no pun intended) all this again.

------------------------------------------------------------------------------
src/hotspot/share/gc/shared/referenceProcessor.cpp 
 457   // Close the reachable set
 458   complete_gc->do_void();

Same issues here for PhantomReferences as earlier for
Soft/Weak/FinalReferences.  And I think the same solution; pay
attention to the marks_oops_alive bit.

------------------------------------------------------------------------------
src/hotspot/share/gc/shared/referenceProcessor.cpp 
 595     : ProcessTask(ref_processor, true /* marks_oops_alive */, phase_times) { }

For PhantomReferences, marks_oops_alive should be false, for the same
reason as for soft/weak/final.

------------------------------------------------------------------------------