RFR: 8066827: Remove ReferenceProcessor::clean_up_discovered_references()

Srinivas Ramakrishna ysr1729 at gmail.com
Thu Dec 18 02:51:34 UTC 2014


Looks good to me.

-- Ramki

ysr1729

> On Dec 11, 2014, at 23:28, Kim Barrett <kim.barrett at oracle.com> wrote:
> 
> Note: I will need a sponsor for this change.
> 
> Please review this change to replace the one call to
> ReferenceProcessor::clean_up_discovered_references() with a call to
> ReferenceProcessor::abandon_partial_discovery(), and then remove the
> clean_up function and some follow-on cleanup.
> 
> As noted in the bug report, the benefit from passing a partial
> discovery list from the CMS collector to the compacting collector is
> questionable, while incurring significant complexity and appearing to
> impose a performance cost on the reference processing of other
> collectors, e.g. the need for
> DiscoveredListIterator::update_discovered(). 
> 
> The change to disable and abandon rather than "clean up" the
> discovered list allows us to change the call to
> ReferenceProcessor::enable_discovery() by
> CMSCollector::do_compaction_work() to pass true values for both
> arguments, rather than false values as previously needed.  As a result
> of this, the first argument (verify_disabled) is no longer needed, as
> all callers always pass true.  Also as a result, the only caller
> providing a false value for the second argument (verify_no_refs) is
> the destructor for NoRefDiscovery.  So I've eliminated the first
> argument and made the second default to true, and updated all of the
> callers accordingly (all but ~NoRefDiscovery passing no arguments).
> 
> The bug report suggests that the calls to
> DiscoveredListIterator::update_discovered() are no longer needed and
> that function can also be removed.  I'm deferring that (I'll be filing
> a new RFE) because that's a more complex change to analyze and test,
> so I'm separating it from this much simpler to understand change.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8066827 
> 
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8066827/webrev/
> 
> Testing: JPRT, local jtreg of gc tests
> refworkload with CMS as the collector, with TraceCMSState enabled so I
> could examine the logs and verify some of the tests passed through the
> changed code; there were 113 occurrences with an iteration count of 1.
> 
> JPRT was after Bengt's recent changes to include gc jtreg tests.
> 


More information about the hotspot-gc-dev mailing list