RFR: 8284734: Undiscovered references if using ReferentBasedDiscovery

[Albert Mingkun Yang]

On Mon, 18 Apr 2022 10:14:33 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> src/hotspot/share/gc/shared/referenceProcessor.cpp line 1008:
>> 
>>> 1006:     //  2. CM restarts after mark-stack overflow.
>>> 1007:     assert(UseG1GC, "inv");
>>> 1008:     return true;
>> 
>> This whole block that is checking discovered is, for non-G1 is really just an assert that discovered is nullptr.  It would be better to entirely avoid the fetch of discovered here for other collectors in non-debug builds.  Maybe this block ought to instead be something like
>> 
>>   if (UseG1GC) {
>>     // G1 might process (and so attempt to discover) a reference multiple
>>     // times, due to CM restart after mark-stack overflow.
>>     const oop discovered = ...;
>>     if (discovered != nullptr) {
>>       assert(oopDesc::is_oop(discovered), ...);
>>       return true;
>>     }
>>   } else {
>>     assert(java_lang_ref_Reference::discovered(obj) == nullptr, ...);
>>   }
>
> Can the UseG1GC test instead be discovery_is_concurrent()?  The old comments talk about concurrent discovery.  But can a G1 young-only collection during a concurrent cycle attempt to discover a reference that is already in the CM reference processor?  I'm not sure, and the discovery policy might be relevant to the answer.

> It would be better to entirely avoid the fetch of discovered here for other collectors in non-debug builds.

Agree; since this is a bugfix, I went for the less intrusive approach.

> Can the UseG1GC test instead be discovery_is_concurrent()?

Rediscovery is caused by a peculiar impl detail (`_finger`) in CM not CM being concurrent. Testing `discovery_is_concurrent()` can be a bit misleading. (Both are correct though, because G1 is the only concurrent collector using ref-processor.)

>  But can a G1 young-only collection during a concurrent cycle attempt to discover a reference that is already in the CM reference processor?

No, it cannot, because at the start of young-gc, all refs in CM discovered list are treated as roots, as shown in `G1RootProcessor::evacuate_roots`. IOW, the discovered lists of young-gc and CM share no refs.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8196