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

Kim Barrett kim.barrett at oracle.com
Tue May 15 13:48:03 UTC 2018

> On May 15, 2018, at 4:17 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> Hi,
> On Mon, 2018-05-14 at 18:09 -0400, Kim Barrett wrote:
>>> On May 14, 2018, at 8:45 AM, Thomas Schatzl <thomas.schatzl at oracle.
>>> com> wrote:
>>> Hi Kim,
>>> some initial comments…
>> Thanks for looking at this.
>>> On Sun, 2018-05-13 at 21:32 -0400, Kim Barrett wrote:
>>>> […]
>>>> CR:
> [...]
>>> - please remove the unused complete_gc parameter from
>>> ReferenceProcessor::process_phase2.
>> I intentionally left it alone.  Removing it completely starts getting
>> messy when one starts walking up the callers and hits the parallel
>> task layer (either drop the argument there instead, or have two
>> different work functions with consequent fannout).  And I expect this
>> interface is going to be completely changed by JDK-8202845, so it
>> didn't seem worth doing much here.
> Oh, I would have simply kept the parameter in the ProcessTask layer.
> You are right, we want to discuss this later though.
> [...]
> Thanks for all the other clarifications in the documentation.
>> New webrev, rebased to include JDK-8201491:
>> http://cr.openjdk.java.net/~kbarrett/8203028/open.01/
>> New webrevs with above changes:
>> full: http://cr.openjdk.java.net/~kbarrett/8203028/open.02/
>> incr: http://cr.openjdk.java.net/~kbarrett/8203028/open.02.inc/
>  maybe some really stupid question, but: do we really not need to
> update the next field if we discovered an (inactive) Reference? (in
> instanceRefKlass.inline.hpp, in oop_oop_iterate_discovery).

The next field does get updated, just not via any bespoke mechanism.
It's now treated as an ordinary oop-containing field, just like queue.
See the InstanceRefKlass::update_nonstatic_oop_maps change.

oop_oop_iterate_discovery and friends are helpers for
oop_oop_iterate_ref_processing_specialized.  That function is a helper
for InstanceRefKlass::oop_oop_iterate(|_reversed|bounded).  And those
functions call InstanceKlass::oop_oop_iterate (processing the fields
in the oop map, now including next) as well as the _ref_processing

> One situation that comes to my mind is that while a (Final-)Reference
> is on the pending list it might still be moved (concurrent
> discovery/Remark puts it on the pending list, some mixed gc might move
> it later). While nobody seems to be touching that "next" reference any
> more for FinalReferences after being marked that way, it seems bad
> style to have a dangling oop (and probably messes up some heap
> verification).

There’s no dangling oop; next is treated like all normal oop fields, e.g.
it is no longer treated specially like referent and discovered.

> Another is situation is that the next field is used as a link in the
> ReferenceQueue (for non-FinalReference), so the next field is always be
> a valid oop (and fixed up), shouldn't it? (Since these References
> referent is NULL, they won't be discovered, but the next field needs to
> be iterated over?)

And it is, in the normal fashion used for other oop fields.

> I do not think the next field can be the only source of a new
> undiscovered object graph (it is either NULL or self-looped or part of
> a ReferenceQueue) though.

I'm not sure what you are getting at here.  In process_phase2, the
only invocation of keep_alive now is on the is_alive-returned-true
referent.  But maybe the above answer about next field handling helps
here too?

More information about the hotspot-gc-dev mailing list