RFR: 8031401: Remove unused code in the reference processor

Bengt Rutisson bengt.rutisson at oracle.com
Wed Apr 22 08:07:46 UTC 2015


Hi Kim,

On 2015-04-22 04:54, Kim Barrett wrote:
> Please review this change to remove JVM code to support compatibility
> with old (pre-JDK-4243978 fix) versions of the JDK.
>
> All of the JVM code that was conditional on various forms of
> pending_list_uses_discovered_field now assume that to be true, with
> the code for the false case being deleted.
>
> JDK_Version::initialize() now examines the
> pending_list_uses_discovered_field configuration bit provided by the
> JDK, and calls vm_exit_during_initialization if the bit is not set,
> e.g. the JDK is not claiming to provide that behavior.
> [Given that we now build JDK and JVM together, it's not clear that we
> still need this and other similar configuration tests, but I'm going
> to leave that to someone else to worry about.]
>
> As part of this change, removed DiscoveredListIterator::make_active
> and the call to it from ReferenceProcessor::process_phase1.
> make_active is problematic because:
>
> - Only active references are ever discovered. So when the reference
> was added to the list it had a NULL next field.
>
> - The only place that deactivates a reference is enqueue (either by
> the collector or by Java code).
>
> - For a STW collector, there can't be an enqueue operation (of either
> kind) between discovery and process_phase1, so the next field is
> necessarily still NULL.
>
> - For a concurrent collector, gc enqueuing doesn't occur between
> discovery and process_phase1, but Java enqueuing could. But if the
> reference has been enqueued, the next field is the link in the queue
> list. Bashing it to NULL here is very bad.
>
> make_active() seems to be a lingering artifact of
> !pending_list_uses_discovered_field, and should have been
> conditionalized on that by the fix for JDK-4965777.  It would have
> been OK and even correct for a STW collector, but it has always been
> wrong for a concurrent collector.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8031401
>
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8031401/webrev.00/

Thanks for cleaning this up!

I discussed the make_active() removal with Per a bit. As far as we can 
tell you are correct that this was never needed and potentially wrong.

The change looks good to me.

Thanks,
Bengt

>
> Testing:
> JPRT, RefWorkload with G1, CMS, and ParOld.
> Aurora ad hoc GC Nightly & Quick tests
>



More information about the hotspot-gc-dev mailing list