RFR: 8279016: JFR Leak Profiler is broken with Shenandoah [v6]

Aleksey Shipilev shade at openjdk.org
Tue Oct 1 15:19:58 UTC 2024


> While testing unrelated Shenandoah patch, I caught a GC assert when Leak Profiler was running ([JDK-8337194](https://bugs.openjdk.org/browse/JDK-8337194)). 
> 
> Leak Profiler is notorious in using the mark words for its own needs. We have been trying to mitigate its impact on GCs by moving to separate bitsets for tracking marked objects, or by treating "marked without fwdptr" as "JFR marked" and handling it. But this is not reliable, since things like [putting indexes in mark word](https://github.com/openjdk/jdk/blob/3baff2af6a30cc6cb2e0d4391db1cf7e61c33f64/src/hotspot/share/jfr/leakprofiler/chains/edgeStore.cpp#L275-L280) sneak in. This is okay for Leak Profiler alone, since it restores the mark words after the operation completes, but that is still not enough when GC is already running.
> 
> I say we side-step this whack-a-mole by cleanly bailing from JFR op, when we know it is unsafe to do. I thought to use `VM_Operation::doit_prologue`, but I think GC start may sneak in between checking in prologue and op start.
> 
> This realistically only affects Shenandoah. All other STW collectors would never see what Leak Profiler did with mark words. ZGC would not see it, since it does not care about mark words for its own operation.
> 
> Additional testing:
>  - [x] `jdk/jfr/event/oldobject/` pass by default (100x times)
>  - [x] `jdk/jfr/event/oldobject/` pass with `-XX:+UseShenandoah` (1000x)

Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:

 - Merge branch 'master' into JDK-8279016-jfr-leak-profiler-shenandoah
 - Just exclude the tests for Shenandoah
 - Merge branch 'master' into JDK-8279016-jfr-leak-profiler-shenandoah
 - Tighten up comments prose
 - Roman's review: more precise GC state check, more includes
 - Referencing INCLUDE_SHENANDOAHGC nominally requires macro.hpp
 - Try to make tests faster / more robust
 - Fix
   
   Fix
   
   Make JFR tests more reliable with new behavior with Shenandoah
   
   More precisely, only skip when Shenandoah already has forwarded objects
   
   Revert "More precisely, only skip when Shenandoah already has forwarded objects"
   
   This reverts commit 403824d4b27e091807a838c2c86b3228f75fe056.

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/20328/files
  - new: https://git.openjdk.org/jdk/pull/20328/files/249956fc..515c1869

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=20328&range=05
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20328&range=04-05

  Stats: 260783 lines in 3028 files changed: 213404 ins; 30416 del; 16963 mod
  Patch: https://git.openjdk.org/jdk/pull/20328.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/20328/head:pull/20328

PR: https://git.openjdk.org/jdk/pull/20328


More information about the hotspot-jfr-dev mailing list