JDK-8051680 : (ref) unnecessary process_soft_ref_reconsider
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: gc
  • Affected Version: 9
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2014-07-22
  • Updated: 2021-07-15
  • Resolved: 2021-07-12
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 18
18 b06Fixed
Related Reports
Blocks :  
JDK-8269596 - Snapshot soft ref policy before marking/copying
Relates :  
JDK-6990442 - (ref) Regression to clearing of a softly reachable object
Relates :  
JDK-6990438 - (ref) Soft reference could consider strong if GC decides not to clear
Relates :  
JDK-8031401 - Remove unused code in the reference processor
Relates :  
JDK-8043575 - Dynamically parallelize reference processing work
Relates :  
JDK-8270282 - Semantically rename reference processing subphases
Relates :  
JDK-8046124 - JEP 134: Intuitive Semantics for Nested Reference Objects
Relates :  
JDK-8202845 - Refactor reference processing for improved parallelism
Description
I think ReferenceProcessor::process_soft_ref_reconsider (formerly known as process_phase1) is unnecessary.  It seems like it shouldn't find anything, because of the similar policy-based test in ReferenceProcessor::discover_reference.  And if it does find anything, the liveness analysis from the referent is executed with discovery disabled, which I think is wrong for what is being done by this phase.  I think we do need to be performing discovery on references found by chasing through the referents of soft references that we choose by policy to keep alive.

So I think this phase is presently an expensive nop, and should be deleted entirely, along with supporting infrastructure such as DiscoveredListIterator::make_active(), which I think is only used here.
Comments
Changeset: 86a20081 Author: Albert Mingkun Yang <ayang@openjdk.org> Date: 2021-07-12 07:53:20 +0000 URL: https://git.openjdk.java.net/jdk/commit/86a20081aefb6d43dc8a4d404adb2c1fb5729585
12-07-2021
Rather than moving soft refreence reconsideration to the preclean phase (as suggested by [~tschatzl]), we could instead call the serial process_soft_reference_reconsider as part of root scanning in the remark pause for CMS (the only collector that needs this), without processing followers at that time. This has the benefit compared to another step in concurrent preclean that there isn't a place for concurrent mutator accesses to still change the answer.
25-02-2019
JDK-8202845 renamed process_phase1 to process_soft_ref_reconsider.
20-11-2018
Soft reference processing phase 1 is an expensive nop for if reference discovery and processing is done in the same pause. If discovery is concurrent, we can move processing phase 1 out into the preclean phase due to how the current soft reference processing works.
22-05-2018
CMS will have the same chaining behavior as our other collectors for soft references encountered and marked through, rather than "discovered", during concurrent marking. I think CMS could be changed to have the same behavior always, by permitting discovery during process_phase1 marking. That would require some care in dealing with the soft reference discovered lists being modified while the old soft reference discovered lists are being processed.
07-09-2016
make_active() was eliminated as part of JDK-8031401.
08-06-2016
Another argument for process_phase1 being unnecessary is that if the earlier discovery decision is changed to keep the (dead) referent, it calls make_active() on the iter entry, which sets the next field of the reference to NULL to "reactivate" it. However, that's problematic. - 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 would be very bad. I suspect this make_active() is a lingering artifact of !pending_list_uses_discovered_field() ? In that case it would have been OK and even correct for a STW collector. But I think it's always been wrong for a concurrent collector. Note that process_phase1 is the only caller of make_active(), so make_active() can be eliminated as part of eliminating process_phase1(). However, if some reason is found to keep process_phase1, make_active should be removed.
13-09-2014
For process_phase1() to be useful, we need to have a soft reference clearing policy which has the following behavior: - during discovery, policy says we can clear. - during reference processing, policy says we should keep. If the policy is based on time of last referent access, that could happen with a concurrent collector. Collector discovers reference when it's been a long time since last access. Mutator later accesses referent. Reference processing later sees a much more recent access time and policy says we should keep. For G1 (or other SATB collector) this isn't a problem, because the mutator access has a read barrier that will have marked the referent. But there's no such read barrier for the CMS collector. So we do need process_phase1() after all, though I think only for the CMS collector.
11-08-2014
It appears that some collectors aren't handling soft references properly (or at least my interpretation of "properly") - or more accurately, are manipulating discovery state in ways that mess up soft reference handling. See http://openjdk.java.net/jeps/134 and related bugs.
07-08-2014
ILW=LHH => P4 Impact = Low: this behavior has been in the code for ages, it appears that it's not been causing any major problems. Likelihood = High: Part of the shared reference processing code. Workaround = High: No workaround available, reference processing is mandatory.
23-07-2014