RFR: 8284734: Undiscovered references if using ReferentBasedDiscovery

Kim Barrett kbarrett at openjdk.java.net
Mon Apr 18 10:18:41 UTC 2022


On Tue, 12 Apr 2022 08:18:50 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

> Small change of fixing a crash if using `ReferentBasedDiscovery`.
> 
> Test: hotspot_gc

There really needs to be some testing with ReferentBasedDiscovery.

src/hotspot/share/gc/shared/referenceProcessor.cpp line 998:

> 996:   }
> 997: 
> 998:   HeapWord* const discovered_addr = java_lang_ref_Reference::discovered_addr_raw(obj);

The lookup of discovered_addr could be moved later to where it's used.

src/hotspot/share/gc/shared/referenceProcessor.cpp line 1005:

> 1003:     // following two cases:
> 1004:     //  1. `G1CMTask::make_reference_grey` can push the same oop twice onto the
> 1005:     //     mark stack.

I think make_reference_grey uses the next-mark-bits to avoid pushing an oop onto the mark stack multiple times.

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, ...);
  }

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

Changes requested by kbarrett (Reviewer).

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



More information about the hotspot-gc-dev mailing list