JDK-8029255 : G1: Reference processing should not enqueue references on the shared SATB queue
  • Type: Bug
  • Component: hotspot
  • Sub-Component: gc
  • Affected Version: hs25,9
  • Priority: P5
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2013-11-27
  • Updated: 2014-07-29
  • Resolved: 2014-01-10
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 8 JDK 9
8u20Fixed 9 b03Fixed
Related Reports
Blocks :  
Relates :  
Description
A side effect of fixing JDK-8029162 is that the reference processor will start to add references to the shared SATB queue when enqueuing discovered references. This is unnecessary and incorrect because:

1) Processing discovered references does not detach them from the object graph. It just prepends them to a list. I.e. enqueuing them on the SATB queue is unnecessary.
2) It will potentially enqueue references in the SATB queue which are part of the collection set, which will cause verify_no_cset_oops() check to fail and cause an abort.

Because the shared SATB queue is currently never enabled this is not an issue at the moment. However, this needs to be fixed before JDK-8029162 and JDK-8029075 can be completed.

Current ILW = LLL = P5
However, this bug is blocking JDK-8029162, which in turn is blocking JDK-8029075, which at some point will change it to ILW = HMH = P1

Comments
Bengt and I have discussed how best to solve this. There are at least three alternative, all of them accomplish the same thing in the end. I'll shortly document them here in case we want to use an alternative approach in the future. For now we've dicisded to go with alternative 1. 1) Adjust the reference processor so that it doesn't use pre-barriers, and therefore avoids adding references to the shared SATB queue. This is kind of the right thing to do, but the reference processor code is already fairly complex and it's not obvious if the relatively generic reference processor code should have to care about collector specific things like when not to use a specific barrier barrier. 2) During a GC pause we filter the SATB queues so that oops pointing into the collection set are removed. Later we do reference processing, which can add new entries to the shared SATB queue. A simple fix would be to just filter the queue(s) again after reference processing has finished. For example, after evacuate_collection_set() we would simply do something like this: --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp @@ -4098,6 +4098,10 @@ // Actually do the work... evacuate_collection_set(evacuation_info); + if (mark_in_progress()) { + JavaThread::satb_mark_queue_set().filter_thread_buffers(); + } + We would probably want to add a new function (instead of calling filter_thread_buffers()) which just filters the shared STAB queue instead of all SATB queues. The drawback of this approach is that the reference processor is basically doing unnecessary work in the first place by adding references to the SATB queue and the added filter step is just there to undo that work. In case we happened to discover a huge amount of references the extra filter step might in worst case add to the pause time. 3) Temporarily deactivate the shared SATB queue just before doing reference processing and active the queue again when reference processing is done. Just as alternative 2, this approach feels in some way more like a workaround rather than a proper fix. On the other hand it would allow the reference processor to be less collector specific and instead force the caller to setup the environment in which the reference processor should work. As mentioned, we're going with alternative 1 for now, but we might want to reconsider this in the future. Alternative 2 is probably the least attractive option.
08-01-2014

noreg-hard justification: Hard to write a specific test for this as there's no way to provoke this problem at the moment. The shared SATB queue is always disabled at this time so all attempts to enqueued references will be dropped anyway. Should references be incorrectly enqueued anyways the G1 verification code (enabled in release builds) will catch that, so problems in this area will not go unnoticed.
09-12-2013