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

Roman Kennke rkennke at openjdk.org
Mon Jul 29 15:44:32 UTC 2024


On Mon, 29 Jul 2024 15:34:44 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> 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 incrementally with one additional commit since the last revision:
> 
>   Roman's review: more precise GC state check, more includes

Why is it ok to simply skip PathToGcRootsOperation? AFAICT, this is (only) used in EventEmitter::emit(..), to emit events with reference chains. What is the consequence of not doing so during GC?
Also, why does JFR see from-space objects to begin with? This should not be allowed. Does JFR use raw loads of references to figure out chains? If so, should it use the proper Access API instead? If not - how does it see from-space refs?

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

PR Review: https://git.openjdk.org/jdk/pull/20328#pullrequestreview-2205331205


More information about the hotspot-jfr-dev mailing list